Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add unit test on MacOS using Github Actions #14220

Merged
merged 16 commits into from
May 9, 2020
Merged

Conversation

alalazo
Copy link
Member

@alalazo alalazo commented Dec 19, 2019

refers #13485

@alalazo alalazo added macOS tests General test capability(ies) labels Dec 19, 2019
@alalazo

This comment has been minimized.

@alalazo alalazo closed this Dec 19, 2019
@alalazo alalazo reopened this Dec 19, 2019
@alalazo alalazo force-pushed the qa/macos_unit_tests branch from fb0f449 to b8c0302 Compare December 19, 2019 18:52
@alalazo
Copy link
Member Author

alalazo commented Dec 19, 2019

MacOS report uploaded!

@gartung
Copy link
Member

gartung commented Dec 19, 2019

@alalazo see #14236
Spack test will need to be run with

spack test --basetemp /tmp/spack

on macOS once gnupg2 is found correctly.

@alalazo
Copy link
Member Author

alalazo commented Dec 20, 2019

There's another thing we need to understand before merging this PR:

With the exception of GITHUB_TOKEN, secrets are not passed to the runner when a workflow is triggered from a forked repository.

The sentence above can be found here and it's not completely clear to me what does the emphasized part mean.

The previous PR #13826 couldn't upload to codecov because it couldn't find the appropriate secret token. That's reasonable, since from a forked repository I submitted a PR that was trying to use a secret of this repository. I wonder though what happens if we were to merge this in the case of a normal PR that doesn't touch the workflow file but comes from a forked repository - which is fairly normal on Github. If they can't use the secret token to upload to codecov it would be a major pain point.

@alalazo
Copy link
Member Author

alalazo commented Dec 20, 2019

EDIT: Main issue to track for codecov action codecov/codecov-action#29

To follow on the previous comments we might have an answer:

It seems this PR needs to wait for early 2020 to be merged, see codecov/codecov-action#44 (comment). Before a tokenless upload to codecov is implemented all the forks need to store the same secret as the upstream repo in order to upload coverage which is definitely less than optimal.

@alalazo
Copy link
Member Author

alalazo commented Dec 20, 2019

Note there's an issue open for passing secrets to forks. It's marked as "solved" but really means "won't fix".

@alalazo alalazo force-pushed the qa/macos_unit_tests branch 2 times, most recently from 481c75f to 94e20b7 Compare January 3, 2020 07:20
@alalazo
Copy link
Member Author

alalazo commented Jan 3, 2020

@tgamblin For convenience a few references as we were discussing this topic offline:

GITHUB_TOKEN is the only secret passed to forked repos and it basically gives read/write permissions on a lot of stuff if it comes from THIS repo and only read if it comes from a fork. Note that we are not using it in this workflow and my understanding is that it will be useful for us only if we start having workflows that e.g. merge or label issues and PRs automatically (see for instance what you can do with the "contents" permissions).

Currently the only way to have this PR work seamlessly also for forks is to expose the token from codecov in the YAML file - since there's no option at the moment to authenticate with codecov from a fork and preserve our secret codecov token at the same time. The risk is that anybody will in principle be able to upload xml reports to our codecov. Let me know if you want to proceed with that or if we should wait until the codecov guys work on a tokenless integration with Github Actions (the same way they are doing with Travis).

@alalazo alalazo force-pushed the qa/macos_unit_tests branch from 94e20b7 to dead1a9 Compare March 26, 2020 11:15
@alalazo
Copy link
Member Author

alalazo commented Mar 26, 2020

I tried to update this PR since the homepage of codecov/codecov-action now says:

For public repositories, no token is needed

We have another issue to investigate though. It seems that due to a setup specific to Github Actions tests that use capfd to capture output fail and apparently all the output is sent directly to stdout and stderr instead of being captured.

@alalazo alalazo force-pushed the qa/macos_unit_tests branch from 1711ceb to 6839107 Compare April 23, 2020 17:27
@alalazo alalazo force-pushed the qa/macos_unit_tests branch from 351ac16 to ecc59d2 Compare May 9, 2020 11:20
@alalazo alalazo requested a review from ax3l May 9, 2020 12:33
@alalazo
Copy link
Member Author

alalazo commented May 9, 2020

@tgamblin @ax3l Ready for a review!

@tgamblin tgamblin merged commit 05203ec into develop May 9, 2020
@alalazo alalazo deleted the qa/macos_unit_tests branch May 9, 2020 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
macOS tests General test capability(ies)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants