Archive retry summary for layout tests along with layout test results.

Archive retry summary for layout tests along with layout test results.

Motivation: For `webkit-patch rebaseline-cl`, I want to be able to
easily get a list of layout tests that failed with the patch in a try
job but not without the patch. This information is available in the retry
summary step after running without the patch. But, the retry summary is
not currently stored anywhere that is convenient to fetch.

If the retry summary is stored along with the results for the first
run of the layout tests, then it will be simple to fetch and use when
deciding what layout tests to rebasline.

BUG=642980
Committed: https://chromium.googlesource.com/chromium/tools/build/+/3f789ffa3eec23b6e30036f41bc10d25658f45bd

[qyearsley, 2016-10-14 00:16:15]

qyearsley@chromium.org changed reviewers:
+ dpranke@chromium.org, iannucci@chromium.org, wkorman@chromium.org

[qyearsley, 2016-10-14 00:16:15]

Hi!

Dirk, Walter: Does this seem like a good approach in general for making it so
that webkit-patch rebaseline-cl can get the list of tests that fail with a patch
but not without?

Robbie: Could you quickly look and see if this way of passing JSON to a script
from a recipe seems OK? I think it might be encouraged to use json.input from
the json recipe module, but I'm not sure.

This is a first shot, I think that this CL could definitely be improved before
submitting...

[qyearsley, 2016-10-14 20:43:19]

The CQ bit was checked by qyearsley@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-14 20:43:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-14 21:21:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-14 21:21:07]

Dry run: Try jobs failed on following builders:
  Build Presubmit on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/31dddae924f64f10)

[qyearsley, 2016-10-14 21:30:39]

The CQ bit was checked by qyearsley@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-14 21:30:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-14 21:40:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-14 21:40:35]

Dry run: This issue passed the CQ dry run.

[Dirk Pranke, 2016-10-15 22:34:36]

https://codereview.chromium.org/2421733002/diff/40001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/test_utils/api.py (right):

https://codereview.chromium.org/2421733002/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/test_utils/api.py:210: '--retry-summary',
self.m.json.dumps(retry_summary),
Passing this as a command line argument seems awkward; I'd write it to a file
instead.

[qyearsley, 2016-10-17 20:21:46]

On 2016/10/15 at 22:34:36, dpranke wrote:
>
https://codereview.chromium.org/2421733002/diff/40001/scripts/slave/recipe_mo...
> File scripts/slave/recipe_modules/test_utils/api.py (right):
> 
>
https://codereview.chromium.org/2421733002/diff/40001/scripts/slave/recipe_mo...
> scripts/slave/recipe_modules/test_utils/api.py:210: '--retry-summary',
self.m.json.dumps(retry_summary),
> Passing this as a command line argument seems awkward; I'd write it to a file
instead.

Aye, I thought so; now in the latest patch it uses the input method of the json
recipe module to write to a temporary file and pass that.

https://codereview.chromium.org/2421733002/diff/60001/scripts/slave/chromium/...
File scripts/slave/chromium/archive_layout_test_retry_summary.py (right):

https://codereview.chromium.org/2421733002/diff/60001/scripts/slave/chromium/...
scripts/slave/chromium/archive_layout_test_retry_summary.py:34:
os.path.join(gs_base, 'retry_summary.json'))
Alternately, instead of moving it after uploading, we could rename it locally
first (possibly making a temp dir to do this?) - does one way or the other sound
better?

[Dirk Pranke, 2016-10-17 21:29:26]

https://codereview.chromium.org/2421733002/diff/60001/scripts/slave/chromium/...
File scripts/slave/chromium/archive_layout_test_retry_summary.py (right):

https://codereview.chromium.org/2421733002/diff/60001/scripts/slave/chromium/...
scripts/slave/chromium/archive_layout_test_retry_summary.py:34:
os.path.join(gs_base, 'retry_summary.json'))
On 2016/10/17 20:21:46, qyearsley wrote:
> Alternately, instead of moving it after uploading, we could rename it locally
> first (possibly making a temp dir to do this?) - does one way or the other
sound
> better?

Why are you moving instead of copying? I would be worried that a move would
confuse the ownership issue (and hence the recipe might try to delete a file
that is no longer there)?

Otherwise, I don't think it matters too much one way or another.

[qyearsley, 2016-10-17 21:43:52]

https://codereview.chromium.org/2421733002/diff/60001/scripts/slave/chromium/...
File scripts/slave/chromium/archive_layout_test_retry_summary.py (right):

https://codereview.chromium.org/2421733002/diff/60001/scripts/slave/chromium/...
scripts/slave/chromium/archive_layout_test_retry_summary.py:34:
os.path.join(gs_base, 'retry_summary.json'))
On 2016/10/17 at 21:29:26, Dirk Pranke wrote:
> On 2016/10/17 20:21:46, qyearsley wrote:
> > Alternately, instead of moving it after uploading, we could rename it
locally
> > first (possibly making a temp dir to do this?) - does one way or the other
sound
> > better?
> 
> Why are you moving instead of copying? I would be worried that a move would
confuse the ownership issue (and hence the recipe might try to delete a file
that is no longer there)?
> 
> Otherwise, I don't think it matters too much one way or another.

The reason for renaming the file here is that the filename given in
args.retry_summary_json could be anything and we want it to always have the same
filename in GS, but we can't control this in the call to GSUtilCopyFile; the
destination file name for slave_utils.GSUtilCopyFile is always the same as the
local file name.

Are you suggesting copying the file somewhere locally, e.g.
"/<some-temp-dir>/retry_summary.json", before uploading?

[Dirk Pranke, 2016-10-17 21:55:33]

https://codereview.chromium.org/2421733002/diff/60001/scripts/slave/chromium/...
File scripts/slave/chromium/archive_layout_test_retry_summary.py (right):

https://codereview.chromium.org/2421733002/diff/60001/scripts/slave/chromium/...
scripts/slave/chromium/archive_layout_test_retry_summary.py:34:
os.path.join(gs_base, 'retry_summary.json'))
On 2016/10/17 21:43:52, qyearsley wrote:
> On 2016/10/17 at 21:29:26, Dirk Pranke wrote:
> > On 2016/10/17 20:21:46, qyearsley wrote:
> > > Alternately, instead of moving it after uploading, we could rename it
> locally
> > > first (possibly making a temp dir to do this?) - does one way or the other
> sound
> > > better?
> > 
> > Why are you moving instead of copying? I would be worried that a move would
> confuse the ownership issue (and hence the recipe might try to delete a file
> that is no longer there)?
> > 
> > Otherwise, I don't think it matters too much one way or another.
> 
> The reason for renaming the file here is that the filename given in
> args.retry_summary_json could be anything and we want it to always have the
same
> filename in GS, but we can't control this in the call to GSUtilCopyFile; the
> destination file name for slave_utils.GSUtilCopyFile is always the same as the
> local file name.
> 
> Are you suggesting copying the file somewhere locally, e.g.
> "/<some-temp-dir>/retry_summary.json", before uploading?

Oh, sorry, I misread this and you're copying it to the server and then
renaming/moving it. That's fine. I thought you were directly moving (i.e.,
uploading and deleting) the client-side file.

lgtm, then.

[qyearsley, 2016-10-17 23:07:02]

The CQ bit was checked by qyearsley@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-17 23:07:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-17 23:16:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-17 23:16:55]

Dry run: This issue passed the CQ dry run.

[qyearsley, 2016-10-18 01:24:39]

The CQ bit was checked by qyearsley@chromium.org

[commit-bot: I haz the power, 2016-10-18 01:24:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-18 01:35:26]

Description was changed from

==========
Archive retry summary for layout tests along with layout test results.

Motivation: For `webkit-patch rebaseline-cl`, I want to be able to
easily get a list of layout tests that failed with the patch in a try
job but not without the patch. This information is available in the retry
summary step after running without the patch. But, the retry summary is
not currently stored anywhere that is convenient to fetch.

If the retry summary is stored along with the results for the first
run of the layout tests, then it will be simple to fetch and use when
deciding what layout tests to rebasline.

BUG=642980
==========

to

==========
Archive retry summary for layout tests along with layout test results.

Motivation: For `webkit-patch rebaseline-cl`, I want to be able to
easily get a list of layout tests that failed with the patch in a try
job but not without the patch. This information is available in the retry
summary step after running without the patch. But, the retry summary is
not currently stored anywhere that is convenient to fetch.

If the retry summary is stored along with the results for the first
run of the layout tests, then it will be simple to fetch and use when
deciding what layout tests to rebasline.

BUG=642980

Committed:
https://chromium.googlesource.com/chromium/tools/build/+/3f789ffa3eec23b6e300...
==========

[commit-bot: I haz the power, 2016-10-18 01:35:27]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/tools/build/+/3f789ffa3eec23b6e300...