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

Issue 2074183002: [telemetry] Try to print browser logs and capture a screenshot upon story failure (Closed)

Created:
4 years, 6 months ago by petrcermak
Modified:
4 years, 6 months ago
Reviewers:
nednguyen
CC:
catapult-reviews_chromium.org, telemetry-reviews_chromium.org
Base URL:
git@github.com:catapult-project/catapult.git@master
Target Ref:
refs/heads/master
Project:
catapult
Visibility:
Public.

Description

[telemetry] Try to print browser logs and capture a screenshot upon story failure This patch modifies story_runner to try to: 1. print browser standard output, 2. print browser log and 3. capture a screenshot upon failures in _RunStoryAndProcessErrorIfNeeded. BUG=chromium:612140 Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/2c7c14263b4e48f3899aefff47b8c8acb2d35dee

Patch Set 1 #

Patch Set 2 : Generalize #

Total comments: 6

Patch Set 3 : Address comments #

Total comments: 2

Patch Set 4 : Add tests #

Patch Set 5 : Add DumpStateUponFailure to _FakeBrowser #

Unified diffs Side-by-side diffs Delta from patch set Stats (+302 lines, -28 lines) Patch
M telemetry/telemetry/android/shared_android_state.py View 1 2 3 1 chunk +4 lines, -0 lines 0 comments Download
M telemetry/telemetry/internal/browser/browser.py View 1 chunk +15 lines, -0 lines 0 comments Download
M telemetry/telemetry/internal/story_runner.py View 1 2 3 chunks +5 lines, -5 lines 0 comments Download
M telemetry/telemetry/internal/story_runner_unittest.py View 1 2 3 8 chunks +249 lines, -1 line 0 comments Download
M telemetry/telemetry/page/shared_page_state.py View 1 2 chunks +13 lines, -8 lines 0 comments Download
M telemetry/telemetry/story/shared_state.py View 1 2 3 1 chunk +8 lines, -0 lines 0 comments Download
M telemetry/telemetry/testing/browser_test_case.py View 1 chunk +5 lines, -14 lines 0 comments Download
M telemetry/telemetry/testing/fakes/__init__.py View 1 2 3 4 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 20 (9 generated)
petrcermak
PTAL. Thanks, Petr
4 years, 6 months ago (2016-06-17 15:01:23 UTC) #3
nednguyen
Seems like a good change to have. Though I have some comment https://codereview.chromium.org/2074183002/diff/20001/telemetry/telemetry/internal/story_runner.py File telemetry/telemetry/internal/story_runner.py ...
4 years, 6 months ago (2016-06-17 15:54:33 UTC) #5
petrcermak
Thanks for your comments. PTAL. Petr https://codereview.chromium.org/2074183002/diff/20001/telemetry/telemetry/internal/story_runner.py File telemetry/telemetry/internal/story_runner.py (right): https://codereview.chromium.org/2074183002/diff/20001/telemetry/telemetry/internal/story_runner.py#newcode113 telemetry/telemetry/internal/story_runner.py:113: state.DumpStateUponFailure(story, results) On ...
4 years, 6 months ago (2016-06-17 16:23:44 UTC) #6
nednguyen
lg2me. Can you add unittest to story_runner_unittest? This control flow is tricky & can easly ...
4 years, 6 months ago (2016-06-17 16:38:57 UTC) #7
petrcermak
Thanks for your comments. PTAL. Thanks, Petr https://codereview.chromium.org/2074183002/diff/40001/telemetry/telemetry/story/shared_state.py File telemetry/telemetry/story/shared_state.py (right): https://codereview.chromium.org/2074183002/diff/40001/telemetry/telemetry/story/shared_state.py#newcode76 telemetry/telemetry/story/shared_state.py:76: pass On ...
4 years, 6 months ago (2016-06-20 13:35:48 UTC) #9
petrcermak
Note: The pre-requirement has landed in Chrome (https://codereview.chromium.org/2085483002/).
4 years, 6 months ago (2016-06-20 15:52:46 UTC) #10
nednguyen
lgtm
4 years, 6 months ago (2016-06-20 16:54:27 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2074183002/80001
4 years, 6 months ago (2016-06-21 07:34:53 UTC) #13
commit-bot: I haz the power
Try jobs failed on following builders: Catapult Windows Tryserver on master.tryserver.client.catapult (JOB_FAILED, https://build.chromium.org/p/tryserver.client.catapult/builders/Catapult%20Windows%20Tryserver/builds/3486)
4 years, 6 months ago (2016-06-21 07:46:25 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2074183002/100001
4 years, 6 months ago (2016-06-21 08:10:09 UTC) #18
commit-bot: I haz the power
4 years, 6 months ago (2016-06-21 08:31:41 UTC) #20
Message was sent while issue was closed.
Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...

Powered by Google App Engine
This is Rietveld 408576698