Repository navigation
investigate flaky test-tls-regr-gh-5108 #10966
Description
- Version: v8.0.0-pre
- Platform: smartos16-64
- Subsystem: tls
https://ci.nodejs.org/job/node-test-commit-smartos/6523/nodes=smartos16-64/consoleFull:
not ok 1230 parallel/test-tls-regr-gh-5108
---
duration_ms: 60.94
severity: fail
stack: |-
timeout
Activity
Trott commentedon Jan 23, 2017
Refs: aa05269
Refs: #5108
/cc @indutny @ChALkeR for any input/insight but not a lot of data to go on right now...
Trott commentedon Jan 23, 2017
(Aside: This is a really good example of why I think it's good to have
console.log()and/orconsole.error()statements in tests sometimes. I know it's contrary to the philosophy of some folks, but in a case like this where the test times out, it's helpful to be able to narrow down where the problem might be. And tests timing out with no info like this seems to be an increasing share of our flaky tests.)joyeecheung commentedon Jan 23, 2017
FWIW I think doing
console.erroronly when some environment variables or something incommonare present for debugging purposes would be more acceptable to people who believe it shouldn't be used in tests? (don't really have a strong opinion about it myself)gibfahn commentedon Jan 23, 2017
+1 to console.log/console.error being used (sparingly) in tests, especially console.log, as it currently is only displayed if the test fails, so it doesn't clutter up normal test output.
evanlucas commentedon Jan 25, 2017
If we don't want to normally see them, why not add a util.debuglog to common and use something like
common.log()?gibfahn commentedon Jan 25, 2017
@evanlucas how would you then allow them to be seen?
evanlucas commentedon Jan 25, 2017
@gibfahn
NODE_DEBUG=test ./node test/...gibfahn commentedon Jan 25, 2017
@evanlucas Cool, I didn't know about that. Definite +1 on having a
common.log().Doc link: https://nodejs.org/api/util.html#util_util_debuglog_section
misterdjules commentedon Jan 31, 2017
To paraphrase what I wrote in #11026:
So far, I haven't been able to reproduce the problem described by any of the issues listed above.
In order to be able to get more information and investigate future spurious failures, I submitted a PR that sends
SIGABRTinstead ofSIGTERMto test processes that timeout. This will allow us to take a look at core files generated from these processes with tools such as llnode and mdb_v8, and will potentially help us root cause these issues.In the meantime I'll continue trying to reproduce and investigate those issues, I'll keep you posted.
Trott commentedon Jul 16, 2017
Haven't seen this in a while. Closing. Can re-open if it recurs.