In archive_layout_test_results, only support archiving to GS.

In archive_layout_test_results, only support archiving to GS.

BUG=655810
Committed: https://chromium.googlesource.com/chromium/tools/build/+/c0b4818f62ae008fa858794ac278ce7f65cdf92d

[qyearsley, 2016-10-13 23:06:29]

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

[Dirk Pranke, 2016-10-13 23:16:15]

Close. LGTM if the diff files are restored.

https://codereview.chromium.org/2414153003/diff/1/scripts/slave/chromium/arch...
File scripts/slave/chromium/archive_layout_test_results.py (left):

https://codereview.chromium.org/2414153003/diff/1/scripts/slave/chromium/arch...
scripts/slave/chromium/archive_layout_test_results.py:91:
name.endswith('-diff.png'))
Why are you deleting these? We need these files.

[qyearsley, 2016-10-13 23:26:42]

https://codereview.chromium.org/2414153003/diff/1/scripts/slave/chromium/arch...
File scripts/slave/chromium/archive_layout_test_results.py (left):

https://codereview.chromium.org/2414153003/diff/1/scripts/slave/chromium/arch...
scripts/slave/chromium/archive_layout_test_results.py:91:
name.endswith('-diff.png'))
On 2016/10/13 at 23:16:15, Dirk Pranke wrote:
> Why are you deleting these? We need these files.

diff_file_list was only used in _ArchiveFullLayoutTestResults, which is only
called if --gs-bucket is not given.

I believe that what's actually happening currently is that the diff files along
with everything else are archived to GS when then layout-test-results directory
is uploaded -- but the diff files are not currently included in the zip file.

[Dirk Pranke, 2016-10-13 23:29:51]

On 2016/10/13 23:26:42, qyearsley wrote:
>
https://codereview.chromium.org/2414153003/diff/1/scripts/slave/chromium/arch...
> File scripts/slave/chromium/archive_layout_test_results.py (left):
> 
>
https://codereview.chromium.org/2414153003/diff/1/scripts/slave/chromium/arch...
> scripts/slave/chromium/archive_layout_test_results.py:91:
> name.endswith('-diff.png'))
> On 2016/10/13 at 23:16:15, Dirk Pranke wrote:
> > Why are you deleting these? We need these files.
> 
> diff_file_list was only used in _ArchiveFullLayoutTestResults, which is only
> called if --gs-bucket is not given.
> 
> I believe that what's actually happening currently is that the diff files
along
> with everything else are archived to GS when then layout-test-results
directory
> is uploaded -- but the diff files are not currently included in the zip file.

Hm. If that's the case, then the zip file is broken and should be fixed.

[qyearsley, 2016-10-17 16:56:18]

On 2016/10/13 at 23:29:51, dpranke wrote:
> On 2016/10/13 23:26:42, qyearsley wrote:
> >
https://codereview.chromium.org/2414153003/diff/1/scripts/slave/chromium/arch...
> > File scripts/slave/chromium/archive_layout_test_results.py (left):
> > 
> >
https://codereview.chromium.org/2414153003/diff/1/scripts/slave/chromium/arch...
> > scripts/slave/chromium/archive_layout_test_results.py:91:
> > name.endswith('-diff.png'))
> > On 2016/10/13 at 23:16:15, Dirk Pranke wrote:
> > > Why are you deleting these? We need these files.
> > 
> > diff_file_list was only used in _ArchiveFullLayoutTestResults, which is only
> > called if --gs-bucket is not given.
> > 
> > I believe that what's actually happening currently is that the diff files
along
> > with everything else are archived to GS when then layout-test-results
directory
> > is uploaded -- but the diff files are not currently included in the zip
file.
> 
> Hm. If that's the case, then the zip file is broken and should be fixed.

Confirmed that diff files are in the uploaded directory but not the zip file:
For example, there is a diff file uploaded at:
 
https://storage.googleapis.com/chromium-layout-test-archives/mac10_10_blink_r...
But in
 
https://storage.googleapis.com/chromium-layout-test-archives/mac10_10_blink_r...
at layout-test-results/animations/3d/ there is only the -actual.png file, but no
diff file.

Filed bug http://crbug.com/656684.

In the interest of keeping each CL focused on one thing, does it sound OK to
submit this CL as-is (which is not expected to change behavior), and then submit
a fix for http://crbug.com/656684 which makes it so that the zip file contains
diff files as well?

[Dirk Pranke, 2016-10-17 17:16:08]

On 2016/10/17 16:56:18, qyearsley wrote:
> In the interest of keeping each CL focused on one thing, does it sound OK to
> submit this CL as-is (which is not expected to change behavior), and then
submit
> a fix for http://crbug.com/656684 which makes it so that the zip file contains
> diff files as well?

That's fine.

[qyearsley, 2016-10-17 17:20:59]

The CQ bit was checked by qyearsley@chromium.org

[qyearsley, 2016-10-17 17:21:00]

The patchset sent to the CQ was uploaded after l-g-t-m from dpranke@chromium.org
Link to the patchset: https://codereview.chromium.org/2414153003/#ps20001
(title: "Rebased")

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

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 17:31:07]

Description was changed from

==========
In archive_layout_test_results, only support archiving to GS.

BUG=655810
==========

to

==========
In archive_layout_test_results, only support archiving to GS.

BUG=655810

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

[commit-bot: I haz the power, 2016-10-17 17:31:08]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/tools/build/+/c0b4818f62ae008fa858...