Closed
Bug 1472686
Opened 6 years ago
Closed 6 years ago
Don't reset counters for baseline tests
Categories
(Testing :: Code Coverage, defect)
Testing
Code Coverage
Tracking
(firefox63 fixed)
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: marco, Assigned: marco)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•6 years ago
|
||
We already don't set GCOV_RESULTS_DIR for baseline tests a few lines before, but if we don't clean the env, a baseline test that comes after a non-baseline test will use the same GCOV_RESULTS_DIR (and, by having a non-empty GCOV_RESULTS_DIR, it will reset/dump the counters).
Attachment #8989171 -
Flags: review?(jmaher)
Comment 2•6 years ago
|
||
Comment on attachment 8989171 [details] [diff] [review]
Patch
Review of attachment 8989171 [details] [diff] [review]:
-----------------------------------------------------------------
I trust this is ok
::: testing/mozharness/scripts/desktop_unittest.py
@@ +909,5 @@
> per_test_args[-1]
> )
> if 'GCOV_RESULTS_DIR' in env:
> shutil.rmtree(gcov_dir)
> + del env['GCOV_RESULTS_DIR']
I thought this was always deleted in add_per_test_coverage_report. I remember asking about this specifically in my review last week and saw that it was after you mentioned it.
Attachment #8989171 -
Flags: review?(jmaher) → review+
Assignee | ||
Comment 3•6 years ago
|
||
(In reply to Joel Maher ( :jmaher ) (UTC-4) from comment #2)
> Comment on attachment 8989171 [details] [diff] [review]
> Patch
>
> Review of attachment 8989171 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> I trust this is ok
>
> ::: testing/mozharness/scripts/desktop_unittest.py
> @@ +909,5 @@
> > per_test_args[-1]
> > )
> > if 'GCOV_RESULTS_DIR' in env:
> > shutil.rmtree(gcov_dir)
> > + del env['GCOV_RESULTS_DIR']
>
> I thought this was always deleted in add_per_test_coverage_report. I
> remember asking about this specifically in my review last week and saw that
> it was after you mentioned it.
That was about the directory (https://bugzilla.mozilla.org/show_bug.cgi?id=1471769#c7) itself, which is removed by add_per_test_coverage_report, but the env variable isn't :(
Pushed by mcastelluccio@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1ef50d759e7d
Don't reset/dump counters for baseline tests. r=jmaher
Comment 5•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in
before you can comment on or make changes to this bug.
Description
•