build: use c8 for coverage

[JustinBeckwith]

No description provided.

[alexander-fenster]

Tests are totally broken, who we gonna call?

[JustinBeckwith]

@DominicKramer !

[DominicKramer]

I'm not sure offhand why these are failing. I'll have to look into more detail.

[bcoe]

@DominicKramer this looks like it's potentially surfaced an error with Istanbul, let me know when you have some time to dig in (we might need to patch something in the lcov reporter it looks like, assuming we continue getting exceptions).

[DominicKramer]

@bcoe I should be able to get to this by the end of the week. I'll let you know if I need any help. Thanks.

[DominicKramer]

Yes, it looks like instanbul and c8 are making semantic changes to the tests that are causing them to fail. I haven't been able to isolate exactly how.

Is there are hard requirement to switching to c8, and, if so, what is the timeline when the change is needed. Thanks.

[bcoe]

@DominicKramer there's no hard requirement, and we can close this. But I would like to eventually figure out how V8's built in coverage is interfering with our profiling, for the benefit of the Node.js project.

c8 doesn't do anything other than setting the environment variable NODE_V8_COVERAGE, this in turn results in running Node's inspector session with slightly different configuration settings, I'm very curious how these configuration settings are interfering with this test suite.

[codecov]

Codecov Report

Merging #806 into master will decrease coverage by 61.34%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #806       +/-   ##
==========================================
- Coverage   66.00%   4.65%   -61.35%     
==========================================
  Files          20      20               
  Lines        1606    6357     +4751     
  Branches      330      21      -309     
==========================================
- Hits         1060     296      -764     
- Misses        470    6061     +5591     
+ Partials       76       0       -76     

Impacted Files	Coverage Δ
src/agent/config.ts	0.00% <0.00%> (-100.00%)	⬇️
src/agent/util/debug-assert.ts	0.00% <0.00%> (-100.00%)	⬇️
src/client/stackdriver/status-message.ts	0.00% <0.00%> (-100.00%)	⬇️
src/agent/io/scanner.ts	0.00% <0.00%> (-96.35%)	⬇️
src/agent/util/utils.ts	0.00% <0.00%> (-96.35%)	⬇️
src/index.ts	0.00% <0.00%> (-94.74%)	⬇️
src/agent/state/inspector-state.ts	0.00% <0.00%> (-92.13%)	⬇️
src/agent/util/validator.ts	0.00% <0.00%> (-84.62%)	⬇️
src/agent/v8/inspector-debugapi.ts	0.00% <0.00%> (-84.43%)	⬇️
src/agent/io/sourcemapper.ts	0.00% <0.00%> (-84.29%)	⬇️
... and 12 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77264fc...45b12e8. Read the comment docs.

[bcoe]

see: https://bugs.chromium.org/p/v8/issues/detail?id=10856

[bcoe]

@JustinBeckwith I think we can make coverage work now if we add a feature to c8 that allows us to turn it on for only certain runtimes, e.g.,

c8 --min-node-version=15

[JustinBeckwith]

Totally your call - this is pretty low stakes :)

[JustinBeckwith]

Cleaning up old PRs. I don't want to throw a birthday party for this thing so I'm closing it out. We can come back around in a few node versions :)