Skip to content

drop node 12 / matrix against current, and latest 3 LTS node versions #369

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

Closed
wants to merge 7 commits into from
Closed

drop node 12 / matrix against current, and latest 3 LTS node versions #369

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9a3b0cd
drop node 12 / add node 18 to test matrix
theinterned Dec 28, 2022
5057fbc
also matrix for latest node (19)
theinterned Dec 29, 2022
a8fa032
use dynamic LTS and latest versions
theinterned Dec 29, 2022
20e83a5
add test to fail on latest node version
theinterned Jan 9, 2023
f273fd9
fail on latest node version
theinterned Jan 9, 2023
786bc35
create an issue on current node fail
theinterned Jan 10, 2023
4f9be41
remove failing node-version test
theinterned Jan 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion .github/workflows/nodejs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ jobs:

strategy:
matrix:
node-version: [12.x, 14.x, 16.x]
node-version:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we'll get confused / have some unexpected behavior when these things turn over.

Like, the day that a new version gets added, what if CI starts failing? That would be annoying to fix without a hardcoded list. WDYT?

Copy link
Contributor

@smockle smockle Jan 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unexpected behavior when these things turn over

Because the matrix includes latest, when lts/* begins pointing to a new version, no failures should occur, because that new version would have already been tested, back when it was latest. So floating version (viz. lts/-2, lts/-1, lts/*) values ought to be safe.

That said, when latest itself begins pointing to a new version, failures might occur. But if we remove latest, then lts/* loses the safety described above.

Copy link
Contributor

@smockle smockle Jan 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when latest itself begins pointing to a new version, failures might occur

Here’s one workaround: For latest (and only latest), if any step of the job fails (failure()), open an issue, then exit 0, so the overall workflow run is still green.

So, we’d still be notified of failures, but they wouldn’t block PRs (unless another version is also failing), etc.

Copy link
Contributor Author

@theinterned theinterned Jan 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay! cool. I got this working!

Here's a sample failed run: https://github.com/github/eslint-plugin-github/actions/runs/3884555154, and the issue it created: #378

To test this I created a failing test 20e83a5 which I will now delete.

- lts/-2
- lts/-1
- lts/*
- current

steps:
- uses: actions/checkout@v3
Expand All @@ -23,5 +27,21 @@ jobs:
cache: npm
- name: Install
run: npm ci
- name: Node version
id: version
run: echo "node-version=$(node -v)" >> $GITHUB_OUTPUT
- name: Test
id: test
run: npm test
continue-on-error: ${{ matrix.node-version == 'current' }}
- name: Create Issue on Fail
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this going to create a new issue on every failure?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it will. Can you think of a way to improve that?

if: ${{ matrix.node-version == 'current' && steps.test.outcome != 'success' }}
uses: actions/github-script@v6
with:
script: |
github.rest.issues.create({
owner: context.repo.owner,
repo: context.repo.repo,
title: 'Test failed on Node.js ${{ matrix.node-version }} (${{ steps.version.outputs.node-version }})',
body: 'The test failed on Node.js ${{ steps.version.outputs.node-version }}, [possibly due to a new version being promoted to node@current](https://github.com/nodejs/release#release-schedule). See [Action run output](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}) for more information.'
})