Make coverage results more stable #1293
Conversation
mszabo-wikia
requested review from
everett980,
rubenvp8510 and
tiffon
as code owners
mszabo-wikia
force-pushed
the
more-stable-coverage
branch
from
50465ea to
04c3707
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1293 +/- ##
=======================================
Coverage 95.58% 95.58%
=======================================
Files 244 244
Lines 7651 7651
Branches 2006 2006
=======================================
Hits 7313 7313
Misses 338 338
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
|
Looks like there's 1 line left still |
|
Wanna fix it before merging? Also, how are you finding which lines' coverage changes? Codecov.io keeps breaking for me, refuses to show diffs. |
|
Yeah, I just need to come up with a good trace that can be used to test it—it's been proving elusive so far. For determining coverage, is useful to observe how coverage results for this single file change over multiple executions of the entire suite, and does the same but only running a single test file. |
Coverage results are currently flaky because src/components/TracePage/index.test.js uses randomly generated trace data to integration test the page, which sometimes covers code paths in tableValues.tsx not covered by its own unit tests. Attempt to make things more stable by extending unit test coverage for tableValues.tsx. Signed-off-by: Máté Szabó <[email protected]>
mszabo-wikia
force-pushed
the
more-stable-coverage
branch
from
04c3707 to
cbd49dd
Compare
| @@ -209,7 +209,7 @@ function calculateContent(trace: Trace, span: Span, allSpans: Span[], resultValu | |||
|
|
|||
| tempSelf = onlyOverlay(overlayWithout, allChildrenWithout, tempSelf, span); | |||
| const diff = span.relativeStartTime + span.duration - earliestLongerAsParent.relativeStartTime; | |||
| tempSelf -= diff; | |||
| tempSelf = Math.max(0, tempSelf - diff); | |||
There was a problem hiding this comment.
Apparently a bug - the final test case returns a negative value without this guard. The example is somewhat contrived as it hinges on two child spans firing off exactly at trace start time—which is unlikely, but that seems to be the only way to reach line 34.
## Which problem is this PR solving? - Makes coverage checks more stable ## Short description of the changes Coverage results are currently flaky because src/components/TracePage/index.test.js uses randomly generated trace data to integration test the page, which sometimes covers code paths in tableValues.tsx not covered by its own unit tests. Attempt to make things more stable by extending unit test coverage for tableValues.tsx. Signed-off-by: Máté Szabó <[email protected]>
Which problem is this PR solving?
Short description of the changes
Coverage results are currently flaky because
src/components/TracePage/index.test.js uses randomly generated trace data to integration test the page, which sometimes covers code paths in tableValues.tsx not covered by its own unit tests. Attempt to make things more stable by extending unit test coverage for tableValues.tsx.