Closed Bug 1428296 Opened 2 years ago Closed 2 years ago

Package coverage artifacts silently

Categories

(Testing :: Code Coverage, enhancement)

enhancement
Not set

Tracking

(firefox60 fixed)

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: marco, Assigned: u602518, Mentored)

Details

(Keywords: good-first-bug, Whiteboard: [lang=py])

Attachments

(1 file)

More than 50% of the logs in code coverage tests is taken by 'zip' listing the files it is adding.

We should make it package files silently.

The code performing the packaging is here: https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/testing/codecoverage.py.
Whiteboard: [lang=py]
Hello I would like to work on this issue .
Hello Ashish, there's Vinicius that has already expressed interest in working on this bug. Let's give him a bit of time first and then, if he changes his mind, you can pick this up.
In the meantime, there are other Python good first bugs that you could tackle, feel free to send me an email and I can give you a list of examples :)
(In reply to Marco Castelluccio [:marco] from comment #2)
> Hello Ashish, there's Vinicius that has already expressed interest in
> working on this bug. Let's give him a bit of time first and then, if he
> changes his mind, you can pick this up.
> In the meantime, there are other Python good first bugs that you could
> tackle, feel free to send me an email and I can give you a list of examples
> :)
Thanks, Marco, I have sent you an email.Requesting a list of python good-first-bug.
My EmailID:-ashish1500616@gmail.com
Comment on attachment 8945058 [details]
Bug 1428296 - Compress coverage artifacts silently.

https://reviewboard.mozilla.org/r/215272/#review220842

It looks good overall, but there's the bootstrap.py file that shouldn't be committed and a couple of other 'zip' command calls that should be made silent too.

::: bootstrap.py:1
(Diff revision 1)
> +#!/usr/bin/env python

This file should not be added.

::: testing/mozharness/mozharness/mozilla/testing/codecoverage.py:139
(Diff revision 1)
>          if self.jsd_code_coverage_enabled:
>              # Setup the command for compression
>              dirs = self.query_abs_dirs()
>              jsdcov_dir = dirs['abs_blob_upload_dir']
>              zipFile = os.path.join(jsdcov_dir, "jsdcov_artifacts.zip")
>              command = ["zip", "-r", zipFile, ".", "-i", "jscov*.json"]

This should be silent too.

::: testing/mozharness/mozharness/mozilla/testing/codecoverage.py:200
(Diff revision 1)
>              output_file_name = 'grcov_lcov_output.info'
>              shutil.move(grcov_output, os.path.join(self.grcov_dir, output_file_name))
>  
>              # Zip the grcov output and upload it.
>              self.run_command(
>                  ['zip', os.path.join(dirs['abs_blob_upload_dir'], 'code-coverage-grcov.zip'), output_file_name],

This should be silent too.
Attachment #8945058 - Flags: review?(mcastelluccio) → review-
It looks good now, but the changes are split between two commits. It's fine in general when you have multiple commits implementing self-contained changes, but in this case it's a single logic change so it should be in a single commit.

You can use "hg histedit" to merge the two commits. Please also make sure the resulting commit does not contain bootstrap.py and remove it if it does.
Flags: needinfo?(viniciuscosta0197)
Comment on attachment 8945058 [details]
Bug 1428296 - Compress coverage artifacts silently.

https://reviewboard.mozilla.org/r/215272/#review221016

Thanks! It looks good!
I'm going to spin a try build to see that everything is working correctly and then land the patch.
Attachment #8945058 - Flags: review?(mcastelluccio) → review+
Comment on attachment 8945058 [details]
Bug 1428296 - Compress coverage artifacts silently.

https://reviewboard.mozilla.org/r/215272/#review221020

::: commit-message-c5461:1
(Diff revision 4)
> +Bug 1428296 - zip silently in run_command. r?marco

Only a small required change for the commit message. As it is, it isn't descriptive enough.
You should make it clear what the patch is doing. Something like "Compress coverage artifacts silently" or "zip coverage artifacts silently" or "Make coverage artifacts compression silent".
Comment on attachment 8945058 [details]
Bug 1428296 - Compress coverage artifacts silently.

https://reviewboard.mozilla.org/r/215272/#review221048
Pushed by mcastelluccio@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c5b1c820d70e
Compress coverage artifacts silently. r=marco
Assignee: nobody → viniciuscosta0197
Status: NEW → ASSIGNED
Flags: needinfo?(viniciuscosta0197)
https://hg.mozilla.org/mozilla-central/rev/c5b1c820d70e
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.