Skip to content

Commit

Permalink
Test on Node 8 and 10 on Circle CI
Browse files Browse the repository at this point in the history
  • Loading branch information
@martijnwalraven
martijnwalraven committed Jun 11, 2018
1 parent 4a068cf commit 301e144
Showing 1 changed file with 3 additions and 25 deletions.
28 changes: 3 additions & 25 deletions .circleci/config.yml
View file
Original file line number Diff line number Diff line change
@@ -1,21 +1,5 @@
version: 2

#
# Reusable Snippets!
#
# These are re-used by the various tests below, to avoid repetition.
#
run_install_desired_npm: &run_install_desired_npm
run:
# Due to a bug, npm upgrades from the version of npm that ships with
# Node.js 6 (npm v3.10.10) go poorly and generally causes other problems
# with the environment. Since yarn is already available here we can just
# use that to work-around the issue. It's possible that npm cleanup might
# prevent this from being necessary, but this can be removed once Node 6 is
# no longer being built below.
name: Install npm@5, but with yarn.

This comment has been minimized.

Sign in to view

Copy link
@abernix

abernix Jun 11, 2018

Member

@martijnwalraven This code was used to ensure a specific version of npm was installed, rather than just the "last version which shipped with a particular version of Node.js".

This repository is (now) testing Node.js 8 and Node.js 10, which ship with different versions of npm: 8 ships with npm 5.6.0 and 10 ships with 6.1.0. While 6.1.0 is the latest version in the 6-line, npm 5.6.0 was a bit buggy in some regards (5.10.0 is the latest v5 stable dist-tag now).

I would suggest keeping a baseline and expected version of npm on all versions of Node. Even if Node 8 was out of the picture, this would still fail to use the latest "stable" version of npm which is often what is desired (and most developers will be on, thanks to the "npm is outdated" banner which most developers have enabled).

This comment has been minimized.

Sign in to view

Copy link
@martijnwalraven

martijnwalraven Jun 11, 2018

Author Contributor

Ah, I just read 'this can be removed once Node 6 is no longer being built below', but that makes sense. Wouldn't we want to test Node 10 with the latest npm 6 though?

This comment has been minimized.

Sign in to view

Copy link
@abernix

abernix Jun 11, 2018 •
edited
Loading

Member

Ah, that comment was written by me, but not clear enough. My intention was to remove the yarn portion of it and continue to use npm, since it's actually the npm version which shipped with Node.js 6 (3.10.10) which was the problem requiring us to use yarn to install other npm versions.

Previously, this was installing npm@latest, which became problematic in the brief week or two after npm@6 was released since v6 dropped support for Node.js 4 but we were still testing Node.js 4 even though it was still LTS maintenance for another couple weeks. (See #984).

Finally, Node.js 10 actually shipped with npm@5 (rather than 6) because of a miscommunication between the npm and Node.js projects. That's all reconciled now.

I'd recommend installing npm@6 (or even npm@latest again, since npm has mostly figured out their version tagging now 🤕) using npm (whatever version ships with Node.js) now that Node.js 6 has been dropped from tests.
command: sudo yarn global add npm@5

# These are the steps used for each version of Node which we're testing
# against. Thanks to YAMLs inability to merge arrays (though it is able
# to merge objects), every version of Node must use the exact same steps,
Expand All @@ -36,16 +20,12 @@ jobs:
# Platform tests, each with the same tests but different platform or version.
# The docker tag represents the Node.js version and the full list is available
# at https://hub.docker.com/r/circleci/node/.
Node.js 6:
docker: [ { image: 'circleci/node:6' } ]
<<: *common_test_steps

Node.js 8:
docker: [ { image: 'circleci/node:8' } ]
<<: *common_test_steps

Node.js 9:
docker: [ { image: 'circleci/node:9' } ]
Node.js 10:
docker: [ { image: 'circleci/node:10' } ]
<<: *common_test_steps

# Other tests, unrelated to typical code tests.
Expand Down Expand Up @@ -75,11 +55,9 @@ workflows:
version: 2
Build and Test:
jobs:
- Node.js 6:
<<: *ignore_doc_branches
- Node.js 8:
<<: *ignore_doc_branches
- Node.js 9:
- Node.js 10:
<<: *ignore_doc_branches
- Linting:
<<: *ignore_doc_branches
Expand Down

0 comments on commit 301e144

Please sign in to comment.