build: drop Travis in favor of Actions #32450

mmarchini · 2020-03-24T01:35:47Z

GitHub Actions is running all tests already present on Travis, as well
as building on more platforms (OS X and Windows). With Travis we're also
getting timeouts more frequently than with Actions, which gives the
false impression tests are failing (making it harder to triage PRs ready
to merge).

To make our config simpler, CI.yml and pythonpackage.yml got merged. The
coverage is also increased by running tests on OS X.

Signed-off-by: Matheus Marchini mmarchini@netflix.com

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

mmarchini · 2020-03-24T01:38:42Z

Note that the two Travis jobs which were allowed to fail were removed (commit message lint and flake8 on dependencies) because I don't think folks were checking it (since GitHub reported those as success). But I might be wrong, so I'm happy to add them back if folks want to (commit message linting is already proposed on #32336).

mmarchini · 2020-03-24T01:40:40Z

Alternatively, we might want to break this into multiple files (workflows), so if one of them fail due to flaky tests, we can rerun it separately (I think that's how it works, although I'm not sure).

gengjiawen · 2020-03-24T02:54:39Z

I am thinking keep pythonpackage.yml as it is. Just remove travis config file.

sam-github · 2020-03-24T02:56:31Z

Despite being a fan of good commit messages, I don't think anybody checks the builds if they passed, fine by me to delete it.

himself65

remove Travis LGTM. It will auto-canceled if running more than 45mins

GitHub Actions look better. but It a new CI so will also have other issues, example #31329

richardlau

I'm fairly certain most of the lint jobs don't require Python.

There are references to Travis in our collaborator's guide which need to be removed.

I'd be in favour of keeping separate workflows as at the moment you can only restart a complete workflow and not an individual job. Maybe move some of the build jobs out of CI.yml (or move the various linters into something like lint.yml) but that could be done separately.

.github/workflows/CI.yml

mmarchini · 2020-03-24T17:21:55Z

I am thinking keep pythonpackage.yml as it is. Just remove travis config file.

I'm fine keeping pythonpackage.yml, but I would like to understand the reasoning first.

mmarchini · 2020-03-24T17:25:37Z

I'd be in favour of keeping separate workflows as at the moment you can only restart a complete workflow and not an individual job. Maybe move some of the build jobs out of CI.yml (or move the various linters into something like lint.yml) but that could be done separately.

I like this idea. The reason I removed pythonpackage.yml is that it was re-building on Linux (while we also build on Linux on CI.yml) and it ran our JS tests (so it was not just a Python check, it was testing almost everything, which kills the purpose of a Python workflow). I'll re-work the actions as:

linters.yml
build-linux.yml
build-osx.yml
build-windows.yml
misc.yml (for building docs and other minor checks which doesn't qualify as linters)

richardlau · 2020-03-24T17:33:02Z

@mmarchini I'd suggest build-macos.yml instead of build-osx.yml but otherwise the plan sounds good.

gengjiawen · 2020-03-26T00:36:57Z

On the python part, not sure @cclauss want to do multi python version verify ?

gengjiawen · 2020-03-26T00:38:13Z

.github/workflows/test-macos.yml

@@ -0,0 +1,23 @@
+name: test-macOS


Since jenkins convers all platform test, maybe just test linux on github action is enough ?

Running on all three platforms gives authors an early signal on all Tier 1 platforms (~~I could even argue that we should be testing ARM here~~ nevermind, ARM is for self-hosted hosts only, which is a whole other conversation). This is less a matter of coverage (we're not covering anything extra with Travis or GitHub Actions today), but rather a matter of early feedback on an easier to consume interface for outside contributors (this was the goal when Travis was introduced at least).

If we add macOS, should we add windows test too ?

I think so, yes. I just didn't add because I have no idea how we test Windows on Jenkins (the commands I used on Linux and macOS came from Jenkins config, so they are more suited for CI environments).

If anyone knows the best way to run tests on Windows on CI, feel free to comment :)

There's always the option to add tests for Windows in a follow up.

I'm okay with Tier 1 platforms for now. Linux is actually the slowest platform on Actions for our build/tests (* may possibly change if we start running tests on Windows).

I'm also happy for tests to be added on Windows in a follow up PR (we're not running tests on Windows in Actions or Travis CI before this PR).

mmarchini · 2020-03-26T00:44:39Z

On the python part, not sure @cclauss want to do multi python version verify ?

There was no multi python version before, which is why I removed the file. And if we want to introduce multi-version, there's no need for a separate file (matrix is a job attribute, not a file attribute).

cclauss · 2020-03-26T00:50:45Z

https://github.com/nodejs/node/blob/master/.github/workflows/pythonpackage.yml#L12 Is the way to turn on multi Python verify but my sense is that passing on Py 3.5 (until September) and 3.8 is sufficient? Py 2.7 is no longer tested? Is support for that dropped?
https://devguide.python.org/#status-of-python-branches

mmarchini · 2020-03-26T01:04:18Z

https://github.com/nodejs/node/blob/master/.github/workflows/pythonpackage.yml#L12 Is the way to turn on multi Python

Right. Not sure what others think, but keeping a comment in the workflow file to "turn on" other versions is not very useful, since you'll need to edit the file and commit the changes on a PR to test it. The same workflow can be accomplished by just changing the version on the file, no need to keep a stray comment. Also, the comment doesn't mean those versions are supported, supported versions are listed on BUILDING.md.

my sense is that passing on Py 3.5 (until September) and 3.8 is sufficient

I'm fine running Py 3.5 and 3.8 on linters since those are fast, but doing so on the whole build seems too much (we'd be spawning 3 other runs for ~1 hour each). We are not doing this right now anyway, so it's out of the scope of this PR. If running both is really important, seems like something we should be doing on Jenkins instead. Or maybe only in one platform, but we'll be losing coverage in that case.

Py 2.7 is no longer tested? Is support for that dropped?

In theory it's tested on Jenkins, but I don't want to say it for sure.

mmarchini · 2020-03-26T01:14:26Z

If folks are unhappy with the changes I made to pythonpackage.yml, I can revert it and keep this file untouched. The goal of this PR is to drop Travis, and I don't want to diverge the focus to Python coverage in our build infrastructure, I only changed/removed that file because it was redundantly building on Linux.

GitHub Actions is running all tests already present on Travis, as well as building on more platforms (OS X and Windows). With Travis we're also getting timeouts more frequently than with Actions, which gives the false impression tests are failing (making it harder to triage PRs ready to merge). To make our config simpler, CI.yml and pythonpackage.yml got merged. The coverage is also increased by running tests on OS X. Signed-off-by: Matheus Marchini <mmarchini@netflix.com>

mmarchini · 2020-03-26T01:20:08Z

@richardlau I believe I addressed all your comments :)

cclauss

Nice work. I think we are fine to focus only on Python 3.8 for the first line testing.

Jenkins can deal with ~~all the~~ combinations and permutations of hardware and language versions.

gengjiawen

LGTM

mmarchini · 2020-03-26T01:37:16Z

I don't think it deals with all combinations. Might be worth asking on nodejs/build. But I'm 99% sure we use Python 2.7 on some platforms and at least one 3.x on others.

himself65 · 2020-03-26T11:37:52Z

Github Actions haven't cache by default. Should we add actions/cache@v1 to the config?
It will take less time to complete.

richardlau · 2020-03-26T11:48:15Z

Github Actions haven't cache by default. Should we add actions/cache@v1 to the config?
It will take less time to complete.

It's outside the scope of this PR. I've looked at caching but GitHub's implementation is very rudimentary and you have to tell it what to cache and restore. I've looked at implementing ccache (which Travis has built in support for) but we'd have to do a lot more work and there didn't seem to be an easy way to manage the cache (I've had to manually delete the cache several times for Travis to get passing runs).

There's also a limits for caches:

GitHub will remove any cache entries that have not been accessed in over 7 days. There is no limit on the number of caches you can store, but the total size of all caches in a repository is limited to 5 GB. If you exceed this limit, GitHub will save your cache but will begin evicting caches until the total size is less than 5 GB.

richardlau · 2020-03-26T11:58:35Z

I don't think it deals with all combinations. Might be worth asking on nodejs/build. But I'm 99% sure we use Python 2.7 on some platforms and at least one 3.x on others.

We do not have the capability (either people wise or resources) to test every combination of build tooling. All of the hardware used by build has been generously donated and we need to be able to build two LTS lines, current and master which all have different spreads of minimum supported toolchain versions. With regards to Python, there was an effort last year to get Python 3 onto as many of the CI hosts as possible. Newer hosts (e.g. the macOS 10.15 machines) will be Python 3.

mmarchini · 2020-03-26T21:07:16Z

I believe this is a significant enough change to justify a @nodejs/collaborators ping. I'll wait a few days, if there are no objections I'll land it.

mcollina

lgtm

bcoe

I've been gradually switching other open-source like yargs over to actions, and we've been using it at work for the past couple months too. I think this is a great move.

addaleax · 2020-03-27T15:03:08Z

Landed in 7c66f45

GitHub Actions is running all tests already present on Travis, as well as building on more platforms (OS X and Windows). With Travis we're also getting timeouts more frequently than with Actions, which gives the false impression tests are failing (making it harder to triage PRs ready to merge). To make our config simpler, CI.yml and pythonpackage.yml got merged. The coverage is also increased by running tests on OS X. Signed-off-by: Matheus Marchini <mmarchini@netflix.com> PR-URL: #32450 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ben Coe <bencoe@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Michaël Zasso <targos@protonmail.com>

GitHub Actions is running all tests already present on Travis, as well as building on more platforms (OS X and Windows). With Travis we're also getting timeouts more frequently than with Actions, which gives the false impression tests are failing (making it harder to triage PRs ready to merge). To make our config simpler, CI.yml and pythonpackage.yml got merged. The coverage is also increased by running tests on OS X. Signed-off-by: Matheus Marchini <mmarchini@netflix.com> PR-URL: nodejs#32450 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ben Coe <bencoe@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Michaël Zasso <targos@protonmail.com>

GitHub Actions is running all tests already present on Travis, as well as building on more platforms (OS X and Windows). With Travis we're also getting timeouts more frequently than with Actions, which gives the false impression tests are failing (making it harder to triage PRs ready to merge). To make our config simpler, CI.yml and pythonpackage.yml got merged. The coverage is also increased by running tests on OS X. Signed-off-by: Matheus Marchini <mmarchini@netflix.com> Backport-PR-URL: #32608 PR-URL: #32450 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Christian Clauss <cclauss@me.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ben Coe <bencoe@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Michaël Zasso <targos@protonmail.com>

nodejs-github-bot added build Issues and PRs related to build files or the CI. meta Issues and PRs related to the general management of the project. labels Mar 24, 2020

himself65 reviewed Mar 24, 2020

View reviewed changes

richardlau requested changes Mar 24, 2020

View reviewed changes

.github/workflows/CI.yml Outdated Show resolved Hide resolved

.github/workflows/CI.yml Outdated Show resolved Hide resolved

mmarchini added the wip Issues and PRs that are still a work in progress. label Mar 24, 2020

mmarchini force-pushed the remove-travis branch from 1bcc78b to cd6aed7 Compare March 26, 2020 00:28

gengjiawen reviewed Mar 26, 2020

View reviewed changes

mmarchini force-pushed the remove-travis branch from cd6aed7 to 8b68ed4 Compare March 26, 2020 01:19

cclauss approved these changes Mar 26, 2020

View reviewed changes

gengjiawen approved these changes Mar 26, 2020

View reviewed changes

gengjiawen requested a review from richardlau March 26, 2020 01:38

gengjiawen mentioned this pull request Mar 26, 2020

build: add build from tarball #32129

Closed

2 tasks

richardlau approved these changes Mar 26, 2020

View reviewed changes

mmarchini added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed wip Issues and PRs that are still a work in progress. labels Mar 26, 2020

mmarchini self-assigned this Mar 26, 2020

mcollina approved these changes Mar 26, 2020

View reviewed changes

bcoe approved these changes Mar 26, 2020

View reviewed changes

tniessen approved these changes Mar 27, 2020

View reviewed changes

targos approved these changes Mar 27, 2020

View reviewed changes

addaleax closed this Mar 27, 2020

richardlau mentioned this pull request Mar 27, 2020

Revert "test: mark empty udp tests flaky on OS X" #32489

Closed

2 tasks

codebytere mentioned this pull request Apr 3, 2020

v12.16.2 proposal #32313

Merged

targos mentioned this pull request Apr 13, 2020

v13.13.0 release proposal #32813

Merged

sffc mentioned this pull request May 12, 2020

Set up continous integration unicode-org/icu4x#81

Closed

richardlau mentioned this pull request Oct 31, 2023

Add a GHA Windows test workflow? #50489

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: drop Travis in favor of Actions #32450

build: drop Travis in favor of Actions #32450

mmarchini commented Mar 24, 2020

mmarchini commented Mar 24, 2020

mmarchini commented Mar 24, 2020

gengjiawen commented Mar 24, 2020

sam-github commented Mar 24, 2020

himself65 left a comment •

edited

Loading

richardlau left a comment •

edited

Loading

mmarchini commented Mar 24, 2020

mmarchini commented Mar 24, 2020

richardlau commented Mar 24, 2020

gengjiawen commented Mar 26, 2020

gengjiawen Mar 26, 2020

mmarchini Mar 26, 2020 •

edited

Loading

gengjiawen Mar 26, 2020

mmarchini Mar 26, 2020

richardlau Mar 26, 2020

mmarchini commented Mar 26, 2020

cclauss commented Mar 26, 2020 •

edited

Loading

mmarchini commented Mar 26, 2020

mmarchini commented Mar 26, 2020 •

edited

Loading

mmarchini commented Mar 26, 2020

cclauss left a comment •

edited

Loading

gengjiawen left a comment

mmarchini commented Mar 26, 2020 •

edited

Loading

himself65 commented Mar 26, 2020

richardlau commented Mar 26, 2020

richardlau commented Mar 26, 2020

mmarchini commented Mar 26, 2020

mcollina left a comment

bcoe left a comment

addaleax commented Mar 27, 2020

build: drop Travis in favor of Actions #32450

build: drop Travis in favor of Actions #32450

Conversation

mmarchini commented Mar 24, 2020

Checklist

mmarchini commented Mar 24, 2020

mmarchini commented Mar 24, 2020

gengjiawen commented Mar 24, 2020

sam-github commented Mar 24, 2020

himself65 left a comment • edited Loading

Choose a reason for hiding this comment

richardlau left a comment • edited Loading

Choose a reason for hiding this comment

mmarchini commented Mar 24, 2020

mmarchini commented Mar 24, 2020

richardlau commented Mar 24, 2020

gengjiawen commented Mar 26, 2020

gengjiawen Mar 26, 2020

Choose a reason for hiding this comment

mmarchini Mar 26, 2020 • edited Loading

Choose a reason for hiding this comment

gengjiawen Mar 26, 2020

Choose a reason for hiding this comment

mmarchini Mar 26, 2020

Choose a reason for hiding this comment

richardlau Mar 26, 2020

Choose a reason for hiding this comment

mmarchini commented Mar 26, 2020

cclauss commented Mar 26, 2020 • edited Loading

mmarchini commented Mar 26, 2020

mmarchini commented Mar 26, 2020 • edited Loading

mmarchini commented Mar 26, 2020

cclauss left a comment • edited Loading

Choose a reason for hiding this comment

gengjiawen left a comment

Choose a reason for hiding this comment

mmarchini commented Mar 26, 2020 • edited Loading

himself65 commented Mar 26, 2020

richardlau commented Mar 26, 2020

richardlau commented Mar 26, 2020

mmarchini commented Mar 26, 2020

mcollina left a comment

Choose a reason for hiding this comment

bcoe left a comment

Choose a reason for hiding this comment

addaleax commented Mar 27, 2020

himself65 left a comment •

edited

Loading

richardlau left a comment •

edited

Loading

mmarchini Mar 26, 2020 •

edited

Loading

cclauss commented Mar 26, 2020 •

edited

Loading

mmarchini commented Mar 26, 2020 •

edited

Loading

cclauss left a comment •

edited

Loading

mmarchini commented Mar 26, 2020 •

edited

Loading