Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: improve abort signal dropping test #56339

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

geeksilva97
Copy link
Contributor

@geeksilva97 geeksilva97 commented Dec 22, 2024

Refs #56190

This PR does two things in the specific test case that is flaky:

  • Make the composedSignal a weak ref so the test won't hold any reference that could keep that variable from being GCed.
  • Add gcUntil so it will make more calls to global.gc. Precisely, until that condition is reached or a maximum number of attempts is hit.

@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 Dec 22, 2024
@geeksilva97 geeksilva97 marked this pull request as ready for review December 22, 2024 16:36
Copy link
Member

@lpinca lpinca left a comment

Choose a reason for hiding this comment

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

RSLGTM

@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 22, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 22, 2024
@nodejs-github-bot
Copy link
Collaborator

Copy link

codecov bot commented Dec 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.53%. Comparing base (4fd5db4) to head (50a5763).
Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #56339      +/-   ##
==========================================
- Coverage   88.54%   88.53%   -0.01%     
==========================================
  Files         657      657              
  Lines      190395   190395              
  Branches    36552    36554       +2     
==========================================
- Hits       168586   168570      -16     
- Misses      14992    15001       +9     
- Partials     6817     6824       +7     

see 36 files with indirect coverage changes

@nodejs-github-bot
Copy link
Collaborator

@jazelly jazelly added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed needs-ci PRs that need a full CI run. labels Dec 24, 2024
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. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants