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

Issue 1994653004: telemetry: Stop measuring tough_path_rendering_cases chalkboard test after it stops (Closed)

Created:
4 years, 7 months ago by Kimmo Kinnunen
Modified:
4 years, 6 months ago
CC:
chromium-reviews, telemetry-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@fix-chalkboard-recording
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

telemetry: Stop measuring tough_path_rendering_cases chalkboard test after it stops Stop measuring tough_path_rendering_cases chalkboard test-case after the page stops its "benchmark" and presents the results. The page used to wait 20s. On my desktop it runs <10s and on my particular Android device, ~17s. This avoids measuring the page after it stops updating. Committed: https://crrev.com/6a26af29d26f974d63d78f2a29757f0f8f6162f4 Cr-Commit-Position: refs/heads/master@{#399432}

Patch Set 1 #

Total comments: 4

Patch Set 2 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -1 line) Patch
M tools/perf/page_sets/tough_path_rendering_cases.py View 1 1 chunk +3 lines, -1 line 0 comments Download

Messages

Total messages: 15 (5 generated)
Kimmo Kinnunen
I noticed nednguyen@ was concerned about test length. Does it make sense to measure chalkboard ...
4 years, 7 months ago (2016-05-19 08:10:57 UTC) #2
nednguyen
lgtm
4 years, 7 months ago (2016-05-19 15:24:35 UTC) #3
Stephen White
https://codereview.chromium.org/1994653004/diff/1/tools/perf/page_sets/tough_path_rendering_cases.py File tools/perf/page_sets/tough_path_rendering_cases.py (right): https://codereview.chromium.org/1994653004/diff/1/tools/perf/page_sets/tough_path_rendering_cases.py#newcode18 tools/perf/page_sets/tough_path_rendering_cases.py:18: action_runner.tab.WaitForDocumentReadyStateToBeComplete() Hmmm! Note to self: we should probably add ...
4 years, 7 months ago (2016-05-19 15:26:45 UTC) #5
Stephen White
LGTM
4 years, 7 months ago (2016-05-19 15:27:20 UTC) #6
Kimmo Kinnunen
https://codereview.chromium.org/1994653004/diff/1/tools/perf/page_sets/tough_path_rendering_cases.py File tools/perf/page_sets/tough_path_rendering_cases.py (right): https://codereview.chromium.org/1994653004/diff/1/tools/perf/page_sets/tough_path_rendering_cases.py#newcode23 tools/perf/page_sets/tough_path_rendering_cases.py:23: 'document.getElementById("FinalTime").innerText != ""') On 2016/05/19 15:26:41, Stephen White wrote: ...
4 years, 7 months ago (2016-05-20 10:54:43 UTC) #7
Stephen White
https://codereview.chromium.org/1994653004/diff/1/tools/perf/page_sets/tough_path_rendering_cases.py File tools/perf/page_sets/tough_path_rendering_cases.py (right): https://codereview.chromium.org/1994653004/diff/1/tools/perf/page_sets/tough_path_rendering_cases.py#newcode23 tools/perf/page_sets/tough_path_rendering_cases.py:23: 'document.getElementById("FinalTime").innerText != ""') On 2016/05/20 10:54:42, Kimmo Kinnunen wrote: ...
4 years, 7 months ago (2016-05-20 13:25:12 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1994653004/20001
4 years, 6 months ago (2016-06-13 10:43:18 UTC) #11
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 6 months ago (2016-06-13 12:02:58 UTC) #12
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/6a26af29d26f974d63d78f2a29757f0f8f6162f4 Cr-Commit-Position: refs/heads/master@{#399432}
4 years, 6 months ago (2016-06-13 12:04:14 UTC) #14
Zhen Wang
4 years, 6 months ago (2016-06-13 21:55:01 UTC) #15
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2063013002/ by zhenw@chromium.org.

The reason for reverting is: smoothness.tough_path_rendering_cases fails on
Android Nexus5 Perf (1) and Android Nexus7v2 Perf (1) bots.

Seems clear that this CL causes it.

BUG=619716.

Powered by Google App Engine
This is Rietveld 408576698