Skip to content

Conversation

@BethGriggs
Copy link
Member

Competing timers were causing a race condition and thus the test was
flaky. Instead, we check an object property on process exit.

Fixes: #7650
PR-URL: #7857
Reviewed-By: Santiago Gimeno santiago.gimeno@gmail.com
Reviewed-By: Fedor Indutny fedor.indutny@gmail.com
Reviewed-By: jasnell - James M Snell jasnell@gmail.com

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

Competing timers were causing a race condition and thus the test was
flaky. Instead, we check an object property on process exit.

Fixes: nodejs#7650
PR-URL: nodejs#7857
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: jasnell - James M Snell <jasnell@gmail.com>
@nodejs-github-bot nodejs-github-bot added test Issues and PRs related to the tests. v4.x labels Apr 21, 2017
@gibfahn
Copy link
Member

gibfahn commented Apr 21, 2017

@MylesBorins it'd be really helpful to have this land on v4.x if possible. This test is flaky, and we've seen it fail several times.

I'm also not sure why it wasn't backported a while ago, as it landed in master last July.

@mscdex mscdex added the tls Issues and PRs related to the tls subsystem. label Apr 21, 2017
@MylesBorins
Copy link
Contributor

@gibfahn by the maintenance contract we are no longer supposed to be backporting fixes to tests. It didn't land because we have not been super adamant about making sure tests that don't land cleanly backport.

Personally I'm -1 on making exceptions for tests, but willing to reconsider if @nodejs/lts thinks this should land

@gibfahn
Copy link
Member

gibfahn commented Apr 28, 2017

@MylesBorins did you say you were going to land this, or should we close this PR?

MylesBorins pushed a commit that referenced this pull request May 2, 2017
Competing timers were causing a race condition and thus the test was
flaky. Instead, we check an object property on process exit.

Fixes: #7650
Backport-PR-URL: #12567
PR-URL: #7857
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: jasnell - James M Snell <jasnell@gmail.com>
@MylesBorins
Copy link
Contributor

landed in 6040efd

@MylesBorins MylesBorins closed this May 2, 2017
@BethGriggs BethGriggs deleted the backport-7857-to-v4 branch February 21, 2018 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test Issues and PRs related to the tests. tls Issues and PRs related to the tls subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants