Repository navigation
test/async-hooks/test-graph.http.js flaky #27617
Description
Activity
Without the strict matching the graph tests are mostly useless. The test test-graph.http.js had
HTTPPARSERin the expected graph and was green evenHTTPPARSERwas never emitted.If we remove the strict check I added in #27477 we should add something else instead in all the graph tests to verify that the relevant (?) resources are present.
I can think about following options:
- add a flag like
isMandatoryto the graph and verify only the presence of them instead all of them. - remove non use case relevant entries from the expected graph and concentrate on the essence instead the details (e.g. in case of HTTP keep TCP/HTTP stuff but remove at least optional timeouts)
- add a flag like
The approach of listing
isMandatoryorisRequiredsounds reasonable to me. I'll try to open a PR for this the next time I get some time at the keyboard, but others are welcome to open a PR too.- added a commit that references this issue
on May 16, 2019 - added a commit that references this issue
on May 19, 2019 - added a commit that references this issue
on May 20, 2019
Background: #27558 (comment)
Async Hooks tests that verify that async resource graph – specifically
test/async-hooks/test-graph.http.js– are being flaky with a low probability. We suspect this is due to theverifyGraphimprovements in #27477.Async Hooks exposes a lot of internal details, and in this case it ends being sensitive to the fact that for optimization and implementation details reasons we may occasionally (1 in 1000 in this case) create an async resource graph that the test originally expected.
IMO, we shouldn't have strict expectations on the structure of the async resource graph – there is a lot of implementation detail that is captured hence. The stricter check for expected types added as part of #27477 may end up being more problematic in the long term, IMO. At the least we should default to non-strict matching and use strict matching where the test-cases explicitly calls for it.
/cc @Flarna thoughts?