tests(ci): run unit tests on all active node versions #12513
Conversation
| runs-on: ubuntu-latest | ||
| name: node ${{ matrix.node }} | ||
| env: | ||
| LATEST_NODE: '16' |
There was a problem hiding this comment.
AFAICT it's not possible to refer to the matrix.node full array in steps nor env variables inside matrix (even with fancy fromJSON stuff), so this was the best I could do to make a way to keep the conditionals on latest node easy to keep up to date.
| unit: | ||
| strategy: | ||
| matrix: | ||
| node: ['12', '14', '16'] |
There was a problem hiding this comment.
should we just run on ['12', '16']? Not sure if there's a lot of downside to running on all three besides more jobs queued up and more chances for flakes
| - run: sudo apt-get install xvfb | ||
|
|
||
| - name: yarn unit | ||
| if: ${{ matrix.node != env.LATEST_NODE }} |
There was a problem hiding this comment.
only running coverage on latest node means better coverage (since it'll have the latest v8 coverage updates) and saves ~3 minutes on the unit run for those not doing coverage
| flags: unit | ||
| file: ./unit-coverage.lcov | ||
|
|
||
| # For windows, just test the potentially platform-specific code. |
There was a problem hiding this comment.
the existing unit-windows is different enough from the full unit jobs that it's worth just keeping it separate instead of another dimension in the matrix
| @@ -19,6 +19,7 @@ describe('locales', () => { | |||
| in: 'id', | |||
| iw: 'he', | |||
| mo: 'ro', | |||
| tl: 'fil', | |||
There was a problem hiding this comment.
in node 16, the updated ICU version gives 'fil' for Intl.getCanonicalLocales('tl') (as anticipated), so need to add it here.
|
Should we retire the 馃挕馃彔 now? |
We're already breaking the required checks list forcing a rebase when this lands, so I'd say if we're going to do it let's do it now :) |
the workflow name surprisingly doesn't matter for selecting the required status checks, but I can also change 馃挕馃彔 now anyways :) |
|
ok, we settled on |
As we continue to support the wide LTS -> current Node version spread, we need to make sure we aren't broken on latest while still maintaining support for Node 12 (e.g. ICU support differs significantly between Node 12 and 13+ as in #12426, nolanlawson/marky#18 had
lighthouse-loggerbroken in Node 16, etc). We could maybe consider smoke test support across versions in the future, but running the unit tests across versions was an easy and decent first step.The
unitci jobs were getting unwieldy enough that it was worth splitting it out into another workflow file 馃し