test: #5555 - move to node test runner

[SamSalvatico]

Checklist

Moving the tests from tap to the node test runner, based on what requested in the following issue
Fastify #5555

• 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

[SamSalvatico]

I did not move all the tests yet, only the ones under commonjs and typescript esm
Just to understand if you like how I’ve moved the tests and to avoid modifying too many files in the same PR, I’ll make the next changes in a subsequent PR

[jean-michelet]

Thanks for your PR.

I’ll make the next changes in a subsequent PR

This plugin is a bit tricky regarding testing, we might discover later then the changes are difficult to apply.
So imho, this PR should demonstrate if we can do it or not.
Plus, refactoring tests is generally very risky, as we don't have feedback about how the changes have affected the project so we might need some more reviews.

[SamSalvatico]

Thanks for your PR.

I’ll make the next changes in a subsequent PR

This plugin is a bit tricky regarding testing, we might discover later then the changes are difficult to apply. So imho, this PR should demonstrate if we can do it or not. Plus, refactoring tests is generally very risky, as we don't have feedback about how the changes have affected the project so we might need some more reviews.

Got it! So would you prefer that I try to update all the tests in this PR?

[jean-michelet]

So would you prefer that I try to update all the tests in this PR

I think it's a good idea, you might want some more reviews before going forward.

[jean-michelet]

@climba03003 @Fdawgs wdyt?

[dancastillo]

since t.assert.plan was not added until node v22 wondering if we should use tspl or something similar, wdyt? @mcollina @gurgunday

[climba03003]

I think it's a good idea, you might want some more reviews before going forward.

I believe we can do incremental one, but it should be green before land.

since t.assert.plan was not added until node v22 wondering if we should use tspl or something similar, wdyt?

If you were mentioning t.plan, it exists since 20.15.0.

[SamSalvatico]

I should have changed all the tests from tap to node test runner (at least searching for tap there are no occurences 😁 )

[SamSalvatico]

I am having some problems running tests using patterns, because windows is not managing them automatically
Before, when using tap, I think it was using the glob on which the library depends

I tried using the glob library by myself, but it failed.
The last idea I have is trying with fast-glob, but if it doesn't work I have to ask: do you have any clue about this?

[jean-michelet]

I tried using the glob library by myself, but it failed.

The link you are sharing is not a failure, this test suite has been cancelled because the Node 20 on latest MacOS failed: https://github.com/fastify/fastify-autoload/actions/runs/12870529340/job/35882162556

"Exceeded timeout of 30000 ms for a test.

This is a timeout failure we had for a long time with Jest, I am not even sure we fixed it since.

[jsumners]

See fastify/avvio#302

[jean-michelet]

I will perform a more precise review during the day before accepting, but looks good.

[mcollina]

lgtm

[jean-michelet]

Could be a good idea to use a LLM to check this diff, to be sure we didn't miss anything.

[jean-michelet]

@jsumners

Curious about the laugh emoji, I had a good experience with LLM catching omissions while refactoring.
Pretty bad at understanding complex context, but this case appear legit to me.

[jsumners]

@jsumners

Curious about the laugh emoji, I had a good experience with LLM catching omissions while refactoring. Pretty bad at understanding complex context, but this case appear legit to me.

I have zero confidence those things will provide anything useful, particularly in this context.

[jean-michelet]

I have zero confidence those things will provide anything useful

Strong opinion if you think this about programming more broadly (I do not suggest using it for Fastify projects in general).

@climba03003
Do you think we can merge?

[jean-michelet]

My bad, forget my previous comment.

[jean-michelet]

We should use borp here @jsumners?

[jsumners]

This project already has a scripts/ directory and uses them for running tests. I assume it's due to the way the module works. I don't really know; I don't pay much attention to this module. So I'm not overly concerned about using borp here. If it simplifies things, go for it.

[SamSalvatico]

I moved the tests to borp as done in avvio
I used the same patterns as used in the master branch with tap

[jean-michelet]

I am worry the coverage is not checked anymore, can you please try with -C --check-coverage on your machine? What happen if you remove one of these tests?