New issue
Advanced search Search tips

Issue 772740 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac , Fuchsia
Pri: 1
Type: Bug



Sign in to add a comment

upload_to_google_storage_unittests and download_to_google_storage_unittests aren't run by depot_tools presubmit and are hence broken at head

Project Member Reported by thakis@chromium.org, Oct 8 2017

Issue description

depot_tools/PRESUBMIT.py runs `test/*test.py`. The storage tests end in `tests.py` (note trailing s).

So they don't run.

Of course, they bitrot and are now broken.

Fix:
- rename to `test.py`
- delete currently-broken tests to get back to green.
 
Components: -Infra Infra>Client>Chrome
Components: -Infra>Client>Chrome Infra>SDK
Starred, but depot_tools is outside of I>C>C's purview.

Comment 3 by hinoka@chromium.org, Oct 10 2017

Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/83fd81f8a4eef6c8f176a7257662b18cac3e89be

commit 83fd81f8a4eef6c8f176a7257662b18cac3e89be
Author: Ryan Tseng <hinoka@google.com>
Date: Mon Oct 23 18:19:28 2017

download_from_google_storage: Fix tests and rename

the tests haven't been ran by presumbit for a while because of the plural in
the filename.

At some point some post "gsutil cp" file checking happened, which
broke the tests.  This adds a callback to the fake gsutil cp so
that the expect file is copied over.

This also removes "gsutil version" checking from gsutil.py
and just assume that if the file exists, then it's good, which
should shave about 1-2s off of each gsutil.py call.

Bug:  772740 ,776311
Change-Id: I4fef62cfd46a849afed1f095dd6a96069376d13d
Reviewed-on: https://chromium-review.googlesource.com/707758
Commit-Queue: Ryan Tseng <hinoka@chromium.org>
Reviewed-by: Robbie Iannucci <iannucci@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>

[modify] https://crrev.com/83fd81f8a4eef6c8f176a7257662b18cac3e89be/gsutil.py
[rename] https://crrev.com/83fd81f8a4eef6c8f176a7257662b18cac3e89be/tests/download_from_google_storage_unittest.py
[modify] https://crrev.com/83fd81f8a4eef6c8f176a7257662b18cac3e89be/tests/gsutil_test.py
[rename] https://crrev.com/83fd81f8a4eef6c8f176a7257662b18cac3e89be/tests/upload_to_google_storage_unittest.py

Comment 5 by dbehr@chromium.org, Oct 31 2017

It looks like this CL 83fd81f8a4eef6c8f176a7257662b18cac3e89be causes temporary files to be created in depot_tools directory when building Chrome from source in Chrome OS portage and sandbox kills the build.
Also https://canary-chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/753383 improved this.

Ryan, can you confirm if this is fixed now?
Status: Fixed (was: Started)
Yes this should be fixed.

Sign in to add a comment