Closed Bug 1367763 Opened 8 years ago Closed 7 years ago

Use grcov immediately after coverage test run

Categories

(Testing :: Code Coverage, enhancement)

Product:
Testing
Component:
Code Coverage
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox56 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla56
Tracking Flags:
Tracking Status
firefox56 --- fixed

People

(Reporter: ekyle, Assigned: sparky)

References

Details

Attachments

(1 file)

Bug 1367763 - Run grcov after code coverage collection on linux64-ccov.
59 bytes, text/x-review-board-request
jmaher
: review+
Details
grcov, and all other coverage tools, require the repository and the symlinks to produce source file names in the coverage reports. Maybe we should run grcov immediately after the tests so it can use the test environment, rather than building its own on a different machine.
There are four steps that I think this task splits into: 1.* Download GCNO archive onto the test machines. 2.** Install grcov on the test machines. (Using the pre-action listener I imagine, like running gcov?). 3.** Run grcov on the test machines. (Using the post-action listener?). * Could be done at the same time as 2 and 3. ** These could be done differently and at the same time at the end of the test-chunk. 4. As marco said on IRC, rather than uploading another artifact, we can use chris' work from this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1349640 This would involve either modifying grcov to use the mappings, or with a script at the end of grcov's run. The former would require less processing time than the latter. Any thoughts on this breakdown? Am I missing anything?
The summary looks correct to me. I'm not sure what we usually do when installing external tools. Do we clone their source code and build them? I can take care of adding support to grcov for using the mapping introduced in bug 1349640.
it is best to put binaries up on tooltool, but we can |git clone| and run code; the test machines might not have all the libraries/tools for building, so be careful.
(In reply to Joel Maher ( :jmaher) from comment #3) > it is best to put binaries up on tooltool, but we can |git clone| and run > code; the test machines might not have all the libraries/tools for building, > so be careful. If we decide to build, the only dependency should be the Rust compiler.
(In reply to Marco Castelluccio [:marco] from comment #2) > I can take care of adding support to grcov for using the mapping introduced > in bug 1349640. This is now done.
Great! I'm looking into how we can integrate it but this will depend on where we want to build. Do you have any preferences? I am thinking that using a pre-built version on tooltool will be the easier way.
Flags: needinfo?(mcastelluccio)
Using a pre-built version sounds fine to me, you can download one from https://github.com/marco-c/grcov/releases (I suggest the grcov-linux-standalone-x86_64.tar.bz2 one, as it doesn't depend on a specific glibc version).
Flags: needinfo?(mcastelluccio)
I don't have sufficient permission to get a token for upload for RelengAPI. Marco, would you be able to do this? I was following the instructions from here: https://wiki.mozilla.org/ReleaseEngineering/Applications/Tooltool
Flags: needinfo?(mcastelluccio)
Depends on: 1368702
Uploaded!
Flags: needinfo?(mcastelluccio)
Thanks Marco! I should have something going befote the end of the day...assuming my computer cooperates with me.
(In reply to Greg Mierzwinski [:gmierz] from comment #10) > Thanks Marco! I should have something going befote the end of the > day...assuming my computer cooperates with me. Feel free to ping me should you need any help with grcov.
Depends on: 1369523
Here we go: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dcec85b995d7677a1bd5aadd701aead0239d1e05 It's running, and we are gathering output. The output needs to be reformatted though I think as it doesn't have the standard LCOV look to it (there are no new lines).
(In reply to Greg Mierzwinski [:gmierz] from comment #12) > Here we go: > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=dcec85b995d7677a1bd5aadd701aead0239d1e05 > > It's running, and we are gathering output. The output needs to be > reformatted though I think as it doesn't have the standard LCOV look to it > (there are no new lines). Are you on Windows? Perhaps the issue is that they are Unix newlines.
Woohoo! :) Thank you for catching that Marco, it definitely uses linux new lines. So, everything is working great then from what I've seen so far. I'll analyze the data a bit tomorrow to see if there are any problems in it. After that, I'll submit the patch for review.
Here's a test run with all test suites running: https://treeherder.mozilla.org/#/jobs?repo=try&revision=57727d3955c2c78197ed4290db919cef0ed1c023 Also, looking at the time the test suites took before and after this patch, there seems to be no difference (or very little).
Comment on attachment 8873888 [details] Bug 1367763 - Run grcov after code coverage collection on linux64-ccov. https://reviewboard.mozilla.org/r/145282/#review149244 just the one issue ::: testing/mozharness/mozharness/mozilla/testing/codecoverage.py:125 (Diff revision 1) > self.run_command(command, cwd=rel_topsrcdir) > + > + # Run grcov on the zipped .gcno and .gcda files. > + grcov_command = [os.path.join(self.grcov_dir, 'grcov'), '-t', 'lcov' , '-z', \ > + '-s', rel_topsrcdir, '-p', '/home/worker/workspace/build/src/', \ > + '--path-mapping', os.path.join(self.grcov_dir, 'linked-files-map.json'), \ I see /home/worker/workspace/build/src/ hardcoded, do we need that hardcoded?
Attachment #8873888 - Flags: review?(jmaher) → review-
(In reply to Joel Maher ( :jmaher) from comment #17) > Comment on attachment 8873888 [details] > Bug 1367763 - Run grcov after code coverage collection on linux64-ccov. > > https://reviewboard.mozilla.org/r/145282/#review149244 > > just the one issue > > ::: testing/mozharness/mozharness/mozilla/testing/codecoverage.py:125 > (Diff revision 1) > > self.run_command(command, cwd=rel_topsrcdir) > > + > > + # Run grcov on the zipped .gcno and .gcda files. > > + grcov_command = [os.path.join(self.grcov_dir, 'grcov'), '-t', 'lcov' , '-z', \ > > + '-s', rel_topsrcdir, '-p', '/home/worker/workspace/build/src/', \ > > + '--path-mapping', os.path.join(self.grcov_dir, 'linked-files-map.json'), \ > > I see /home/worker/workspace/build/src/ hardcoded, do we need that hardcoded? The '-p' argument is needed for removing the prefix from the paths (the paths in the gcno/gcda files all start with "/home/worker/workspace/build/src/"). Is there a way to get the prefix without hardcoding? Perhaps |os.cwd()| gives the same result, when running on a test machine?
Thanks for the information Marco, we can use rel_topsrcdir, or one of the directories in dirs. Is the prefix path ever different between the gcnos and gcdas?
(In reply to Greg Mierzwinski [:gmierz] from comment #19) > Thanks for the information Marco, we can use rel_topsrcdir, or one of the > directories in dirs. > > Is the prefix path ever different between the gcnos and gcdas? So far, I've always seen the same prefix in both gcnos and gcdas.
Ok, I'll be opening a bug to remind us to write some code that can check if the two prefixes are the same or different for us. I seem to have stumbled upon an interesting problem where grcov is failing for some of the test chunks: https://treeherder.mozilla.org/#/jobs?repo=try&author=gmierz2@outlook.com&selectedJob=104091043 What is odd though is that it is not failing on all the test chunks of a given suite. For example, the link above shows that 'code-coverage-grcov.zip' is 230 bytes, but chunk 4 is fine. (Most of mochitest-plain though is broken). The error output says that gcov was not successfully executed. I'm hoping that it's because the path given to '-s' is incorrect and I'm running a test to see this now: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5c280cb5ec0b58315a691dd25cf1f8689c1d41fd
Depends on: 1369937
It seems like the tests which were passing managed to find enough information from within the gcov_dir path which contains the gcda's but doesn't contain any source code. Interestingly enough, it looks like the directory structure was enough to let them pass. I'm running another test now to look at the directories that we have and see if there even is a src directory because it seems like it doesn't exist. (Either it's just not there, or it was deleted before we could get to it). Here's a link to the test: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3fcbcc1315b0644175e262ec0467cff3ea35763d Marco, would you have any ideas about this problem? I'm wondering if it's gcov itself that failed to execute properly and caused grcov to fail.
Flags: needinfo?(mcastelluccio)
Also, the test in comment 21 checks directories before runtests is running. The test in comment 22 checks directories after it's run.
I think the problem is that the 'gcov' binary on test machines is not the same version as the 'gcov' binary in build machines. Build machines use a newer version of GCC. We should use the same version in test machines.
Flags: needinfo?(mcastelluccio)
Ok, thanks for looking into this Marco, I'm glad it's not grcov that broke. I'll look into getting this fixed.
From my investigation, it's actually the other way around. The build machine uses the older version, at 4.9.4, and the test machine uses the newer version at 5.4.0.
(In reply to Greg Mierzwinski [:gmierz] from comment #26) > From my investigation, it's actually the other way around. The build machine > uses the older version, at 4.9.4, and the test machine uses the newer > version at 5.4.0. Correction, the GPU test suite uses a newer version than all the others at 5.4.0, the others use an earlier version at 4.6.3.
So, to get this working we will have to migrate all the other test suites to Ubuntu 16.04. We can do it in another way, but the migration would override that change anyway. Here's a test run that shows that mochitest-plain simply needs to be run with ubuntu 16.04 to get the newer gcov binary: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0f300a252772a0cb2f5e2c9644655d6b4962c024 If you look at the uploaded grcov files, they now have data in them. Mochitest-chrome is also failing in this way because it hasn't been migrated yet.
Depends on: 1319782, 1319804
Couldn't GCC be downloaded via tooltool? In https://github.com/marco-c/grcov/issues/20#issue-223478909, Maja said the build task uses tooltool to fetch gcc 4.9.4.
Yes, we can do that. But (imo) it's going to be redundant work since the linux64 migration will fix the problem, and there's only a few test left to fix there. Would you rather do it through tooltool for the time being? We could do it at the same time grcov is downloaded and installed.
(In reply to Greg Mierzwinski [:gmierz] from comment #30) > Yes, we can do that. > > But (imo) it's going to be redundant work since the linux64 migration will > fix the problem, and there's only a few test left to fix there. Would you > rather do it through tooltool for the time being? We could do it at the same > time grcov is downloaded and installed. Do you know when the migration will happen? If it's going to happen very soon, then I think we can wait.
it is fixing 3 tests that are highly intermittent, possibly 1 more week and we are good if Greg can focus on those tests.
Blocks: 1373956
Comment on attachment 8873888 [details] Bug 1367763 - Run grcov after code coverage collection on linux64-ccov. https://reviewboard.mozilla.org/r/145282/#review155260 thanks, this is looking really good.
Attachment #8873888 - Flags: review?(jmaher) → review+
Comment on attachment 8873888 [details] Bug 1367763 - Run grcov after code coverage collection on linux64-ccov. https://reviewboard.mozilla.org/r/145282/#review155302 ::: testing/mozharness/mozharness/mozilla/testing/codecoverage.py:133 (Diff revision 2) > + # GRCOV post-processing > + # Download the gcno fom the build machine. > + self.download_file(self.url_to_gcno, file_name=None, parent_dir=self.grcov_dir) > + > + # Run grcov on the zipped .gcno and .gcda files. > + grcov_command = [os.path.join(self.grcov_dir, 'grcov'), '-t', 'lcov' , '-z', \ With the latest version of grcov, `-z` won't be needed anymore, and you will be able to directly pass the ZIP containing the GCNO files and the directory to the GCDA files (this will avoid grcov decompressing the GCDA ZIP needlessly). Let's open a new bug for this, since I also need to upload a new version to tooltool first. ::: testing/mozharness/mozharness/mozilla/testing/codecoverage.py:139 (Diff revision 2) > + os.path.join(self.grcov_dir, 'target.code-coverage-gcno.zip'), file_path_gcda] > + > + # 'grcov_output' will be a tuple, the first variable is the path to the lcov output, > + # the other is the path to the standard error output. > + grcov_output = self.get_output_from_command(grcov_command, cwd=self.grcov_dir, \ > + silent=True, tmpfile_base_path=os.path.join(self.grcov_dir, 'grcov_lcov_output'), \ Nit: since the output is a LCOV info file, we could set the extension to ".info".
Attachment #8873888 - Flags: review?(mcastelluccio) → review+
Comment on attachment 8873888 [details] Bug 1367763 - Run grcov after code coverage collection on linux64-ccov. https://reviewboard.mozilla.org/r/145282/#review155302 > With the latest version of grcov, `-z` won't be needed anymore, and you will be able to directly pass the ZIP containing the GCNO files and the directory to the GCDA files (this will avoid grcov decompressing the GCDA ZIP needlessly). > > Let's open a new bug for this, since I also need to upload a new version to tooltool first. Ok, sounds good! > Nit: since the output is a LCOV info file, we could set the extension to ".info". Fixed in latest revision.
Blocks: 1374663
Comment 38 has the latest try push with the changes that were made in that latest revision. Also, bug 1374663 is for the follow-up work that needs to be done.
Comment 42 has the try run with the latest changes and this patch is good to go.
Pushed by jmaher@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e8b9b9cabc8c Run grcov after code coverage collection on linux64-ccov. r=jmaher,marco
https://hg.mozilla.org/mozilla-central/rev/e8b9b9cabc8c
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Assignee: nobody → gmierz2
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: