Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1853643006:
Use generated android wrapper scripts in runtest.py (Closed)
|
Created:
4 years, 8 months ago by agrieve Modified:
4 years, 7 months ago Reviewers:
jbudorick CC:
chromium-reviews, infra-reviews+build_chromium.org, kjellander-cc_chromium.org, cblume, Bernhard Bauer Base URL:
https://chromium.googlesource.com/chromium/tools/build.git@master Target Ref:
refs/heads/master Project:
build Visibility:
Public. |
DescriptionAndroid: Delete non --run-python-script code path for runtest.py
The script is never invoked without --run-python-script
BUG=599919
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299697
Patch Set 1 #Patch Set 2 : delete dead runtest.py code #
Dependent Patchsets: Messages
Total messages: 17 (5 generated)
Description was changed from ========== Use generated android wrapper scripts in runtest.py BUG=599919 ========== to ========== Use generated android wrapper scripts in runtest.py BUG=599919 ==========
agrieve@chromium.org changed reviewers: + jbudorick@chromium.org
On 2016/04/02 00:43:42, agrieve wrote: > mailto:agrieve@chromium.org changed reviewers: > + mailto:jbudorick@chromium.org 🍹
This isn't the right place to do this. It may affect some bots (maybe?), but most use https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/re... for gtests and https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/re... for instrumentation tests.
On 2016/04/02 00:47:33, jbudorick wrote: > This isn't the right place to do this. It may affect some bots (maybe?), but > most use > https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/re... > for gtests and > https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/re... > for instrumentation tests. also, beware that you're kind of wandering into a black hole here.
On 2016/04/02 00:48:04, jbudorick wrote: > On 2016/04/02 00:47:33, jbudorick wrote: > > This isn't the right place to do this. It may affect some bots (maybe?), but > > most use > > > https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/re... > > for gtests and > > > https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/re... > > for instrumentation tests. > > also, beware that you're kind of wandering into a black hole here. Do you think I should delete this code instead? Don't want to leave it as-is since it's running test_runner.py directly
On 2016/04/04 14:37:53, agrieve wrote: > On 2016/04/02 00:48:04, jbudorick wrote: > > On 2016/04/02 00:47:33, jbudorick wrote: > > > This isn't the right place to do this. It may affect some bots (maybe?), but > > > most use > > > > > > https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/re... > > > for gtests and > > > > > > https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/re... > > > for instrumentation tests. > > > > also, beware that you're kind of wandering into a black hole here. > > Do you think I should delete this code instead? Don't want to leave it as-is > since it's running test_runner.py directly If it isn't used, yes. That said, I'm not sure if it's completely unused.
On 2016/04/04 14:53:36, jbudorick wrote: > On 2016/04/04 14:37:53, agrieve wrote: > > On 2016/04/02 00:48:04, jbudorick wrote: > > > On 2016/04/02 00:47:33, jbudorick wrote: > > > > This isn't the right place to do this. It may affect some bots (maybe?), > but > > > > most use > > > > > > > > > > https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/re... > > > > for gtests and > > > > > > > > > > https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/re... > > > > for instrumentation tests. > > > > > > also, beware that you're kind of wandering into a black hole here. > > > > Do you think I should delete this code instead? Don't want to leave it as-is > > since it's running test_runner.py directly > > If it isn't used, yes. That said, I'm not sure if it's completely unused. Alright, after grepping through expectation .json files, I've convinced myself that: For *android*.json, runtest.py is never called without --run-python-script Deleted the dead code.
Description was changed from ========== Use generated android wrapper scripts in runtest.py BUG=599919 ========== to ========== Android: Delete non --run-python-script code path for runtest.py The script is never invoked without --run-python-script BUG=599919 ==========
lgtm
The CQ bit was checked by agrieve@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1853643006/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1853643006/20001
On 2016/04/05 14:19:48, commit-bot: I haz the power wrote: > CQ is trying da patch. Follow status at > https://chromium-cq-status.appspot.com/patch-status/1853643006/20001 > View timeline at > https://chromium-cq-status.appspot.com/patch-timeline/1853643006/20001 CC'ing active sheriffs (so you know recipes are changing)
Message was sent while issue was closed.
Description was changed from ========== Android: Delete non --run-python-script code path for runtest.py The script is never invoked without --run-python-script BUG=599919 ========== to ========== Android: Delete non --run-python-script code path for runtest.py The script is never invoked without --run-python-script BUG=599919 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299697 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as http://src.chromium.org/viewvc/chrome?view=rev&revision=299697
Message was sent while issue was closed.
On 2016/04/05 at 14:23:40, commit-bot wrote: > Committed patchset #2 (id:20001) as http://src.chromium.org/viewvc/chrome?view=rev&revision=299697 Hmm, I found a place where it looks like runtest.py is called without --run-python-script on android. :-( It's (sometimes? always?) used on the performance auto-bisect bots when running perf tests on android. Example step: https://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus9_... [ "python", "-u", "/b/build/scripts/slave/runtest.py", "--target", "Release", "--test-platform", "android", "--xvfb", "--factory-properties", "[omitted]", "--build-properties", "[omitted]", "--step-name=Performance Test 1 of 5", "--builder-name=android_nexus9_perf_bisect", "--slave-name=build105-b4", "--build-number=1752", "../../../src/tools/perf/run_benchmark", "-v", "--browser=android-chromium", "--output-format=chartjson", "--upload-results", "--also-run-disabled-tests", "scheduler.tough_scheduling_cases", "--output-dir=/tmp/perf-test-outputUzZ_PS" ] Not sure yet what should be done about this.
Message was sent while issue was closed.
On 2016/04/29 at 20:48:22, qyearsley wrote: > On 2016/04/05 at 14:23:40, commit-bot wrote: > > Committed patchset #2 (id:20001) as http://src.chromium.org/viewvc/chrome?view=rev&revision=299697 > > Hmm, I found a place where it looks like runtest.py is called without --run-python-script on android. :-( > > It's (sometimes? always?) used on the performance auto-bisect bots when running perf tests on android. > > Example step: > https://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus9_... > > [ > "python", > "-u", > "/b/build/scripts/slave/runtest.py", > "--target", > "Release", > "--test-platform", > "android", > "--xvfb", > "--factory-properties", > "[omitted]", > "--build-properties", > "[omitted]", > "--step-name=Performance Test 1 of 5", > "--builder-name=android_nexus9_perf_bisect", > "--slave-name=build105-b4", > "--build-number=1752", > "../../../src/tools/perf/run_benchmark", > "-v", > "--browser=android-chromium", > "--output-format=chartjson", > "--upload-results", > "--also-run-disabled-tests", > "scheduler.tough_scheduling_cases", > "--output-dir=/tmp/perf-test-outputUzZ_PS" > ] > > Not sure yet what should be done about this. Nope, it turns out I was looking at an old failure and this issue has already been fixed: https://codereview.chromium.org/1915183002/ |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
