build: drop Travis in favor of Actions

[mmarchini]

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]

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]

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]

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

[sam-github]

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.

[mmarchini]

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]

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]

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

[gengjiawen]

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

[gengjiawen]

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

[mmarchini]

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).

[gengjiawen]

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

[mmarchini]

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.

[richardlau]

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]

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]

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]

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]

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.

[mmarchini]

@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]

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]

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

[richardlau]

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]

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]

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]

Landed in 7c66f45