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

ci: migrate to github actions #794

Merged
merged 2 commits into from Dec 22, 2020

Conversation

grondo
Copy link
Contributor

@grondo grondo commented Dec 21, 2020

This PR migrates flux-sched from Travis-CI to GitHub Actions/Workflows for all CI tests.

Most of the work was already done in flux-core. I generalized docker-run-checks.sh and checks_run.sh in flux-core first (PR pending), so that those files can be copied into flux-sched (and hopefully other flux-framework projects) with minimal changes (just update the PROJECT name at the top).

I tried to keep the checks run the same as were in Travis-CI.

One thing I didn't do here was add the python format and pylint checks. This is just a straight conversion at this time.

Copy link
Member

@SteVwonder SteVwonder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @grondo! LGTM! Should we wait for the generalization in flux-core to go in first? I'm assuming this is OK to go in whenever?

@grondo
Copy link
Contributor Author

grondo commented Dec 21, 2020

As long as they both go in eventually then future synchronization will be easier (order of PRs merged doesn't matter)

The coverage dropped here. I wonder if it is because python coverage is now supported by virtue of updating to checks_run.sh from flux-core...

@grondo
Copy link
Contributor Author

grondo commented Dec 21, 2020

Looks like the drop is due to hlapi/bindings (perhaps?) Was this source ignored before?

@grondo
Copy link
Contributor Author

grondo commented Dec 21, 2020

Feel free to set MWP when you are ready, if you want to leave the coverage investigation for later.

@SteVwonder SteVwonder added the merge-when-passing mergify.io - merge PR automatically once CI passes label Dec 21, 2020
Copy the updates from flux-core required to move all CI checks from
Travis-CI to Github Actions. This includes

Update to the latest docker-run-checks.sh from flux-core, which includes
enhanced portability between flux-framework projects, runs a `checks_run.sh`
script instead of `travis_run.sh`, and includes new options like
`--recheck` (run `make recheck` on test failure to w/a flaky tests)

Also include from flux-core:

 * A Python script, `generate-matrix.py` to generate the build matrix
   as JSON, instead of putting the matrix directly in the actions YAML.

 * The checks-annotate.sh` script, which attempts to better annotate
   test failures and other errors after a CI failure, resulting in less
   poring through workflow logs post-mortem.

 * The `docker-deploy.sh` script for pushing docker images to Docker Hub.

Finally, remove travis-specific data from the project, and add a
"ci" workflow configuration to .github/workflows/main.yml.
We've moved all CI status checks to GitHub actions.

Remove the reference to Travis CI and explicitly list all required
Github Action status checks.
@grondo
Copy link
Contributor Author

grondo commented Dec 22, 2020

I fixed a typo in src/test/checks_run.sh. This will need to manually merged.

@codecov
Copy link

codecov bot commented Dec 22, 2020

Codecov Report

Merging #794 (c1ef699) into master (b8ba989) will decrease coverage by 0.9%.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           master    #794     +/-   ##
========================================
- Coverage    73.5%   72.5%   -1.0%     
========================================
  Files          81      85      +4     
  Lines        8562    8813    +251     
========================================
+ Hits         6298    6395     +97     
- Misses       2264    2418    +154     
Impacted Files Coverage Δ
resource/hlapi/bindings/c/reapi_cli.cpp 0.0% <0.0%> (ø)
resource/utilities/grug2dot.cpp 0.0% <0.0%> (ø)
resource/hlapi/bindings/c/reapi_module.cpp 0.0% <0.0%> (ø)
src/cmd/flux-tree-helper.py 93.2% <0.0%> (ø)
qmanager/modules/qmanager_callbacks.cpp 78.2% <0.0%> (+0.6%) ⬆️

@grondo
Copy link
Contributor Author

grondo commented Dec 22, 2020

I think the coverage change here can be explained by the fact that the old coverage method seemed to ignore sources that had 0 coverage, i.e. those that aren't run at all, while (somehow) the coverage in this PR finds the following "new" files:

 New file resource/hlapi/bindings/c/reapi_module.cpp
 New file resource/hlapi/bindings/c/reapi_cli.cpp
 New file resource/utilities/grug2dot.cpp
 New file src/cmd/flux-tree-helper.py

flux-tree-helper.py can be explained because updating docker-run-checks.sh from flux-core adds support for Python coverage. I'm not sure why the other files (which seem to be unused) are ignored on the main branch but found by codecov here.

@SteVwonder
Copy link
Member

while (somehow) the coverage in this PR finds the following "new" files:

Thanks for digging into that @grondo!

This will need to manually merged.

Pushing the button.

@SteVwonder SteVwonder merged commit 44833c8 into flux-framework:master Dec 22, 2020
@grondo
Copy link
Contributor Author

grondo commented Dec 22, 2020

Thanks!

@grondo grondo deleted the github-actions branch December 22, 2020 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-when-passing mergify.io - merge PR automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants