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.

Sign up for GitHub

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

Jump to bottom

build: move CI to GitHub Actions #323

Closed
wants to merge 2 commits into from
Closed

build: move CI to GitHub Actions #323

wants to merge 2 commits into from

Conversation

mmarchini
Copy link
Contributor

Reduce CI time by moving to GitHub Actions.

@codecov-io
Copy link

codecov-io commented Nov 7, 2019 •
edited
Loading

Codecov Report

Merging #323 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #323   +/-   ##
=======================================
  Coverage   79.02%   79.02%           
=======================================
  Files          33       33           
  Lines        4247     4247           
=======================================
  Hits         3356     3356           
  Misses        891      891

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec01604...45b83f7. Read the comment docs.

@mmarchini mmarchini force-pushed the github-actions-ci branch 2 times, most recently from 2dbffb5 to f0a77da Compare November 7, 2019 01:08
@richardlau
Copy link
Member

Over in CITGM we're proposing keeping Travis around for code coverage: nodejs/citgm#759

@mmarchini
Copy link
Contributor Author

Over in CITGM we're proposing keeping Travis around for code coverage: nodejs/citgm#759

Let me take a look, I don't think we had to enable any permissions for codecov in this repository (I remember making a PR and codecov was just working).

@richardlau
Copy link
Member

Over in CITGM we're proposing keeping Travis around for code coverage: nodejs/citgm#759

Let me take a look, I don't think we had to enable any permissions for codecov in this repository (I remember making a PR and codecov was just working).

I think that was the point @targos was making -- that on Travis nothing extra had to be enabled for codecov but that wasn't the case for Actions.

@richardlau
Copy link
Member

For reference: https://community.codecov.io/t/whitelist-github-action-servers-to-upload-without-a-token/491

@mmarchini
Copy link
Contributor Author

fyi I'm testing on my fork to avoid notification spam here.

I'm having trouble getting tests to pass because apparently core dumps are not being generated. Not sure why yet, based on system settings it doesn't look like core dumps are blocked.

@mmarchini
Copy link
Contributor Author

mmarchini commented Nov 7, 2019 •
edited
Loading

Should be working now. A few things we still need to do:

  • Daily runs
  • Update README
  • Code coverage. codecov said they will enable GITHUB_TOKEN support after Nov 13 (https://community.codecov.io/t/whitelist-github-action-servers-to-upload-without-a-token/491/8). Still won't work for forks though. TBH I don't feel strong enough about keeping codecov around if we need two CI services for that (llnode needs to run all tests on all supported Node.js versions to generate a relevant coverage report, keeping travis would kill the purpose of speeding up CI).
  • Better handling of nightly/canary tests (I have ideas, will probably open an issue about that later)

@mmarchini
test: use gdb to capture core on tests
c8ebdf7 
gdb let us save core dumps on Linux (similar to what we do on OS X). For
some reason, GitHub Actions are not generating system core dumps, so
this is a good alternative to keep tests working when we move.
@mmarchini mmarchini force-pushed the github-actions-ci branch 2 times, most recently from e5c3eea to 78d5ec8 Compare January 9, 2020 18:08
@mmarchini
build: move CI to GitHub Actions
6c82b1e 
Reduce CI time by moving to GitHub Actions.

Fixes: #312
@mmarchini
Copy link
Contributor Author

Ok, I'm happy with this setup now. CI is a lot faster than Travis, and it's able to run more jobs in parallel as well.

We lost the build matrix for now, but I don't think it's a problem. If someone creates a https://github.com/bjfish/travis-matrix-badges for GitHub Actions, we can use it. I also wouldn't be surprised if GitHub makes this information easily accessible in the repository home page in the future.

I had to change coverage from Codecov to Coveralls because Codecov doesn't use the GITHUB_TOKEN yet. Coveralls doesn't add a comment in the repository, but coverage information is available as a GitHub Check:

img-2020-01-09-103941

cc @nodejs/llnode @richardlau

richardlau
richardlau approved these changes Jan 9, 2020
View reviewed changes
.github/workflows/push.yml Show resolved Hide resolved
mmarchini added a commit that referenced this pull request Jan 9, 2020
@mmarchini
test: use gdb to capture core on tests
6f27d84 
gdb let us save core dumps on Linux (similar to what we do on OS X). For
some reason, GitHub Actions are not generating system core dumps, so
this is a good alternative to keep tests working when we move.

PR-URL: #323
Reviewed-By: Richard Lau <[email protected]>
mmarchini added a commit that referenced this pull request Jan 9, 2020
@mmarchini
build: move CI to GitHub Actions
ecb8d0e 
Reduce CI time by moving to GitHub Actions.

Fixes: #312

PR-URL: #323
Reviewed-By: Richard Lau <[email protected]>
@mmarchini
Copy link
Contributor Author

Landed in 9a58d8c...ecb8d0e

@mmarchini mmarchini closed this Jan 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardlau richardlau richardlau approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mmarchini @codecov-io @richardlau