fastify-multipart Migrate from tap to Node Test Runner

Hi,

The goal of this PR is migrate the tests from Tap to Node test runner, following what's been said in the issue https://github.com/fastify/fastify/issues/5555

Checklist

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

Mar 16 '25 16:03 matteo-gobbo

Thanks for the PR! .taprc also needs removing as part of this (but kept in .gitignore).

Mar 16 '25 18:03 Fdawgs

Could you run npm run lint:fix? it should fix the CI

Apr 06 '25 07:04 Eomm

The issue here is using const tmpdir = fs.mkdtempSync(path.join(os.tmpdir(), '')) instead of the previous t.testDir() that was implemented directly inside tap. Do you have any suggestions about this? Perhaps there are other projects where a similar approach has been implemented that I could use as a reference?"

Thank you in advance for your answer.

Apr 06 '25 13:04 matteo-gobbo

The issue here is using const tmpdir = fs.mkdtempSync(path.join(os.tmpdir(), '')) instead of the previous t.testDir() that was implemented directly inside tap. Do you have any suggestions about this? Perhaps there are other projects where a similar approach has been implemented that I could use as a reference?"

Thank you in advance for your answer.

If i check the testdir implementation I think it was using a path in the project's folder (such as .tap/fixtures). So I think we may not create the ostmp by a random folder in a similar way (that we must add to the gitignore)

Apr 13 '25 08:04 Eomm

@matteo-gobbo are you planning to complete this?

Aug 19 '25 16:08 ilteoood

@matteo-gobbo are you planning to complete this?

Hi, yes sure, sorry for the delay. I’ll work on this by the end of the week. It's not fully clear to me how to handle the temp directory that Tap was using before. Do you know if any of the other packages have implemented something similar?

Aug 20 '25 07:08 matteo-gobbo

What suggested by @Eomm is a good starting point. At the moment there's no other project that has been migrated that used testdir. But if you check your inner implementation as suggested, it doesn't anything special. You just need to point to the right path. Let me know if you need additional help, I'll try to provide additional support if you can't figure it out.

Aug 20 '25 07:08 ilteoood

What suggested by @Eomm is a good starting point. At the moment there's no other project that has been migrated that used testdir. But if you check your inner implementation as suggested, it doesn't anything special. You just need to point to the right path.

Let me know if you need additional help, I'll try to provide additional support if you can't figure it out.

I’m actually out for the next couple of days, but I’ll give it a try as soon as I’m back. Thanks for your help!

Aug 20 '25 11:08 matteo-gobbo

Hi @gurgunday, thanks for your suggestions! I've replaced deepEqual with deepStrictEqual also in multipart-attach-body.test.js.

Aug 25 '25 20:08 matteo-gobbo

Hi @is2ei, I've update the error assertion to use ifError. I also updated the test should throw fileSize limitation error on small payload which had been skipped before my implementation and marked with a FIXME. My misuse of t.assert.ok made it seem to work, but it wasn’t correct. I followed the same approach used in another test, please check if this looks good to you.

Aug 27 '25 20:08 matteo-gobbo

Can you rebase and fix conflicts?

Sep 03 '25 20:09 gurgunday

In this PR the c8 package was added to the dependencies instead of just dev dependencies. I think it would be enough to add it to the dev dependencies as it is just used in a script. What do you think?