Closed Bug 1400979 Opened 2 years ago Closed 2 years ago

test-verify fails on Linux x64 CCov when no tests run

Categories

(Testing :: General, defect)

defect
Not set

Tracking

(firefox57 fixed)

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: gbrown, Assigned: gbrown)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

test-verify runs fine on Linux x64 CCov when supported test files have been modified and test verification is triggered. However, when no test files have been modified, test-verify completes without running any tests (as I would expect) and that seems to trigger a failure in the coverage code, like:

https://public-artifacts.taskcluster.net/YSjFxagrQ3OzywXTkL71yQ/0/public/logs/live_backing.log

[task 2017-09-18T18:33:50.138Z] 18:33:50     INFO - Running post-action listener: _package_coverage_data
[task 2017-09-18T18:33:50.138Z] 18:33:50    FATAL - Could not find relative topsrcdir in code coverage data!
[task 2017-09-18T18:33:50.139Z] 18:33:50    FATAL - Running post_fatal callback...
[task 2017-09-18T18:33:50.139Z] 18:33:50    FATAL - Exiting -1
:gmierz - I'm not very familiar with ccov and it looks like you might be the expert? Would appreciate your thoughts on how best to handle this situation.

My test-verify (TV) test task is designed to sometimes run no tests. It checks, in the mozharness script, to see if the changeset has modified any test files that it cares about; if the changeset has modified such files, tests are run, but if not, then the script just exits without running any tests. When no tests are run on ccov, I run into this error. I suppose ccov expects some tests to be run / the browser to have been started at least once / something like that?

If nothing else, I can exclude test-verify from running on ccov, but I wonder if there is a better solution.
Flags: needinfo?(gmierz2)
Hi :gbrown, I'm happy to help! This error comes from here: https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/testing/codecoverage.py?q=Could+not+find+relative+topsrcdir&redirect_type=single#111

The code here (in the for loop above) is trying to find the top level source directory from a given temporary directory that stores the coverage '.gcda' files. However, because no gcda files were created, there was no top-level source directory created also.

My first idea to get around this problem would be to add a command at [1] and use that on line 111 to check if it's OK that there is no dir and then exit early. If the TV test task is defined in something like [2], then you could add the command to the mozharness extra-options, but only use it on linux64-ccov. Or, you can add it through the transform given here at [3]. What do you think? 

So far, this is the only way that I can think of to get around this problem. We could just skip that part and exit early if there is no directory regardless of the flag, but this error has come in handy at least twice (from what I recall) so I'm a little hesitant on removing it.


[1]: https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/testing/codecoverage.py?q=Could+not+find+relative+topsrcdir&redirect_type=single#16
[2]: https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/test/tests.yml
[3]: https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/transforms/tests.py#624
Flags: needinfo?(gmierz2)
Thanks for your advice. I certainly could add a new option as you suggest, but I wonder if this would be reasonable instead: use the existing --disable-ccov-upload.

test-verify runs only when tests are modified, and then only runs the modified tests -- typically just one, or a few individual tests -- and it runs those modified tests over an over. It's so different from our regular testing, I wonder if we really want to collect the data from test-verify?

Either way is fine with me: Let me know what you think is best.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=00eb2bafb7e6d08645cca4522e3773d5c94b5f62&filter-tier=1&filter-tier=2&filter-tier=3
Attachment #8909992 - Flags: review?(gmierz2)
Comment on attachment 8909992 [details] [diff] [review]
use --disable-ccov-upload for test-verify

Review of attachment 8909992 [details] [diff] [review]:
-----------------------------------------------------------------

This will be fine because it sounds lile you're running tests that are in other test suites that are being run with code coverage any way.

I'm just going to ask for a second opinion from jmaher in case we'd like coverage from this for some reason that I'm missing.

:jmaher, I think it's perfectly fine if we disable code coverage on this test suite, do you have any thoughts?
Attachment #8909992 - Flags: review?(gmierz2) → review+
Comment 5
Flags: needinfo?(jmaher)
:gbrown, if you want to, you could even disable this test suite from running on ccov completely through the transform I gave in comment 3 link [3] (or another way if you know of one) which I think may be the best route with the advantage of conserving some resources.
lgtm, lets land this!
Flags: needinfo?(jmaher)
(In reply to Greg Mierzwinski [:gmierz] from comment #7)
> :gbrown, if you want to, you could even disable this test suite from running
> on ccov completely through the transform I gave in comment 3 link [3] (or
> another way if you know of one) which I think may be the best route with the
> advantage of conserving some resources.

I think you are right: If we are not going to collect the data, why run test-verify on ccov at all?

I think this is the best way to do that -- similar to not running awsy on devedition.
Attachment #8909992 - Attachment is obsolete: true
Attachment #8910299 - Flags: review?(gmierz2)
Comment on attachment 8910299 [details] [diff] [review]
do not run test-verify on ccov

Review of attachment 8910299 [details] [diff] [review]:
-----------------------------------------------------------------

This is much nicer and simpler than what I was thinking. Thanks!
Attachment #8910299 - Flags: review?(gmierz2) → review+
Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/454abbc380ed
Do not run test-verify on linux64-ccov; r=gmierz
:gbrown, sorry for the last minute feedback, but I just realized this will most likely break on the mozilla-central branch because we mass-set the run on projects flag in the transform. Do you want to wait and see if this does indeed happen?
Sigh. 

I don't think it will break...it will just be ineffective, because the transform will over-write the test's 'run-on-projects'...right?
Yes, that's what I expect to happen also, I'm very sorry I didn't catch this before. :(

IMO, the best thing to do here would be to disable the test suite through the transform or some other way that I can't think of.
I think this should work.

Let's leave the original patch, let it land since it does no harm, and add this on top.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=299bf0eefaf3dc997470a3fe7a0d5041fcd310ed&filter-tier=1&filter-tier=2&filter-tier=3
Attachment #8910501 - Flags: review?(gmierz2)
Keywords: leave-open
Comment on attachment 8910501 [details] [diff] [review]
do not run test-verify on ccov, really

Review of attachment 8910501 [details] [diff] [review]:
-----------------------------------------------------------------

This is perfect now, thank you!
Attachment #8910501 - Flags: review?(gmierz2) → review+
Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e024769dac36
Follow-up: Skip test-verify on linux64-ccov; r=gmierz
Thank you for all your help Greg!
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/454abbc380ed
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Pushed by kwierso@gmail.com:
https://hg.mozilla.org/mozilla-central/rev/f8dd3f21e434
Follow-up: Skip test-verify on linux64-ccov; r=gmierz a=merge
You need to log in before you can comment on or make changes to this bug.