Resolve CI issues

[robertshuford]

Problems:

• Latest node, v18.10.0, fails at npm install on xenial, breaking builder.
  • Resolved by running nvm install --lts before nvm install 16, this sets the default node version correctly for the tests and uses 16 for npm install. This is a workaround which won't be required after migrating from Travis CI to GitHub Actions, where more control over the tests to distro orchestration is possible.
• Some old versions of node won't install on versions of ubuntu newer than xenial, breaking tests. Example: 0.6.21
  • Got the tests passing on xenial so no upgrade is needed at this time.
• Brew removed from Ubuntu runner images. [Ubuntu] homebrew will be removed from PATH variable actions/runner-images#6283
  • brew was used to install shellcheck, readding brew using Homebrew/actions/setup-homebrew action.
• ShellCheck fails on install.sh due to which error. https://www.shellcheck.net/wiki/SC2230
  • Disabled the rule at the failure points to get the build passing. Switching from which to command -v will be address in a future PR.

[ljharb]

The which error says "This is an optional suggestion. It must be explicitly enabled with a directive enable=deprecate-which in a # shellcheck comment or .shellcheckrc" - since it's optional, let's avoid fixing it for now and do that in a separate PR?

[ljharb]

ps this is very helpful, thank you - i'd love to get this landed asap so i can unblock a bunch of PRs.

[robertshuford]

The which error says "This is an optional suggestion. It must be explicitly enabled with a directive enable=deprecate-which in a # shellcheck comment or .shellcheckrc" - since it's optional, let's avoid fixing it for now and do that in a separate PR?

Is it possible that documentation is out of date? I don't see a enable=deprecate-which comment in the repo and there isn't a .shellcheckrc file. So if it is still optional, how did it get enabled? On by default in the runner?
Regardless, I'll attempt to disable that check with a comment in this PR so it can be fixed in another PR.

[robertshuford]

FYI I made the nvm install --lts change as a test to see if that would fix the build.

The root problem appears to be that with the latest version of node there is no longer a single Ubuntu version that can install all versions of node.

A fix could be to devise a strategy where nvm is only tested with old versions of node, up to 16, on xenial, and also where nvm is only tested with newer versions of node, starting at some version , up to latest.

This could be done through reorganization of the tests and coordinating which tests are run on which ubuntu runner version through CI configuration. Or we could make the tests aware of an optional minimum and maximum version, and the test itself would only run if the version is within that range, if a range is defined.

[ljharb]

That reorganization should be handled as part of migrating from travis to github actions. This PR is hopefully sufficient to get tests passing, which is valuable.

[robertshuford]

I'm looking at the failing test with nvm install 16 and I'm not sure why it failed with that but passes with nvm install --lts and nvm install node.

I could got with nvm install --lts to fix the build for now, knowing it introduces a race condition between:

migrating from travis to github actions

and

18 becomes lts

[robertshuford]

FYI my rational behind running nvm install --lts first is:

1. It looks like there some some bug will the tests that isn't triggered when using nvm install node or nvm install --lts.
2. By running nvm install --lts before nvm install 16 it looks like we can avoid trigging this bug in the way the tests are run in CI with a compatible node version for xenial.
3. I choose nvm install --lts vs nvm install node because it installs an older version of node and maximizes the time before the build will fail due to nvm install attempting to install a version of node that is too new for the xenial.

[ljharb]

Since nvm install --lts and nvm install 16 are currently identical, I'm very confused why pinning it to 16 wouldn't work, and I'm even more confused why installing two node versions would help anything.

[robertshuford]

Since nvm install --lts and nvm install 16 are currently identical, I'm very confused why pinning it to 16 wouldn't work, and I'm even more confused why installing two node versions would help anything.

I haven't been able to identify the exact issue, but it could be how nvm sets the default version when installing node or --lts vs installing a specific version number, and how that interacts with the flow of the test scripts in CI.

root@e1e23c7c205f:/nvm-sh/nvm# nvm ls
->     v16.17.1
default -> lts/* (-> v16.17.1)
iojs -> N/A (default)
unstable -> N/A (default)
node -> stable (-> v16.17.1) (default)
stable -> 16.17 (-> v16.17.1) (default)
lts/* -> lts/gallium (-> v16.17.1)
lts/argon -> v4.9.1 (-> N/A)
lts/boron -> v6.17.1 (-> N/A)
lts/carbon -> v8.17.0 (-> N/A)
lts/dubnium -> v10.24.1 (-> N/A)
lts/erbium -> v12.22.12 (-> N/A)
lts/fermium -> v14.20.1 (-> N/A)
lts/gallium -> v16.17.1

At this time, because nvm install --lts and nvm install 16 are the same, when executing nvm install 16 it doesn't actually install a second node version:

v16.17.1 is already installed.
Now using node v16.17.1 (npm v8.15.0)

[ljharb]

ahhh - in that case, nvm install 16 && nvm unalias default would work?

[ljharb]

Looks great, thank you!!