Skip to content

Conversation

Jarred-Sumner
Copy link

The test test-worker-message-port-infinite-message-loop.js currently passes if the MessagePort does not emit any messages.

If the message port does not emit a "message" event then the infinite message loop would never occur, therefore it's probably not the intent of the test.

Context: this test was incorrectly passing in Bun when we currently do not start the MessagePort until the start method is called. This test continues to pass in Node and now correctly fails in Bun (we will fix this later).

Add assertion to verify that the MessagePort's message event is
actually emitted in test-worker-message-port-infinite-message-loop.js.
Previously, the test could pass even if the event was not fired.
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Sep 14, 2025
@Jarred-Sumner Jarred-Sumner changed the title Make test-worker-message-port-infinite-message-loop.js fail when the port1.on('message' callback isn't called Make test-worker-message-port-infinite-message-loop.js fail when the port1.on('message' callback isn't called Sep 14, 2025
Copy link

codecov bot commented Sep 14, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.27%. Comparing base (c8c6bfa) to head (c6e6ff3).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #59885      +/-   ##
==========================================
- Coverage   88.29%   88.27%   -0.02%     
==========================================
  Files         702      702              
  Lines      206875   206875              
  Branches    39806    39802       -4     
==========================================
- Hits       182667   182627      -40     
- Misses      16218    16267      +49     
+ Partials     7990     7981       -9     

see 32 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@anonrig anonrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Sep 14, 2025
@anonrig anonrig requested a review from jasnell September 14, 2025 11:51
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 14, 2025
@nodejs-github-bot
Copy link
Collaborator

common.mustCall(() => {
assert(count > 0, 'count should be greater than 0');
})
);
Copy link
Member

Choose a reason for hiding this comment

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

I think the more correct way to do this would be to wrap the callback to the port1.on in a common.mustCallAtLeast(...)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants