ci: Add Nsolid runtime to CI and Integration tests

[riosje]

Propose N|Solid adoption

This pull request proposes the adoption of the N|Solid runtime, a fully compatible Node.js runtime developed by NodeSource.

Implementation

Details of the implementation:

• I have ensured that all existing tests pass when the Fastify server is running on the N|Solid runtime.
• Necessary adjustments have been made to CI and Integration pipelines to accommodate the new runtime and possible new upcoming runtimes..

Thank you for considering this addition to the Fastify project.

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[mcollina]

Can you please amend the LTS document accordingly?

[riosje]

Can you please amend the LTS document accordingly?

Hi @mcollina, thanks for taking a look to this proposal.
I've Updated the LTS document as requested, I'm not 100% sure if it has the expected format, I just tried to make it fit.

[mcollina]

@RafaelGSS are you ok in supporting this change in case there are issues? I don't expect there will be but better safe than sorry.

[RafaelGSS]

@RafaelGSS are you ok in supporting this change in case there are issues? I don't expect there will be but better safe than sorry.

Yes, happy to support it.

[climba03003]

If we add some runtime guarantee that is not node, the question would be why we need to against the support of deno or bun?

If N|Solid is bundling Node.js, why it need to be tested seperately?

[gurgunday]

If we add some runtime guarantee that is not node, the question would be why we need to against the support of deno or bun?

If N|Solid is bundling Node.js, why it need to be tested seperately?

I'd +1 the question, because it's a valid concern

[mcollina]

If we add some runtime guarantee that is not node, the question would be why we need to against the support of deno or bun?

My generic take on this is that we can support them if our tests run unmodified.

[jsumners]

I am not opposed to adding the extra runtime check (though I do wonder why this particular one is necessary given it is a direct rebuild of core). My only concern is regarding the implementation. This PR introduces a lot of complicated configuration to appease the fact that the runtime doesn't support all of the core versions we currently support. I'd rather see a completely separate "alternative runtimes" set of workflows added. Or maybe make the jobs reusable and define new matrix strategies utilizing them.

[Eomm]

My generic take on this is that we can support them if our tests run unmodified.

We should write it down somewhere in case.

Anyway my idea is:

do we block a PR if the NSolid CI does not work?

Assuming the answer is "no" I would move this run by:

1. set it as optional by setting continue-on-error

Or:

1. I would move it to a separate workflow
2. I would run the workflow on merge on main (as we do for the package managers: https://github.com/fastify/fastify/blob/main/.github/workflows/package-manager-ci.yml)
3. (idea/suggestion) Add a notification to someone (maybe by opening an issue here or elsewhere)

[RafaelGSS]

My generic take on this is that we can support them if our tests run unmodified.

We should write it down somewhere in case.

Anyway my idea is:

do we block a PR if the NSolid CI does not work?

Assuming the answer is "no" I would move this run by:

1. set it as optional by setting continue-on-error

Or:

1. I would move it to a separate workflow
2. I would run the workflow on merge on main (as we do for the package managers: https://github.com/fastify/fastify/blob/main/.github/workflows/package-manager-ci.yml)
3. (idea/suggestion) Add a notification to someone (maybe by opening an issue here or elsewhere)

I think creating a new workflow with continue-on-error should be the most appropriate as it won't require any change in the current setup and any errors in nsolid CI can be easily mitigated by us (nodesource).

[riosje]

I think creating a new workflow with continue-on-error should be the most appropriate as it won't require any change in the current setup and any errors in nsolid CI can be easily mitigated by us (nodesource).

Hi @RafaelGSS what you think if we use the same workflow and put the continue-on-erroronly for nsolid, or any other runtime if it is explicitly set.

something like this:

    continue-on-error: ${{ matrix.continue-on-error || false }}
    strategy:
      matrix:
        runtime: [node, nsolid]
        node-version: [14, 16, 18, 20]
        os: [macos-latest, ubuntu-latest, windows-latest]
        include:
          - runtime: nsolid
            nsolid-version: 5
            continue-on-error: true
        exclude:
        # excludes node 14 on Windows
          - os: windows-latest
            runtime: node
            node-version: 14
        # Nsolid 5 does not support Node.js 14, 16
          - runtime: nsolid
            node-version: 14
          - runtime: nsolid
            node-version: 16

---

I am not opposed to adding the extra runtime check (though I do wonder why this particular one is necessary given it is a direct rebuild of core). My only concern is regarding the implementation. This PR introduces a lot of complicated configuration to appease the fact that the runtime doesn't support all of the core versions we currently support. I'd rather see a completely separate "alternative runtimes" set of workflows added. Or maybe make the jobs reusable and define new matrix strategies utilizing them.

hi @jsumners, thanks for joining to the discussion, I really appreciate it.
I understand you think the PR introduces a lot of complicated configuration because of the exclusions added for Node14 and Node16, is that right?
N|solid has been releasing versions since Node4(argon) and I can adjust the pipeline to run with N|solid4(Node 14 - Node16) so we can remove those exclusions making it 'compatible' with the supported core version.

[RafaelGSS]

We only need the test job for nsolid runtime. Lint, coverage are already covered by the original ci.yml

[jsumners]

I'd rather see a separate workflow that is dedicated to alternative runtimes.

[riosje]

I'd rather see a separate workflow that is dedicated to alternative runtimes.

It makes sense, the current approach is capable to receive new runtime contributions just adding the necessary includes/excludes in the matrix config and the conditional step to setup the proposed runtime.