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.

Sign up for GitHub

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

Jump to bottom

test: fix flaky unrefed timers test #4599

Closed
wants to merge 1 commit into from
Closed

test: fix flaky unrefed timers test #4599

wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Jan 9, 2016

Fixes: #4559

A hopefully easier-to-LGTM fix than (my personally preferred) #4561.

R=@Fishrock123

@Trott Trott added timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. test Issues and PRs related to the tests. labels Jan 9, 2016
@Trott
Copy link
Member Author

Trott commented Jan 9, 2016

CI: https://ci.nodejs.org/job/node-test-pull-request/1176/

@jbergstroem
Copy link
Member

This all-green stuff is starting to scare me. LGTM but prefer the other fix; will hopefully land later.

@Trott
Copy link
Member Author

Trott commented Jan 11, 2016

Landed in e071894

@Trott Trott closed this Jan 11, 2016
Trott added a commit that referenced this pull request Jan 11, 2016
@Trott
test: fix flaky unrefed timers test
e071894 
Remove unnecessary `setImmediate()` that causes a minor race condition.
Stop the test after 3 occurrences rather than 5 to allow for slower
hosts running the test in parallel with other tests.

Fixes: #4559
PR-URL: #4599
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
@Fishrock123
Copy link
Member

I think we should still up the timeout duration whatever you feel is necessary, but ok.

MylesBorins pushed a commit that referenced this pull request Jan 11, 2016
@Trott
test: fix flaky unrefed timers test
36b385d 
Remove unnecessary `setImmediate()` that causes a minor race condition.
Stop the test after 3 occurrences rather than 5 to allow for slower
hosts running the test in parallel with other tests.

Fixes: #4559
PR-URL: #4599
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
@MylesBorins MylesBorins mentioned this pull request Jan 11, 2016
Merged
MylesBorins pushed a commit that referenced this pull request Jan 12, 2016
@Trott
test: fix flaky unrefed timers test
6f9a96f 
Remove unnecessary `setImmediate()` that causes a minor race condition.
Stop the test after 3 occurrences rather than 5 to allow for slower
hosts running the test in parallel with other tests.

Fixes: #4559
PR-URL: #4599
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
MylesBorins pushed a commit that referenced this pull request Jan 28, 2016
@Trott
test: fix flaky unrefed timers test
18e968b 
Remove unnecessary `setImmediate()` that causes a minor race condition.
Stop the test after 3 occurrences rather than 5 to allow for slower
hosts running the test in parallel with other tests.

Fixes: #4559
PR-URL: #4599
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
MylesBorins pushed a commit that referenced this pull request Feb 11, 2016
@Trott
test: fix flaky unrefed timers test
94fbf92 
Remove unnecessary `setImmediate()` that causes a minor race condition.
Stop the test after 3 occurrences rather than 5 to allow for slower
hosts running the test in parallel with other tests.

Fixes: #4559
PR-URL: #4599
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 11, 2016
@Trott
test: fix flaky unrefed timers test
c1ba1dc 
Remove unnecessary `setImmediate()` that causes a minor race condition.
Stop the test after 3 occurrences rather than 5 to allow for slower
hosts running the test in parallel with other tests.

Fixes: nodejs#4559
PR-URL: nodejs#4599
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
@MylesBorins MylesBorins mentioned this pull request Feb 11, 2016
Merged
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 15, 2016
@Trott
test: fix flaky unrefed timers test
556703d 
Remove unnecessary `setImmediate()` that causes a minor race condition.
Stop the test after 3 occurrences rather than 5 to allow for slower
hosts running the test in parallel with other tests.

Fixes: nodejs#4559
PR-URL: nodejs#4599
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
@Trott
test: fix flaky unrefed timers test
8afda3e 
Remove unnecessary `setImmediate()` that causes a minor race condition.
Stop the test after 3 occurrences rather than 5 to allow for slower
hosts running the test in parallel with other tests.

Fixes: nodejs#4559
PR-URL: nodejs#4599
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
@Trott Trott deleted the refactor-unrefd branch January 13, 2022 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
test Issues and PRs related to the tests. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

test: test-timers-unrefd-interval-still-fires.js flaky on smartOS
5 participants
@Trott @jbergstroem @Fishrock123 @jasnell @MylesBorins