Upgrade tap to v18

[bizob2828]

Description

We're currently using v16 of tap. v18 is a significant rewrite. I took a look at what it would take and we have to replace t.autoend() with t.end() at the end of every suite. Also adding custom assertions is no longer via addAssert.

Acceptance Criteria

node-newrelic tests are run with tap 18.

Additional context

tap 18 requires you to write plugins for any custom work. It's very heavy handed but in the near term to add custom assertions we're just adding them on tap.Test.prototype

[workato-integration]

https://new-relic.atlassian.net/browse/NR-218084

[jsumners-nr]

@bizob2828 what are your thoughts on not attempting to add custom assertions and instead use utility functions, e.g.?

function validateFoo(t, foo) {
  t.equal(foo, 'foo')
}

tap.test('foo', async t => {
  validateFoo(t, 'foo')
})

[bizob2828]

We could. That's how most of these once were but opted for custom assertions.

[jsumners-nr]

I think doing so would solve a couple problems:

1. If the test suite's API changes again in the future, we don't get surprised by it.
2. If for some reason we decide to change test suites then we are loosely coupled and ready to make the switch.
3. Less mystery in what is available. I find custom assertions to be non-discoverable and typically only known about through rumor and conjecture.

[bizob2828]

I'll prob do this in a separate PR as the changes for 18 are a lot

[bizob2828]

If for some reason we decide to change test suites then we are loosely coupled and ready to make the switch.

We still have tap assertions within the helper so not sure how loosely coupled we are

[bizob2828]

I'm putting into the backlog for now.

Branch

I have one major blocker which is detailed here.

Aside from that we have a failing integration test test/integration/core/exceptions.tap.js, the IPC communication between the test and the test harness in a child process is emitting too many of the same events. I think this is because tap is hooking into process.emit. Lastly, we may not be able to use tap 18 because it's so much slower than 16.

[isaacs]

Less mystery in what is available. I find custom assertions to be non-discoverable and typically only known about through rumor and conjecture.

For what it's worth, while it is a bit more "heavy", the plugin approach in tap v18 is specifically designed to make the system type-aware, so that autocomplete can surface any functionality added by plugins for better discovery (assuming that the plugin exports types, of course.)

we may not be able to use tap 18 because it's so much slower than 16.

I'm curious about this. What is slower? How much is "so much"?

[bizob2828]

Less mystery in what is available. I find custom assertions to be non-discoverable and typically only known about through rumor and conjecture.

For what it's worth, while it is a bit more "heavy", the plugin approach in tap v18 is specifically designed to make the system type-aware, so that autocomplete can surface any functionality added by plugins for better discovery (assuming that the plugin exports types, of course.)

@isaacs I find plugins for internal custom assertions where we don't use typescript/types to be very heavy handed. It also seems to require the use of a monorepo, or separate published modules for the plugins. Maybe I'm not grasping the concept but it wan't clear to me.

we may not be able to use tap 18 because it's so much slower than 16.

I'm curious about this. What is slower? How much is "so much"?

I'm not sure if it's the reporter or what but we used to rely on the classic reporter and you can see unit test runs in tap 16 were finishing in 1 min. With tap 18 it's taking almost 4 minutes. This is a deal breaker for us and I don't know where to begin to give you enough information to log an issue, any suggestions?

[jsumners-nr]

For what it's worth, while it is a bit more "heavy", the plugin approach in tap v18 is specifically designed to make the system type-aware, so that autocomplete can surface any functionality added by plugins for better discovery (assuming that the plugin exports types, of course.)

For clarity, I'm not at all concerned with autocomplete functionality. My concern is encountering methods on the library that I don't expect and having to figure out how they got defined. One can argue that the path to learning where the method is defined and how it works is the same and learning where a function is defined, but I personally find it easier to reason through standalone function than one that probably relies on internals of another thing I don't know.

[isaacs]

@isaacs I find plugins for internal custom assertions where we don't use typescript/types to be very heavy handed. It also seems to require the use of a monorepo, or separate published modules for the plugins. Maybe I'm not grasping the concept but it wan't clear to me.

Yes, plugins are dependencies. Depending on your view of how "heavy" a thing must be to justify publishing to npm (which can be quite a low bar indeed), that can be either heavy or light.

I'm not sure if it's the reporter or what but we used to rely on the classic reporter and you can see unit test runs in tap 16 were finishing in 1 min. With tap 18 it's taking almost 4 minutes. This is a deal breaker for us and I don't know where to begin to give you enough information to log an issue, any suggestions?

Yeah, 4 minutes is a lot. Since you're not using typescript, I'd maybe see if tap plugin rm @tapjs/typescript or tap config set typecheck=false makes it go faster. Ts-node can be pretty slow, especially if you are using typescript with allowJs: true in tsconfig. This will be improved somewhat once I finish combing through the nest of edge cases to get tsimp stable enough for prime time.

For clarity, I'm not at all concerned with autocomplete functionality.

Ah, a fellow old, I see ;) Yeah, kids these days don't read docs or source anymore. They just press tab in their editor and expect it to be correct. When that fails, ask copilot or chatgpt. (All jokes aside, "types as inline docs" is actually a pretty nice pattern, I've found myself leaning on it more and more, and appreciating when a library has accurate types included, for that reason.)

[bizob2828]

@isaacs I tried tap plugin rm @tapjs/typescript and it didn't affect the runtime of unit tests

In CI:

tap 16 unit tests - ~1 min 16s
tap 18 unit tests stock - ~3m 30s
tap 18 unit tests with typescript plugin removed - ~3m 30s

[isaacs]

@bizob2828 That's very bizarre. Can you reproduce it locally? Is it doing a build before running the tests, or are the tests themselves slower? Maybe it's defaulting to a lower concurrency value?

[bizob2828]

@bizob2828 That's very bizarre. Can you reproduce it locally? Is it doing a build before running the tests, or are the tests themselves slower? Maybe it's defaulting to a lower concurrency value?

No building. We are using c8 for coverage and disabling tap c8. Reason is we have to pass in an option to c8 and couldn't figure out how to do that through tap.

Dumping config between 16 and 18 it has jobs set to 8 so concurrency is the same. A quick check for you would be to clone this repo and just run

npm ci
npm run unit

That's with tap 16. Then with tap 18 it'd be to checkout this branch in fork and run the same 2 commands