New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 751346 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 0
Type: Bug



Sign in to add a comment

run_telemetry_benchmark_as_googletest is broken, affecting the whole perf waterfall

Project Member Reported by nedngu...@google.com, Aug 2 2017

Issue description

Log:

https://chromium-swarm.appspot.com/task?id=37b7e0f3574be110&refresh=10

View result at file:///b/swarming/w/itEa7DxY/tmp9Cn6Owtelemetry/results-chart.json
View result at file:///b/swarming/w/itEa7DxY/tmp9Cn6Owtelemetry/test-results.json
(INFO) 2017-08-01 18:31:39,992 cmd_helper._ValidateAndLogCommand:158  [host]> /b/swarming/w/ir/third_party/catapult/devil/bin/deps/linux2/x86_64/bin/adb -s 01e15cf0a25daa8b shell '( dumpsys battery );echo %$?'
(INFO) 2017-08-01 18:31:40,123 android_power_monitor_controller._ReenableChargingIfNeeded:13  Charging status checked at exit.
Traceback (most recent call last):
  File "../../testing/scripts/run_telemetry_benchmark_as_googletest.py", line 91, in main
    with open(tempfile_name) as f:
IOError: [Errno 2] No such file or directory: '/b/swarming/w/itEa7DxY/tmp9Cn6Owtelemetry/results.json'



I will investigate & manually fix this for now. As the follow up, we should try to add CQ coverage for this.
 
Cc: -ashleymarie@chromium.org
Owner: ashleymarie@chromium.org
Status: Assigned (was: Untriaged)
Ashley has WIP CL
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8148f5fdc47e7f1a4c62c4df85770d1ec2594896

commit 8148f5fdc47e7f1a4c62c4df85770d1ec2594896
Author: Ashley Enstad <ashleymarie@chromium.org>
Date: Wed Aug 02 03:08:57 2017

Updating run_telemetry_benchmark_as_googletest to resolve crash.

Previously, run_telemetry_benchmark_as_googletest would check the
validity of the benchmark run by opening results.json. This file
was generated using the --output-format=json flag. By removing
that flag in https://chromium-review.googlesource.com/c/589820,
I broke all the Telemetry benchmarks on the perf waterfall o.O my
bad. This change should fix the failing tests by checking the
validity of the benchmark run by opening test-results.json,
generated using the --output-format=json-test-results flag now
being used for all Telemetry benchmark runs on the waterfall.

BUG= 751346 , 748638

NOTRY=true  # This script is not run on trybot/CQ, hence should be safe to notry

Change-Id: I75e0abb5801f1cbaa480e65ade1827335a1195fd
Reviewed-on: https://chromium-review.googlesource.com/597428
Commit-Queue: Ashley Enstad <ashleymarie@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Cr-Commit-Position: refs/heads/master@{#491242}
[modify] https://crrev.com/8148f5fdc47e7f1a4c62c4df85770d1ec2594896/testing/scripts/run_telemetry_benchmark_as_googletest.py

yikes, yes this is a known problem.  thanks for jumping on it ned and ashley.

the procedure of every time we make a change to this file watch the waterfall like a hawk (for 30 hours since that is the cycle time) to make sure it doesn't break the world, is not a good one.  


In the short term, we can add some CQ test to https://cs.chromium.org/chromium/src/tools/perf/scripts_smoke_unittest.py

s.t like:
run_telemetry_benchmark_as_googletest tools/perf/run_benchmark dummy_benchmark...
Cc: nednguyen@chromium.org
 Issue 751881  has been merged into this issue.
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eb478a08ad6f0944e1ddf17e25948e6686abb300

commit eb478a08ad6f0944e1ddf17e25948e6686abb300
Author: Ashley Enstad <ashleymarie@chromium.org>
Date: Thu Aug 03 02:42:50 2017

Only check for json-test-results output when the benchmark is enabled

If a benchmark is disabled but still run as a step, FormatDisabled is
called on the output_formatter instead of Format. In the case of
json-test-results, this means that no json will be output by the test
(since there is no empty version of json-test-results).

BUG= chromium:751346 

NOTRY=true  # This script is not run on trybot/CQ, hence should be safe to notry

Change-Id: I4f19eeab40c1b6976e0f6c92cd5b7fff8ee17e2e
Reviewed-on: https://chromium-review.googlesource.com/598729
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Ashley Enstad <ashleymarie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491622}
[modify] https://crrev.com/eb478a08ad6f0944e1ddf17e25948e6686abb300/testing/scripts/run_telemetry_benchmark_as_googletest.py

Cc: rnep...@chromium.org charliea@chromium.org
 Issue 751876  has been merged into this issue.
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5e1171c8c293d8bcc8cf0eeb53583e7aaaff8c34

commit 5e1171c8c293d8bcc8cf0eeb53583e7aaaff8c34
Author: catapult-deps-roller@chromium.org <catapult-deps-roller@chromium.org>
Date: Fri Aug 04 23:24:48 2017

Roll src/third_party/catapult/ 357e1deef..0fb50e3f8 (5 commits)

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/357e1deefca7..0fb50e3f84ef

$ git log 357e1deef..0fb50e3f8 --date=short --no-merges --format='%ad %ae %s'
2017-08-04 etienneb Add support for fetching M61 clang builds R=erikchen@chromium.org BUG= chromium:752336 
2017-08-04 pfeldman Take non-default browser target location into account when attaching to it.
2017-08-04 konkers Add fuchsia tracing importer.
2017-08-04 ashleymarie Add FormatDisabled for json_3_output_formatter
2017-08-04 benjhayden Display histogram and sample diagnostics in a tab-view in histogram-span.

Created with:
  roll-dep src/third_party/catapult
BUG= 666865 , 751346 


Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls


CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel
TBR=sullivan@chromium.org

Change-Id: If43a288c7c2d36402d4e3183345de7f6065803a8
Reviewed-on: https://chromium-review.googlesource.com/602681
Reviewed-by: <catapult-deps-roller@chromium.org>
Commit-Queue: <catapult-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492169}
[modify] https://crrev.com/5e1171c8c293d8bcc8cf0eeb53583e7aaaff8c34/DEPS

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/81d004bf3b3efb3c0c4a7bb4a62520504cfd8f0f

commit 81d004bf3b3efb3c0c4a7bb4a62520504cfd8f0f
Author: Ashley Enstad <ashleymarie@chromium.org>
Date: Mon Aug 07 15:52:57 2017

Still load json-test-results when the benchmark is disabled

Will allow results to be merged together instead of crashing on disabled
benchmarks.

BUG= chromium:751346 

Change-Id: Id51b7997511dc1eade3b69807a791f25314704f1
Reviewed-on: https://chromium-review.googlesource.com/602068
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Ashley Enstad <ashleymarie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492330}
[modify] https://crrev.com/81d004bf3b3efb3c0c4a7bb4a62520504cfd8f0f/testing/scripts/run_telemetry_benchmark_as_googletest.py

Status: Fixed (was: Assigned)
I believe this is now fixed and the new failures on the waterfall are unrelated
Please reopen if something is showing signs of related failure (json results not being found, uploads failing especially due to epoch time failures, etc)
Thanks!
Follow up bug to add test to CQ is here: https://bugs.chromium.org/p/chromium/issues/detail?id=753834
I'm also working on a post mortem
Postmortem here: go/breaking_the_perf_waterfall
Cc: -nednguyen@chromium.org nedngu...@google.com
Project Member

Comment 14 by bugdroid1@chromium.org, Aug 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a73edfee27fef58bc296ed420333288a04b3d628

commit a73edfee27fef58bc296ed420333288a04b3d628
Author: Ashley Enstad <ashleymarie@chromium.org>
Date: Fri Aug 11 05:43:34 2017

Adding a smoke test for run_telemetry_benchmark_as_googletest script

This test will prevent changes that break
run_telemetry_benchmark_as_googletest from being checked in at CQ level
before they break the waterfall.

BUG= chromium:751346 

Change-Id: Ice0f7bbc0380edb4b3a154d03c2f5b95a572a795
Reviewed-on: https://chromium-review.googlesource.com/602056
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Ashley Enstad <ashleymarie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493674}
[modify] https://crrev.com/a73edfee27fef58bc296ed420333288a04b3d628/chrome/test/BUILD.gn
[modify] https://crrev.com/a73edfee27fef58bc296ed420333288a04b3d628/tools/perf/scripts_smoke_unittest.py

Sign in to add a comment