Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2605083002:
[android] Clean up test_runner.py arguments. (Closed)
Description[android] Clean up test_runner.py arguments.
This includes:
- reformatting
- purging dead / deprecated / unused arguments
Review-Url: https://codereview.chromium.org/2605083002
Cr-Commit-Position: refs/heads/master@{#456835}
Committed: https://chromium.googlesource.com/chromium/src/+/761f60864c37e7af6fe9dba49256a8b014860049
Patch Set 1 #
Total comments: 3
Patch Set 2 : Fix adb and json results file handling. #Patch Set 3 : rebase and recipe change #Patch Set 4 : bpastene comment from other review #
Total comments: 2
Patch Set 5 : mikecase comments #
Dependent Patchsets: Messages
Total messages: 29 (18 generated)
The CQ bit was checked by jbudorick@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
jbudorick@chromium.org changed reviewers: + mikecase@chromium.org, rnephew@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
lgtm once the problems with junit args is fixed. https://codereview.chromium.org/2605083002/diff/1/build/android/test_runner.py File build/android/test_runner.py (right): https://codereview.chromium.org/2605083002/diff/1/build/android/test_runner.p... build/android/test_runner.py:59: '--json-results-file', junit test is complaining about this not being recognized. Also --adb-path
nit: Remove BUG= from commit desc.
https://codereview.chromium.org/2605083002/diff/1/build/android/test_runner.py File build/android/test_runner.py (right): https://codereview.chromium.org/2605083002/diff/1/build/android/test_runner.p... build/android/test_runner.py:59: '--json-results-file', On 2017/01/03 16:50:36, rnephew (Reviews Here) wrote: > junit test is complaining about this not being recognized. Also --adb-path Yeah, I'm going to have to rework the recipes a bit for the --adb-path part.
The CQ bit was checked by jbudorick@chromium.org to run a CQ dry run
Description was changed from ========== [android] Clean up test_runner.py arguments. This includes: - reformatting - purging dead / deprecated / unused arguments BUG= ========== to ========== [android] Clean up test_runner.py arguments. This includes: - reformatting - purging dead / deprecated / unused arguments ==========
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by jbudorick@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Dropped BUG=. It's been 2 months, so ptal. https://codereview.chromium.org/2605083002/diff/1/build/android/test_runner.py File build/android/test_runner.py (right): https://codereview.chromium.org/2605083002/diff/1/build/android/test_runner.p... build/android/test_runner.py:59: '--json-results-file', On 2017/01/04 15:29:04, jbudorick wrote: > On 2017/01/03 16:50:36, rnephew (Reviews Here) wrote: > > junit test is complaining about this not being recognized. Also --adb-path > > Yeah, I'm going to have to rework the recipes a bit for the --adb-path part. Fixed.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2017/03/10 18:39:06, jbudorick wrote: > Dropped BUG=. > > It's been 2 months, so ptal. > > https://codereview.chromium.org/2605083002/diff/1/build/android/test_runner.py > File build/android/test_runner.py (right): > > https://codereview.chromium.org/2605083002/diff/1/build/android/test_runner.p... > build/android/test_runner.py:59: '--json-results-file', > On 2017/01/04 15:29:04, jbudorick wrote: > > On 2017/01/03 16:50:36, rnephew (Reviews Here) wrote: > > > junit test is complaining about this not being recognized. Also --adb-path > > > > Yeah, I'm going to have to rework the recipes a bit for the --adb-path part. > > Fixed. friendly ping
oops, I confused this with the gtest --command-line-arg CL. Will review on bus or tomorrow morning.
lgtm with 1 comment. https://codereview.chromium.org/2605083002/diff/60001/build/android/pylib/loc... File build/android/pylib/local/device/local_device_environment.py (right): https://codereview.chromium.org/2605083002/diff/60001/build/android/pylib/loc... build/android/pylib/local/device/local_device_environment.py:103: adb_dir = os.path.dirname(constants.GetAdbPath()) You have a TODO to remove existing callers of constants.GetAdbPath(). So you should probably instead call AdbWrapper.GetAdbPath()
https://codereview.chromium.org/2605083002/diff/60001/build/android/pylib/loc... File build/android/pylib/local/device/local_device_environment.py (right): https://codereview.chromium.org/2605083002/diff/60001/build/android/pylib/loc... build/android/pylib/local/device/local_device_environment.py:103: adb_dir = os.path.dirname(constants.GetAdbPath()) On 2017/03/14 16:02:44, mikecase wrote: > You have a TODO to remove existing callers of constants.GetAdbPath(). So you > should probably instead call AdbWrapper.GetAdbPath() Good call. Done.
The CQ bit was checked by jbudorick@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rnephew@chromium.org, mikecase@chromium.org Link to the patchset: https://codereview.chromium.org/2605083002/#ps80001 (title: "mikecase comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1489521640642350,
"parent_rev": "6649d2e398d245c73a967f35862d192446daeee2", "commit_rev":
"761f60864c37e7af6fe9dba49256a8b014860049"}
Message was sent while issue was closed.
Description was changed from ========== [android] Clean up test_runner.py arguments. This includes: - reformatting - purging dead / deprecated / unused arguments ========== to ========== [android] Clean up test_runner.py arguments. This includes: - reformatting - purging dead / deprecated / unused arguments Review-Url: https://codereview.chromium.org/2605083002 Cr-Commit-Position: refs/heads/master@{#456835} Committed: https://chromium.googlesource.com/chromium/src/+/761f60864c37e7af6fe9dba49256... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/761f60864c37e7af6fe9dba49256... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
