Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(292)

Issue 106523006: Add options to GPU pixel test to use cloud storage for reference and error images. (Closed)

Created:
7 years ago by Ken Russell (switch to Gerrit)
Modified:
7 years ago
Reviewers:
dtu, bajones
CC:
chromium-reviews, chrome-speed-team+watch_google.com, jam, joi+watch-content_chromium.org, darin-cc_chromium.org, piman+watch_chromium.org, telemetry+watch_chromium.org, Zhenyao Mo, alokp
Visibility:
Public.

Description

Add options to GPU pixel test to use cloud storage for reference and error images. In order to run the pixel tests in isolated mode it's necessary to stop storing the reference and generated images on the local disk. This CL adds command line options to the test to optionally store and retrieve reference images from cloud storage, and to upload any error images to cloud storage directly, instead of using the separate archive_gpu_pixel_test_results step. Once this lands, the GPU recipe will be updated to switch to the new command line arguments, at which point the old on-disk storage code path will be deleted. As part of this: - Added an Exists() method to Telemetry's cloud_storage for convenience, and added an option to make the uploaded file publicly visible (required for the GPU pixel tests' web app). - Added Bitmap.WritePngToFile to supplement the method which takes a file path. BUG=330053 TEST=tested manually locally and verified upload, download, and error paths all work Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=242289

Patch Set 1 #

Total comments: 2

Patch Set 2 : Addressed review feedback. #

Patch Set 3 : Fixed problems with temp files on Windows. Undid changes to Bitmap. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+168 lines, -7 lines) Patch
M content/test/gpu/gpu_tests/pixel.py View 1 2 6 chunks +153 lines, -4 lines 0 comments Download
M tools/telemetry/telemetry/page/cloud_storage.py View 2 chunks +15 lines, -3 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
Ken Russell (switch to Gerrit)
dtu: please review Telemetry changes bajones: please review pixel.py changes Thanks.
7 years ago (2013-12-20 00:27:52 UTC) #1
dtu
telemetry lgtm https://chromiumcodereview.appspot.com/106523006/diff/1/tools/telemetry/telemetry/core/bitmap.py File tools/telemetry/telemetry/core/bitmap.py (right): https://chromiumcodereview.appspot.com/106523006/diff/1/tools/telemetry/telemetry/core/bitmap.py#newcode97 tools/telemetry/telemetry/core/bitmap.py:97: def WritePngToFile(self, f): WritePngToFileObject Also unit test
7 years ago (2013-12-20 00:37:56 UTC) #2
bajones
LGTM
7 years ago (2013-12-20 00:56:20 UTC) #3
Ken Russell (switch to Gerrit)
@dtu: addressed your review feedback. @bajones: addressed your concern mentioned offline about checking the --refimg-cloud-storage-bucket ...
7 years ago (2013-12-20 01:52:51 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kbr@chromium.org/106523006/20001
7 years ago (2013-12-20 01:59:25 UTC) #5
commit-bot: I haz the power
Retried try job too often on win_rel for step(s) telemetry_unittests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&number=238821
7 years ago (2013-12-20 03:29:51 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kbr@chromium.org/106523006/40001
7 years ago (2013-12-21 01:09:09 UTC) #7
commit-bot: I haz the power
Retried try job too often on win_rel for step(s) unit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&number=239455
7 years ago (2013-12-21 02:52:32 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kbr@chromium.org/106523006/40001
7 years ago (2013-12-21 08:45:50 UTC) #9
commit-bot: I haz the power
7 years ago (2013-12-21 17:57:04 UTC) #10
Message was sent while issue was closed.
Change committed as 242289

Powered by Google App Engine
This is Rietveld 408576698