Skip to content

feat(tape-to-node-test): introduce #296

Open
AugustinMauroy wants to merge 27 commits intomainfrom
tape-to-node-test
Open

feat(tape-to-node-test): introduce #296
AugustinMauroy wants to merge 27 commits intomainfrom
tape-to-node-test

Conversation

@AugustinMauroy
Copy link
Copy Markdown
Member

@AugustinMauroy AugustinMauroy commented Dec 10, 2025 •
edited
Loading

Related issue

Close #260

@AugustinMauroy AugustinMauroy marked this pull request as ready for review December 15, 2025 19:28
@AugustinMauroy AugustinMauroy requested review from a team and ljharb December 15, 2025 19:28
@AugustinMauroy AugustinMauroy added the awaiting reviewer Author has responded and needs action from the reviewer label Dec 15, 2025
brunocroh
brunocroh reviewed Dec 18, 2025
View reviewed changes
Comment thread recipes/tape-to-node-test/src/workflow.ts Outdated
Comment thread recipes/tape-to-node-test/src/workflow.ts
Comment thread recipes/tape-to-node-test/src/workflow.ts Outdated
Comment thread recipes/tape-to-node-test/src/workflow.ts
AugustinMauroy and others added 2 commits December 18, 2025 22:35
@AugustinMauroy @brunocroh
Apply suggestions from code review
5b9ce43 
Co-authored-by: Bruno Rodrigues <swe@brunocroh.com>
@AugustinMauroy
fix: code style
5e5d232
brunocroh
brunocroh reviewed Jan 6, 2026
View reviewed changes
Comment thread recipes/tape-to-node-test/src/workflow.ts
ljharb
ljharb reviewed Jan 7, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

(btw the diff would read cleaner if it was "output" instead of "expected", because then "input" would be sorted before "output" instead of the reverse)

some of my comments apply on all the input/expected combos, so i'll post this now before being exhaustive

Comment thread recipes/tape-to-node-test/tests/new-features/expected.js
Comment thread recipes/tape-to-node-test/tests/no-callback-args/expected.js Outdated
Comment thread recipes/tape-to-node-test/tests/require-import/expected.js Outdated
@brunocroh brunocroh added awaiting author Reviewer has requested something from the author and removed awaiting reviewer Author has responded and needs action from the reviewer labels Jan 9, 2026
@AugustinMauroy
Copy link
Copy Markdown
Member Author

AugustinMauroy commented Jan 17, 2026

(btw the diff would read cleaner if it was "output" instead of "expected", because then "input" would be sorted before "output" instead of the reverse)

some of my comments apply on all the input/expected combos, so i'll post this now before being exhaustive

the input/expect structure isn't chose by us it's the design of the codemod tool

@AugustinMauroy
Copy link
Copy Markdown
Member Author

AugustinMauroy commented Jan 17, 2026

@ljharb i had resolved all of your concerns

@AugustinMauroy AugustinMauroy added awaiting reviewer Author has responded and needs action from the reviewer and removed awaiting author Reviewer has requested something from the author labels Jan 17, 2026
ljharb
ljharb reviewed Jan 19, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

There's a big issue with converting to assert - namely, that tape test suites very intentionally do not terminate at a failed assertion, but node's assert throws (which prevents running the rest of the assertions).

Does the test runner have a non-throwing assertion API?

Comment thread recipes/tape-to-node-test/README.md Outdated
Comment thread recipes/tape-to-node-test/README.md Outdated
Comment thread recipes/tape-to-node-test/README.md
Comment thread recipes/tape-to-node-test/tests/aliased-import/expected.js Outdated
Comment thread recipes/tape-to-node-test/tests/advanced-assertions/expected.js Outdated
Comment thread recipes/tape-to-node-test/tests/cjs-destructuring/expected.js Outdated
Comment thread recipes/tape-to-node-test/tests/nested-test/expected.js Outdated
Comment thread recipes/tape-to-node-test/tests/new-features/expected.js
Comment thread recipes/tape-to-node-test/tests/plan-and-end/expected.js Outdated
Comment thread recipes/tape-to-node-test/tests/timeout-non-object/expected.js Outdated
const opts = { skip: false };

test('timeout with variable opts', opts, async (t) => {
// TODO: Add timeout: `123` to test options manually;
Copy link
Copy Markdown
Member

@ljharb ljharb Jan 19, 2026

Choose a reason for hiding this comment

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

does the test runner not have an imperative timeout API?

Copy link
Copy Markdown
Member Author

@AugustinMauroy AugustinMauroy Jan 21, 2026

Choose a reason for hiding this comment

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

I didn't found it

@AugustinMauroy
Copy link
Copy Markdown
Member Author

AugustinMauroy commented Jan 21, 2026

that's fine, but can the PR be marked draft in the meantime? a green PR is mergeable, and with this todo i don't think that's accurate.

i have updated the comment but the user have to found a way to change their code because I dunno if we have an equivalent

@ljharb
Copy link
Copy Markdown
Member

ljharb commented Jan 27, 2026

If there's no equivalent, then probably it doesn't make sense to provide a codemod until there is?

@AugustinMauroy
Copy link
Copy Markdown
Member Author

AugustinMauroy commented Feb 1, 2026

If there's no equivalent, then probably it doesn't make sense to provide a codemod until there is?

IMO we should warn it on readme and people will be aware and then they know that will change how does it work.
cc @JakobJingleheimer what do you think ?

avivkeller
avivkeller reviewed Feb 2, 2026
View reviewed changes
Comment thread recipes/tape-to-node-test/src/workflow.ts Outdated
Comment thread recipes/tape-to-node-test/src/workflow.ts Outdated
Comment on lines +290 to +292
console.warn(
`[Codemod] Warning: ${methodName} at ${fileName}:${line}:${column} has no direct equivalent in node:test. Please migrate manually.`,
);
Copy link
Copy Markdown
Member

@avivkeller avivkeller Feb 2, 2026

Choose a reason for hiding this comment

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

cc @nodejs/test_runner a onFailure (perhaps afterFail, keeping the naming similar) handler is a pretty neat feature for us to include.

Also, onFinish -> after, right?, or does after not run on the completion of the test it's created in?

Copy link
Copy Markdown
Member Author

@AugustinMauroy AugustinMauroy Apr 9, 2026

Choose a reason for hiding this comment

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

aviv can I invite you to open an issue on test runner repo

Comment thread recipes/tape-to-node-test/src/workflow.ts Outdated
Comment thread recipes/tape-to-node-test/src/workflow.ts
Comment thread recipes/tape-to-node-test/src/workflow.ts
Comment thread recipes/tape-to-node-test/src/workflow.ts Outdated
Comment thread recipes/tape-to-node-test/src/workflow.ts Outdated
Comment thread recipes/tape-to-node-test/src/workflow.ts Outdated
@AugustinMauroy AugustinMauroy changed the title feat(tape-to-node-test): first draft feat(tape-to-node-test): introduce Mar 20, 2026
@AugustinMauroy AugustinMauroy added awaiting author Reviewer has requested something from the author and removed awaiting reviewer Author has responded and needs action from the reviewer labels Mar 20, 2026
AugustinMauroy and others added 3 commits April 6, 2026 10:28
@AugustinMauroy @avivkeller
apply review
f13199b 
Co-Authored-By: Aviv Keller <me@aviv.sh>
@AugustinMauroy @avivkeller
better t.pass
39d0797 
Co-Authored-By: Aviv Keller <me@aviv.sh>
@AugustinMauroy
Update workflow.ts
21a2d7c
@AugustinMauroy AugustinMauroy added awaiting reviewer Author has responded and needs action from the reviewer and removed awaiting author Reviewer has requested something from the author labels Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ljharb ljharb ljharb left review comments

@avivkeller avivkeller avivkeller left review comments

@brunocroh brunocroh brunocroh approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

awaiting reviewer Author has responded and needs action from the reviewer

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

spec: tape-to-node-test-runner

4 participants

@AugustinMauroy @ljharb @brunocroh @avivkeller