chore(test): Migrate test suites from mocha to node test runner

[Zen-cronic]

Resolves #157 & #158

Changes

• Replaced mocha with node --test script
• Replaces expect from chai with node:assert

[Zen-cronic]

The CI tests are ran on node-version: [14.x, 16.x, 18.x, 20.x, 22.x]. But node test runner was added in v16.17.0. So would support for node 14 be dropped?

[evert]

Nice! This is awesome. I think it's fine to finally drop Node 14. It's quite old at this point.

[Zen-cronic]

Interestingly, the tests are not run or only run partially, depending on the node version.

• v16.20.2: no tests run and hanging.
• v18.20.4: only the first (alphabetically) test file is run authorization-code.ts and left hanging.
• v20.17.0: same as v18
• v22.8.0: same as v18

*Tested locally and as observed in CI

The test runner doesn't support running .ts files right out of the box - even more workarounds depending on cjs or esm. Check this issue.

[evert]

Good catch! I recently migrated a different library to npm --test and this is how I've solved that:

https://github.com/badgateway/structured-headers/pull/58/files

Basically I:

• Installed tsx (and ditched ts-node, which doesn't work well with ESM anyway).
• Instead of running node --test, run tsx --test test/**/*.ts

This mostly solved all my problems. tsx is effectively a wrapper around node that sets up parsing Typescript transparently.

[Zen-cronic]

Oh I see. Will try it!

[Zen-cronic]

Interestingly, the tests are not run or only run partially, depending on the node version.

The tests were passing but the process was hanging. It seems that node:test isn't as dev-ready as other mature test frameworks. All connections/promises had to be closed explicitly like this. Whereas with mocha, the process ended as expected after the tests pass/fail.

Currently, the tests in fetch-wrapper.ts will all pass but the process will hang, so the next test file isn't continued. More specifically, the first two tests in that file are causing the hang - verified by attaching only and running locally. Will investigate further.

(So ts-node wasn't the issue, but switched to tsx for esm like you mentioned)

[evert]

The tests were passing but the process was hanging. It seems that node:test isn't as dev-ready as other mature test frameworks. All connections/promises had to be closed explicitly like this. Whereas with mocha, the process ended as expected after the tests pass/fail.

I'd imagine that this is a deliberate choice by Node, to keep things somewhat simple and explicit, but yeah i bet it was annoying to deal with.

Currently, the tests in fetch-wrapper.ts will all pass but the process will hang, so the next test file isn't continued. More specifically, the first two tests in that file are causing the hang - verified by attaching only and running locally. Will investigate further.

I'm almost certain this is because fetch-wrapper calls setTimeout which keeps an event open.

One idea is to explicitly disable this, or call clearTimeout in the cleanup function for this. (you may need to cast that object to any first to ignore the fact that it's a private function).

There's definitely more hoops here than I expected, so I understand if you're out of patience. If so I'll definitely build on your work to complete this. Either way I really appreciate the work so far!

[Zen-cronic]

I'd imagine that this is a deliberate choice by Node, to keep things somewhat simple and explicit, but yeah i bet it was annoying to deal with.

I'm almost certain this is because fetch-wrapper calls setTimeout which keeps an event open.

One idea is to explicitly disable this, or call clearTimeout in the cleanup function for this. (you may need to cast that object to any first to ignore the fact that it's a private function).

Gotcha, I added a fix. Feel free to adjust for any improvement.

[evert]

Very frustrating! If I'm reading the errors right, we need tsx 3 for Node =< 16, and tsx 4 for Node > 20

[evert]

Given that dropping older Node versions is already a major change, maybe it's time to also drop Node 16, and make this PR of a future 3.0 release.