Closed
Bug 1480120
Opened 6 years ago
Closed 6 years ago
Cleanup test file paths in per-test coverage data
Categories
(Testing :: Code Coverage, enhancement)
Testing
Code Coverage
Tracking
(firefox63 fixed)
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: gabriel-v, Assigned: gabriel-v)
References
Details
Attachments
(1 file)
The per-test coverage data contains paths with prefixes, like: /builds/worker/workspace/build/tests/reftest/tests/dom/svg/crashtests/1477853.html should be: dom/svg/crashtests/1477853.html /builds/worker/workspace/build/tests/jsreftest/tests/non262/TypedObject/method_from.js should be: js/src/tests/non262/TypedObject/method_from.js Z:/task_1532258525/build/tests/reftest/tests/layout/generic/crashtests/1474768.html should be: layout/generic/crashtests/1474768.html Z:/task_1532647606/build/tests/jsreftest/tests/non262/TypedObject/method_from.js should be: js/src/tests/non262/TypedObject/method_from.js There are also some incorrect ones: tests/web-platform/tests/webaudio/the-audio-api/the-constantsourcenode-interface/constant-source-output.html should be: testing/web-platform/tests/webaudio/the-audio-api/the-constantsourcenode-interface/constant-source-output.html These are all the examples I could find. Here's a list of all failures if I didn't normalize '\' to '/': https://pastebin.mozilla.org/9090871 Here's a smaller list of failures if I do normalize '\' to '/': https://pastebin.mozilla.org/9090873 The fix should probably be in CodeCoverageMixin, in add_per_test_coverage_report.
Assignee | ||
Updated•6 years ago
|
Status: NEW → ASSIGNED
Comment 1•6 years ago
|
||
The simplest solution would be to fix this in testing/mozharness/mozharness/mozilla/testing/per_test_base.py and testing/mozharness/mozharness/mozilla/testing/codecoverage.py. In there we have the actual paths to the tests in the source tree.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•6 years ago
|
||
New try push with changes: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1aa20c2141221f22ee7245a8c4be615f482279bb Other try pushes have confirmed that this works on Linux. Still need to wait for the windows build to finish, to check for any errors related to the '\' separator being used instead of '/'.
Comment 4•6 years ago
|
||
mozreview-review |
Comment on attachment 8997418 [details] Bug 1480120 - Cleanup reftest, jsreftest and wpt file paths in per-test coverage data. https://reviewboard.mozilla.org/r/261196/#review268248 This looks good to me, but please also ask review to gbrown for the per_test_base.py changes. ::: testing/mozharness/mozharness/mozilla/testing/per_test_base.py:25 (Diff revision 1) > def __init__(self): > self.suites = {} > self.tests_downloaded = False > self.reftest_test_dir = None > self.jsreftest_test_dir = None > + self.test_src_path = {} Add a comment here explaining that this is a mapping between the full test path on the test machine and the test path in a source checkout.
Attachment #8997418 -
Flags: review?(mcastelluccio) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Comment 6•6 years ago
|
||
Try task that exports artifacts with good test paths: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff07fdd1af6ad4a694959faab1c03414e32d581d The Linux TC failing task doesn't seem to be related to my changes. I'll file another bug about the failure.
Comment 7•6 years ago
|
||
mozreview-review |
Comment on attachment 8997418 [details] Bug 1480120 - Cleanup reftest, jsreftest and wpt file paths in per-test coverage data. https://reviewboard.mozilla.org/r/261196/#review268852 ::: testing/mozharness/mozharness/mozilla/testing/per_test_base.py:173 (Diff revision 2) > repo_path = os.path.join(repo_tests_path, path) > # manifest paths use os.sep (like backslash on Windows) but > # automation-relevance uses posixpath.sep > repo_path = repo_path.replace(os.sep, posixpath.sep) > + test_path = os.path.join(tests_path, path) > + self._map_test_path_to_source(test_path, repo_path) Does this need to be ouside of 'if repo_path in changed_files:'?
Attachment #8997418 -
Flags: review?(gbrown) → review+
Assignee | ||
Comment 8•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8997418 [details] Bug 1480120 - Cleanup reftest, jsreftest and wpt file paths in per-test coverage data. https://reviewboard.mozilla.org/r/261196/#review268852 > Does this need to be ouside of 'if repo_path in changed_files:'? Good catch. No, it doesn't need to be outside the if.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 10•6 years ago
|
||
Updated try push with changes: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b1f02b8b0d2c314842e26b8225e90a983300ff1 Note: the linux TC failure is caused by bug 1478141, not this change.
Assignee | ||
Updated•6 years ago
|
Keywords: checkin-needed
Comment 11•6 years ago
|
||
Pushed by archaeopteryx@coole-files.de: https://hg.mozilla.org/integration/autoland/rev/f90c0cecba19 Cleanup reftest, jsreftest and wpt file paths in per-test coverage data. r=gbrown,marco
Keywords: checkin-needed
Comment 12•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f90c0cecba19
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
•