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

Investigate flaky test test-child-process-spawnsync #2470

Closed
joaocgreis opened this issue Aug 20, 2015 · 2 comments
Closed

Investigate flaky test test-child-process-spawnsync #2470

joaocgreis opened this issue Aug 20, 2015 · 2 comments
Labels
build Issues and PRs related to build files or the CI. test Issues and PRs related to the tests.

Comments

@joaocgreis
Copy link
Member

Examples of failures:
@joaocgreis joaocgreis added build Issues and PRs related to build files or the CI. test Issues and PRs related to the tests. labels Aug 20, 2015
@Trott
Copy link
Member

Trott commented Aug 25, 2015

If I'm understanding the test correctly, the part of the test that is flaky is actually invalid. It checks to see if the function passed to setTimeout() executes in the same second as call to spawnSync() finishes. This will not always necessarily be the case, and the test will only get flakier if more things are added to it. (There are two other test cases below the flaky one.)

There's already an assertion to confirm that the spawnSync() ends before setTimeout() runs, so I'll submit a PR to remove the check altogether rather than do something like convert the timestamps to nanoseconds since start and make sure the one set by setTimeout() is greater than the one set by spawnSync().

@Trott Trott mentioned this issue Aug 25, 2015
Closed
Trott added a commit to Trott/io.js that referenced this issue Aug 25, 2015
@Trott
test: make spawnSync() test robust
afbda2c 
The test had checked that a timer fired within a period after
spawnSync() returns. The result was a test that sometimes was
flaky.

Because there's no guarantee of how long a timer will take
before running, remove the check. There is a check that the
timer runs after spawnSync() so that is sufficient.

Fixes: nodejs#2470
Trott added a commit that referenced this issue Aug 25, 2015
@Trott
test: make spawnSync() test robust
7e63eb7 
The test had checked that a timer fired within a period after
spawnSync() returns. The result was a test that sometimes was
flaky.

Because there's no guarantee of how long a timer will take
before running, remove the check. There is a check that the
timer runs after spawnSync() so that is sufficient.

PR-URL: #2535
Fixes: #2470
Reviewed-By: Ben Noordhuis <[email protected]>
@Trott
Copy link
Member

Trott commented Aug 25, 2015

Fixed in 7e63eb7 / #2535

@Trott Trott closed this as completed Aug 25, 2015
@joaocgreis joaocgreis mentioned this issue Aug 25, 2015
Closed
Trott added a commit that referenced this issue Aug 26, 2015
@Trott @rvagg
test: make spawnSync() test robust
856baf4 
The test had checked that a timer fired within a period after
spawnSync() returns. The result was a test that sometimes was
flaky.

Because there's no guarantee of how long a timer will take
before running, remove the check. There is a check that the
timer runs after spawnSync() so that is sufficient.

PR-URL: #2535
Fixes: #2470
Reviewed-By: Ben Noordhuis <[email protected]>
@Trott Trott mentioned this issue Aug 27, 2015
Closed
@trongham trongham mentioned this issue May 13, 2022
Open
@lasalasa lasalasa mentioned this issue May 13, 2022
Open
@aliceUnhinged613 aliceUnhinged613 mentioned this issue May 13, 2022
Open
@adamlaska adamlaska mentioned this issue May 13, 2022
Open
@pratapadi pratapadi mentioned this issue May 13, 2022
Open
@aliscco aliscco mentioned this issue May 13, 2022
Open
@bsmoove247 bsmoove247 mentioned this issue May 13, 2022
Open
@snyk-bot snyk-bot mentioned this issue May 13, 2022
Open
@baby636 baby636 mentioned this issue May 13, 2022
Open
@Verdinjoshua26 Verdinjoshua26 mentioned this issue May 13, 2022
Open
@baophucct baophucct mentioned this issue May 13, 2022
Open
@saeedahassan saeedahassan mentioned this issue May 13, 2022
Open
@enterstudio enterstudio mentioned this issue May 13, 2022
Open
@GregorFendt GregorFendt mentioned this issue May 13, 2022
Open
@ShubhraKar ShubhraKar mentioned this issue May 13, 2022
Open
@Mcculloughj Mcculloughj mentioned this issue May 13, 2022
Open
@snyk-bot snyk-bot mentioned this issue May 13, 2022
Open
@awaisab172 awaisab172 mentioned this issue May 14, 2022
Open
@pratapadi pratapadi mentioned this issue May 14, 2022
Open
@ajesse11x ajesse11x mentioned this issue May 14, 2022
Open
@wedataintelligence wedataintelligence mentioned this issue May 14, 2022
Open
@bumplzz69 bumplzz69 mentioned this issue May 14, 2022
Open
@erdun erdun mentioned this issue May 14, 2022
Open
@sirinartk sirinartk mentioned this issue May 14, 2022
Open
@Qdigital Qdigital mentioned this issue May 14, 2022
Open
@kaocher82 kaocher82 mentioned this issue May 14, 2022
Open
@vaibhavb vaibhavb mentioned this issue May 14, 2022
Open
@solebox solebox mentioned this issue May 14, 2022
Open
@apoorvanand apoorvanand mentioned this issue May 14, 2022
Open
@terrorizer1980 terrorizer1980 mentioned this issue May 14, 2022
Open
@snyk-bot snyk-bot mentioned this issue May 14, 2022
Open
@molllyn1 molllyn1 mentioned this issue May 14, 2022
Open
@AyushG3112 AyushG3112 mentioned this issue May 14, 2022
Open
@s4sc s4sc mentioned this issue May 14, 2022
Open
@yotamselementor yotamselementor mentioned this issue May 14, 2022
Open
@xphillyx xphillyx mentioned this issue May 14, 2022
Open
@AyushG3112 AyushG3112 mentioned this issue Jun 22, 2022
Open
@snyk-bot snyk-bot mentioned this issue Jun 24, 2022
Open
@snyk-bot snyk-bot mentioned this issue Sep 23, 2022
Open
@Verdinjoshua26 Verdinjoshua26 mentioned this issue Sep 26, 2022
Open
This was referenced Sep 26, 2022
Open
Open
@RoeeOren RoeeOren mentioned this issue Oct 5, 2022
Open
@MrBrain295 MrBrain295 mentioned this issue Oct 5, 2022
Open
@bumplzz69 bumplzz69 mentioned this issue Oct 5, 2022
Open
@bsmoove247 bsmoove247 mentioned this issue Oct 6, 2022
Open
@snyk-bot snyk-bot mentioned this issue Oct 9, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@joaocgreis @Trott