Skip to content

test: migrate tap suite to node:test#869

Open
mcollina wants to merge 2 commits intomainfrom
chore/migrate-tap-to-node-test
Open

test: migrate tap suite to node:test#869
mcollina wants to merge 2 commits intomainfrom
chore/migrate-tap-to-node-test

Conversation

@mcollina
Copy link
Member

@mcollina mcollina commented Mar 2, 2026

Summary

  • migrate remaining Tap-based tests to node:test
  • replace Tap assertions in test/start.test.js via a local compat wrapper
  • migrate test/graceful-shutdown.test.js to node:test
  • remove Tap usage from examples/plugin-with-preloaded.js
  • switch npm test scripts from Tap CLI to suite-runner.js
  • remove tap and @types/tap from devDependencies

Validation

  • node --test test/start.test.js
  • node --test test/graceful-shutdown.test.js

Copy link
Member

@climba03003 climba03003 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using const { test: nodeTest } = require('node:test') and keep the test() in all files
More consistence usage and easier to migrate if we plan to use another test framework.


const t = require('tap')
const { once } = require('node:events')
const { test, beforeEach, afterEach } = require('node:test')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const { test, beforeEach, afterEach } = require('node:test')
const { test: nodeTest, beforeEach, afterEach } = require('node:test')

const assert = require('node:assert/strict')
// Tests skip on win32 platforms due SIGINT signal is not supported across all windows platforms
const test = (process.platform === 'win32') ? t.skip : t.test
const testFn = (process.platform === 'win32') ? (name, fn) => test(name, { skip: true }, fn) : test
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const testFn = (process.platform === 'win32') ? (name, fn) => test(name, { skip: true }, fn) : test
const test = (process.platform === 'win32') ? (name, fn) => test(name, { skip: true }, fn) : test

t.plan(2)

t.equal(process.listenerCount('SIGINT'), signalCounter + 1)
testFn('should add and remove SIGINT listener as expected ', async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
testFn('should add and remove SIGINT listener as expected ', async () => {
test('should add and remove SIGINT listener as expected ', async () => {

sinon.assert.called(spy)

t.end()
testFn('should have called fastify.close() when receives a SIGINT signal', async (t) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
testFn('should have called fastify.close() when receives a SIGINT signal', async (t) => {
test('should have called fastify.close() when receives a SIGINT signal', async (t) => {

@mcollina
Copy link
Member Author

mcollina commented Mar 5, 2026

Addressed review feedback:

  • switched to const { test: nodeTest } = require('node:test') in test/graceful-shutdown.test.js
  • kept test(...) usage consistently by defining const test = ... ? ... : nodeTest

Validation:

  • node --test test/graceful-shutdown.test.js (pass)

Copy link
Member

@climba03003 climba03003 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants