Skip to content

Replace tap with the built-in node:test runner#58

Merged
szegedi merged 2 commits into
mainfrom
migrate-tap-to-node-test
Jun 9, 2026
Merged

Replace tap with the built-in node:test runner#58
szegedi merged 2 commits into
mainfrom
migrate-tap-to-node-test

Conversation

@szegedi

@szegedi szegedi commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Why

The recent tap bump (#49) surfaced that tap — via @tapjs/processinfo — pulls in uuid@14, whose node build references the global crypto. That global is unavailable unflagged on Node 18 and is unreliable inside processinfo's ESM loader hook, which broke CI. Rather than work around a heavyweight test-runner dependency, this drops tap entirely in favor of Node's built-in test runner.

What

  • Rewrite src/index.test.ts against node:test + node:assert. tap's loose assertions map directly:

    • t.equalassert.strictEqual
    • t.sameassert.deepEqual (loose — needed for the protobufjs-compat test, which compares bigint/number against the string longs that toObject({longs: String}) produces)
    • t.has → a small recursive loose-subset helper (assertHas)

    The constructs/encodes/decodes tap plugin became plain functions taking the test context.

  • Compile tests with tsc (tsconfig.test.json) to dist-test/ and run them with node --test, so they work on every supported Node version — including 18, which can't strip types natively.

  • Fix testing/proto/package.json {"module":"commonjs"}{"type":"commonjs"} so the generated CJS protobuf module resolves under Node's native ESM loader. (The old value was a no-op that only worked because tap's loader bypassed native resolution.)

  • Drop the tap dependency and its config block.

  • Re-add Node 18 and add Node 26 to the test matrix (now [18, 20, 22, 24, 26]).

Verification

npm test and npm run lint pass locally. The compiled tests were run on Node 18, 20, 22, 24, and 2672/72 passing on every version. A negative check (corrupting one expected encoding) fails correctly with exit 1.

🤖 Generated with Claude Code

@szegedi szegedi requested review from a team as code owners June 9, 2026 08:32
@szegedi szegedi added the semver-patch Bug and security fixes label Jun 9, 2026
IlyasShabi
IlyasShabi previously approved these changes Jun 9, 2026
@datadog-prod-us1-5

This comment has been minimized.

tap (via @tapjs/processinfo) pulled in uuid@14, whose node build
references the global `crypto`; that is unavailable unflagged on Node 18
and unreliable in processinfo's loader hook, which broke CI. Drop the tap
dependency entirely in favor of Node's built-in test runner.

- Rewrite src/index.test.ts against node:test + node:assert. tap's loose
  assertions are mapped as: equal -> strictEqual, same -> deepEqual
  (loose), and has -> a small recursive loose-subset helper (assertHas).
- Compile the tests with tsc (tsconfig.test.json) to dist-test/ and run
  them with `node --test`, so they work on every supported Node version
  including 18 (which cannot strip types natively).
- Fix testing/proto/package.json to {"type":"commonjs"} so the generated
  CJS protobuf module resolves under Node's native ESM loader.
- Re-add Node 18 and add Node 26 to the test matrix.

Verified 72/72 passing on Node 18, 20, 22, 24, and 26.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@szegedi szegedi force-pushed the migrate-tap-to-node-test branch from 9e4977b to 0de153e Compare June 9, 2026 13:22
@szegedi szegedi requested a review from IlyasShabi June 9, 2026 13:26
@szegedi szegedi enabled auto-merge (squash) June 9, 2026 13:46
@szegedi szegedi merged commit 08f453c into main Jun 9, 2026
13 checks passed
@szegedi szegedi deleted the migrate-tap-to-node-test branch June 9, 2026 14:18
szegedi added a commit that referenced this pull request Jun 10, 2026
PR #45 added these tests after PR #58 switched from tap to node:test,
using the old tap.test/t.same style. Convert to test/assert.deepEqual.

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
szegedi added a commit that referenced this pull request Jun 10, 2026
tap (via @tapjs/processinfo) pulled in uuid@14, whose node build
references the global `crypto`; that is unavailable unflagged on Node 18
and unreliable in processinfo's loader hook, which broke CI. Drop the tap
dependency entirely in favor of Node's built-in test runner.

- Rewrite src/index.test.ts against node:test + node:assert. tap's loose
  assertions are mapped as: equal -> strictEqual, same -> deepEqual
  (loose), and has -> a small recursive loose-subset helper (assertHas).
- Compile the tests with tsc (tsconfig.test.json) to dist-test/ and run
  them with `node --test`, so they work on every supported Node version
  including 18 (which cannot strip types natively).
- Fix testing/proto/package.json to {"type":"commonjs"} so the generated
  CJS protobuf module resolves under Node's native ESM loader.
- Re-add Node 18 and add Node 26 to the test matrix.

Verified 72/72 passing on Node 18, 20, 22, 24, and 26.
szegedi added a commit that referenced this pull request Jun 10, 2026
PR #45 added these tests after PR #58 switched from tap to node:test,
using the old tap.test/t.same style. Convert to test/assert.deepEqual.

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
@szegedi szegedi mentioned this pull request Jun 10, 2026
szegedi added a commit that referenced this pull request Jun 16, 2026
tap (via @tapjs/processinfo) pulled in uuid@14, whose node build
references the global `crypto`; that is unavailable unflagged on Node 18
and unreliable in processinfo's loader hook, which broke CI. Drop the tap
dependency entirely in favor of Node's built-in test runner.

- Rewrite src/index.test.ts against node:test + node:assert. tap's loose
  assertions are mapped as: equal -> strictEqual, same -> deepEqual
  (loose), and has -> a small recursive loose-subset helper (assertHas).
- Compile the tests with tsc (tsconfig.test.json) to dist-test/ and run
  them with `node --test`, so they work on every supported Node version
  including 18 (which cannot strip types natively).
- Fix testing/proto/package.json to {"type":"commonjs"} so the generated
  CJS protobuf module resolves under Node's native ESM loader.
- Re-add Node 18 and add Node 26 to the test matrix.

Verified 72/72 passing on Node 18, 20, 22, 24, and 26.
szegedi added a commit that referenced this pull request Jun 16, 2026
PR #45 added these tests after PR #58 switched from tap to node:test,
using the old tap.test/t.same style. Convert to test/assert.deepEqual.

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver-patch Bug and security fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants