Skip to content

fix: reject repeated done callbacks in TAP harness - #304

Merged
pmatos merged 5 commits into
mainfrom
sym/jsse/272-node-test-harness-reject-repeated-done-callbacks-in-the-describe-it-tap-runner
Jul 20, 2026
Merged

fix: reject repeated done callbacks in TAP harness#304
pmatos merged 5 commits into
mainfrom
sym/jsse/272-node-test-harness-reject-repeated-done-callbacks-in-the-describe-it-tap-runner

Conversation

@pmatos

@pmatos pmatos commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Summary:

  • reject synchronous and post-settlement repeated done() calls in callback-style TAP tests and hooks
  • buffer TAP rows so late duplicates update the original result without duplicate TAP IDs
  • drain user one-shot timers and wait for at most the next tick of each user interval before freezing TAP output
  • require one quiet macrotask turn so promise continuations can enqueue follow-up timers
  • give each suite hook an idempotent failure recorder shared by its rejection and late-duplicate paths
  • cover synchronous, promise/timer, interval, and suite-hook repeated completion regressions

Validation:

  • node --check scripts/node-test-harness.js
  • ./scripts/run-harness-selftest.sh --no-build
  • forced-Node interval and suite-hook fixtures
  • cargo fmt --check
  • cargo clippy
  • ./scripts/lint.sh
  • cargo build --release
  • cargo test --release (294 unit tests plus smoke oracle)
  • uv run python scripts/run-custom-tests.py (7/7)
  • ./scripts/run-library-tests.sh js-sha256 (916/916 on jsse and Node)
  • full test262: 99,546/99,568; same 22 environment-sensitive Intl baseline mismatches and 323 existing new passes. This branch has no engine/Cargo diff, so README and the baseline remain unchanged.

Closes #272

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bd2c6a0879

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/node-test-harness.js

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e21c22ab09

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/node-test-harness.js

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f2555e42e1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/node-test-harness.js Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aaa5a9dceb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/node-test-harness.js Outdated
Comment thread scripts/node-test-harness.js Outdated
@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

@pmatos
pmatos merged commit 38a1b8f into main Jul 20, 2026
8 checks passed
@pmatos
pmatos deleted the sym/jsse/272-node-test-harness-reject-repeated-done-callbacks-in-the-describe-it-tap-runner branch July 20, 2026 05:51
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 0.2.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

node test harness: reject repeated done() callbacks in the describe/it TAP runner

1 participant