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 test-regress-GH-897 #10903

Closed
wants to merge 1 commit into from
Closed

test: fix flaky test-regress-GH-897 #10903

wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Jan 19, 2017

Even after being moved to sequential in
1ce05ad, test-regress-GH-897 still
was occasionally flaky on Raspberry Pi devices on CI.

The test is especially sensitive to resource constraints. It failed
reliably on my laptop if I moved it to parallel and ran 32 competing
node test processes. Even for a flaky test, that's unusually low. I
typically don't see problems, even for flaky tests, until I get up to
around four times that number.

On a Raspberry Pi, of course, that sensitivity to resource constraints
will manifest much sooner.

This change checks the order of timers firing, rather than the duration
before a timer is fired. This eliminates the sensitivity to resource
constraints. The test can now be moved back to parallel. I am able to
run many copies of the test simultaneously without seeing test failures.

Fixes: #10073

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

test timers

@Trott
test: fix flaky test-regress-nodejsGH-897
65d0489 
Even after being moved to `sequential` in
1ce05ad, `test-regress-nodejsGH-897` still
was occasionally flaky on Raspberry Pi devices on CI.

The test is especially sensitive to resource constraints. It failed
reliably on my laptop if I moved it to `parallel` and ran 32 competing
node test processes. Even for a flaky test, that's unusually low. I
typically don't see problems, even for flaky tests, until I get up to
around four times that number.

On a Raspberry Pi, of course, that sensitivity to resource constraints
will manifest much sooner.

This change checks the order of timers firing, rather than the duration
before a timer is fired. This eliminates the sensitivity to resource
constraints. The test can now be moved back to `parallel`. I am able to
run many copies of the test simultaneously without seeing test failures.

Fixes: nodejs#10073
@Trott Trott added test Issues and PRs related to the tests. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. labels Jan 19, 2017
@nodejs-github-bot nodejs-github-bot added test Issues and PRs related to the tests. lts-watch-v6.x labels Jan 19, 2017
@Trott
Copy link
Member Author

Trott commented Jan 19, 2017

/cc @nodejs/testing @Fishrock123 @misterdjules

@Trott
Copy link
Member Author

Trott commented Jan 19, 2017

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

@cjihrig
Copy link
Contributor

cjihrig commented Jan 19, 2017

If you go with this approach, do you think it might be worth throwing a 0 millisecond timeout into the mix?

@Trott
Copy link
Member Author

Trott commented Jan 19, 2017 •
edited
Loading

If you go with this approach, do you think it might be worth throwing a 0 millisecond timeout into the mix?

I was going to do that in a subsequent PR (although if someone eager wants to beat me to it, I will be happy to not do it myself). Basically, find out if we're already testing for things like 0, -1, undefined, those sorts of things. Make sure they all get coerced to 1. If so, consolidate into a single file. If not, move them here, rename the file, and have it be the general "this makes sure your wonky values get coerced to 1 in timers" test.

Trott added a commit to Trott/io.js that referenced this pull request Jan 23, 2017
@Trott
test: fix flaky test-regress-nodejsGH-897
80c72c6 
Even after being moved to `sequential` in
1ce05ad, `test-regress-nodejsGH-897` still
was occasionally flaky on Raspberry Pi devices on CI.

The test is especially sensitive to resource constraints. It failed
reliably on my laptop if I moved it to `parallel` and ran 32 competing
node test processes. Even for a flaky test, that's unusually low. I
typically don't see problems, even for flaky tests, until I get up to
around four times that number.

On a Raspberry Pi, of course, that sensitivity to resource constraints
will manifest much sooner.

This change checks the order of timers firing, rather than the duration
before a timer is fired. This eliminates the sensitivity to resource
constraints. The test can now be moved back to `parallel`. I am able to
run many copies of the test simultaneously without seeing test failures.

PR-URL: nodejs#10903
Fixes: nodejs#10073
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@Trott
Copy link
Member Author

Trott commented Jan 23, 2017

Landed in 80c72c6

@Trott Trott closed this Jan 23, 2017
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 25, 2017
@Trott @italoacasas
test: fix flaky test-regress-nodejsGH-897
09f2831 
Even after being moved to `sequential` in
1ce05ad, `test-regress-nodejsGH-897` still
was occasionally flaky on Raspberry Pi devices on CI.

The test is especially sensitive to resource constraints. It failed
reliably on my laptop if I moved it to `parallel` and ran 32 competing
node test processes. Even for a flaky test, that's unusually low. I
typically don't see problems, even for flaky tests, until I get up to
around four times that number.

On a Raspberry Pi, of course, that sensitivity to resource constraints
will manifest much sooner.

This change checks the order of timers firing, rather than the duration
before a timer is fired. This eliminates the sensitivity to resource
constraints. The test can now be moved back to `parallel`. I am able to
run many copies of the test simultaneously without seeing test failures.

PR-URL: nodejs#10903
Fixes: nodejs#10073
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@italoacasas italoacasas mentioned this pull request Jan 25, 2017
Closed
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 27, 2017
@Trott @italoacasas
test: fix flaky test-regress-nodejsGH-897
3ce9681 
Even after being moved to `sequential` in
1ce05ad, `test-regress-nodejsGH-897` still
was occasionally flaky on Raspberry Pi devices on CI.

The test is especially sensitive to resource constraints. It failed
reliably on my laptop if I moved it to `parallel` and ran 32 competing
node test processes. Even for a flaky test, that's unusually low. I
typically don't see problems, even for flaky tests, until I get up to
around four times that number.

On a Raspberry Pi, of course, that sensitivity to resource constraints
will manifest much sooner.

This change checks the order of timers firing, rather than the duration
before a timer is fired. This eliminates the sensitivity to resource
constraints. The test can now be moved back to `parallel`. I am able to
run many copies of the test simultaneously without seeing test failures.

PR-URL: nodejs#10903
Fixes: nodejs#10073
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@italoacasas italoacasas mentioned this pull request Jan 29, 2017
Merged
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 30, 2017
@Trott @italoacasas
test: fix flaky test-regress-nodejsGH-897
9d5170f 
Even after being moved to `sequential` in
1ce05ad, `test-regress-nodejsGH-897` still
was occasionally flaky on Raspberry Pi devices on CI.

The test is especially sensitive to resource constraints. It failed
reliably on my laptop if I moved it to `parallel` and ran 32 competing
node test processes. Even for a flaky test, that's unusually low. I
typically don't see problems, even for flaky tests, until I get up to
around four times that number.

On a Raspberry Pi, of course, that sensitivity to resource constraints
will manifest much sooner.

This change checks the order of timers firing, rather than the duration
before a timer is fired. This eliminates the sensitivity to resource
constraints. The test can now be moved back to `parallel`. I am able to
run many copies of the test simultaneously without seeing test failures.

PR-URL: nodejs#10903
Fixes: nodejs#10073
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 30, 2017
@Trott @italoacasas
test: fix flaky test-regress-nodejsGH-897
1b8c238 
Even after being moved to `sequential` in
1ce05ad, `test-regress-nodejsGH-897` still
was occasionally flaky on Raspberry Pi devices on CI.

The test is especially sensitive to resource constraints. It failed
reliably on my laptop if I moved it to `parallel` and ran 32 competing
node test processes. Even for a flaky test, that's unusually low. I
typically don't see problems, even for flaky tests, until I get up to
around four times that number.

On a Raspberry Pi, of course, that sensitivity to resource constraints
will manifest much sooner.

This change checks the order of timers firing, rather than the duration
before a timer is fired. This eliminates the sensitivity to resource
constraints. The test can now be moved back to `parallel`. I am able to
run many copies of the test simultaneously without seeing test failures.

PR-URL: nodejs#10903
Fixes: nodejs#10073
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 8, 2017
@Trott @MylesBorins
test: fix flaky test-regress-GH-897
8ecd878 
Even after being moved to `sequential` in
1ce05ad, `test-regress-GH-897` still
was occasionally flaky on Raspberry Pi devices on CI.

The test is especially sensitive to resource constraints. It failed
reliably on my laptop if I moved it to `parallel` and ran 32 competing
node test processes. Even for a flaky test, that's unusually low. I
typically don't see problems, even for flaky tests, until I get up to
around four times that number.

On a Raspberry Pi, of course, that sensitivity to resource constraints
will manifest much sooner.

This change checks the order of timers firing, rather than the duration
before a timer is fired. This eliminates the sensitivity to resource
constraints. The test can now be moved back to `parallel`. I am able to
run many copies of the test simultaneously without seeing test failures.

PR-URL: #10903
Fixes: #10073
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
MylesBorins pushed a commit that referenced this pull request Mar 9, 2017
@Trott @MylesBorins
test: fix flaky test-regress-GH-897
e59697c 
Even after being moved to `sequential` in
1ce05ad, `test-regress-GH-897` still
was occasionally flaky on Raspberry Pi devices on CI.

The test is especially sensitive to resource constraints. It failed
reliably on my laptop if I moved it to `parallel` and ran 32 competing
node test processes. Even for a flaky test, that's unusually low. I
typically don't see problems, even for flaky tests, until I get up to
around four times that number.

On a Raspberry Pi, of course, that sensitivity to resource constraints
will manifest much sooner.

This change checks the order of timers firing, rather than the duration
before a timer is fired. This eliminates the sensitivity to resource
constraints. The test can now be moved back to `parallel`. I am able to
run many copies of the test simultaneously without seeing test failures.

PR-URL: #10903
Fixes: #10073
Reviewed-By: Santiago Gimeno <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Mar 9, 2017
Merged
@Trott Trott deleted the fix-test-regress-gh-897 branch January 13, 2022 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lpinca lpinca lpinca approved these changes

@santigimeno santigimeno santigimeno approved these changes

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.

investigate flaky sequential/test-regress-GH-897 on Raspberry Pi
6 participants
@Trott @cjihrig @santigimeno @lpinca @MylesBorins @nodejs-github-bot