Test Ruby Sass on Ruby 3.1, Dart Sass on Node.js 18

[colinrotherham]

This PR bumps our Sass test runtime versions:

1. Ruby 3.1 (from 2.6) for the Ruby Sass tests
2. Node.js 18 (from 14) for the Dart Sass tests

Also includes:

• Step names for all GitHub workflow steps
• Fix for triple-spaced concurrency: GitHub workflow config

Note: Ruby 2.6 support ended 31 March 2022

Previous changes

During our switch from Node.js 16 to Node.js 18, this PR suggested a GitHub Actions matrix strategy

matrix:
  package-version:
    - 8.x # Node Sass v8.x = LibSass v3.x latest

  node-version:
    - 14
    - 16
    - 18

We've dropped Node.js 16 support (and other LTS releases) so this is no longer necessary

[colinrotherham]

One side effect is that the Node.js (or Ruby) version is included in the status check name

[36degrees]

Repeating my comment from an earlier discussion:

I don't think we need to care about multiple Node or Ruby versions here – the Sass tests exist to give us confidence that our Sass compiles when used with the oldest and newest versions of the compilers we say that we support.

We should just test each compiler running in the latest version of Node / Ruby that is supported by the compiler.

[colinrotherham]

@36degrees Yeah that's the bit I'm aiming to tackle

Do we have documentation on what we consider "Deprecated, but don't break them"?

If we're dropping Node.js 14 and 16 (but keeping 18) would we not want to drop some of these too?

• Node.js 4
• Node.js 8
• Ruby 2.1.9
• Ruby 2.6

But if not, would it be consistent to follow our Node.js approach and upgrade Ruby 2.6 to (latest) 3.1?

(Ruby 2.6 support ended 31 March 2022)

[colinrotherham]

Narrowing down to "latest" only but keeping the special deprecated versions gives us:

[36degrees]

If we're dropping Node.js 14 and 16 (but keeping 18) would we not want to drop some of these too?

• Node.js 4
• Node.js 8
• Ruby 2.1.9
• Ruby 2.6

But if not, would it be consistent to follow our Node.js approach and upgrade Ruby 2.6 to (latest) 3.1?

(Ruby 2.6 support ended 31 March 2022)

We say in our install docs that GOV.UK Frontend should work with:

• Dart Sass v1.0.0 +
• LibSass v3.3.0+
• Ruby Sass v3.4.0+

These tests are trying to ensure that we don't e.g. accidentally start using a feature that was introduced in LibSass v3.5.0 (for example) without realising, as that would be a breaking change.

So we test with the newest and oldest versions of each compiler. We don't really care about Node / Ruby versions here, other than the fact that we need to pick one that will work for the version of the compiler we're trying to test with.

There's also no link between the Node versions we use for our tooling / tests and the Node versions we're using to run the different versions of the compilers.

And we don't need to test each compiler against multiple Node versions, because whether e.g. LibSass v3.3.0 works with Node v14 (purely as an example) or not is just not something that we need to care about.

I think generally we've tried to use the latest version of Node that each compiler supports, but from memory for Ruby Sass I looked at the release dates for Ruby Sass and matched to the latest Ruby version that was available at that time, as I couldn't find explicit version support documented anywhere.

[colinrotherham]

Thanks that makes sense

I'll keep the Ruby 3.1 upgrade so we can always track "latest" (should have 3.2 next month too)

[36degrees]

If we’re not expanding to run against multiple Node versions, do you still think there’s value in running it as a matrix?

Although it’s arguably DRYer it ends up being more lines of code than the original, and personally I find it quite a lot harder to follow what’s going on.

[colinrotherham]

That's absolutely fine, it's what code reviews are for. Can you point out which bits are harder to follow?

Appreciate I've added comments (and named all the steps!) which has made it longer too

Checkout code
Setup Node.js
Install package

I'd love to bring back the multiple Node.js and Ruby versions too but see what the others think

[romaricpascal]

I'd agree the DRYness makes things tougher to follow. You need to jump from the step, up to the matrix, remember that include will override those (if I understood correctly 😬) and for dartsass make the command a bit of a mouthful to read.

In the end that's only 6 tests, so I think we can live with a little duplication, sorry. Especially as it makes reading the workflow more easily.

Regarding testing multiple versions of Node, I'd say it's not our responsibility to check that each tool work with this or that version of node, that's each tool's job. If a tool documents a quirk at some point, it could be worth expanding our testing though.

[colinrotherham]

@36degrees @romaricpascal That's more than fair, appreciate the review

Removing the matrix bits, all that leaves then is:

1. Step names and comments
2. Node.js 18 upgrade for Dart Sass (latest)
3. Ruby 3.1 upgrade for Ruby Sass (latest, deprecated)

Do you want to keep any of those or shall we close this?

[romaricpascal]

Naming the actions and upgrading doesn't hurt imho, so happy to merge these bits 😄

[colinrotherham]

Thanks @romaricpascal

The step names show up in the job summary view too:

Before jobs/6044033396
After jobs/6029398829