Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
410033002:
Run android tests through runtest.py. (Closed)
DescriptionRun android tests through runtest.py.
BUG=329102
R=qyearsley@chromium.org
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=285667
Patch Set 1 #Patch Set 2 : Added generate-json-file option #
Total comments: 7
Patch Set 3 : Respond to codereview. #Messages
Total messages: 19 (0 generated)
ptal
Please include jbudorick@ on anything related to running tests on Android.
b/a/buildbot is more navabi@'s wheelhouse than mine. https://codereview.chromium.org/410033002/diff/20001/build/android/buildbot/b... File build/android/buildbot/bb_device_steps.py (right): https://codereview.chromium.org/410033002/diff/20001/build/android/buildbot/b... build/android/buildbot/bb_device_steps.py:109: '--no-xvfb', style nit: continuation lines should be indented 4 spaces or to match the line above. https://codereview.chromium.org/410033002/diff/20001/build/android/buildbot/b... build/android/buildbot/bb_device_steps.py:113: ] + property_args style: this should be indented https://codereview.chromium.org/410033002/diff/20001/build/android/buildbot/b... build/android/buildbot/bb_device_steps.py:148: cmd = ['build/android/test_runner.py', 'gtest', '-s', suite] + args This seems to be partially duplicated in _MainAndroid in runtest.py.
The arguments passed to runtest.py look OK to me. https://codereview.chromium.org/410033002/diff/20001/build/android/buildbot/b... File build/android/buildbot/bb_device_steps.py (right): https://codereview.chromium.org/410033002/diff/20001/build/android/buildbot/b... build/android/buildbot/bb_device_steps.py:109: '--no-xvfb', Note: Passing --no-xvfb appears to have no effect in runtest.py if the platform is not Linux -- If 'test_platform' in factory properties is 'android', then this is ignored. There's no harm in passing it, except that it might suggest that it's necessary.
Addressed. https://codereview.chromium.org/410033002/diff/20001/build/android/buildbot/b... File build/android/buildbot/bb_device_steps.py (right): https://codereview.chromium.org/410033002/diff/20001/build/android/buildbot/b... build/android/buildbot/bb_device_steps.py:109: '--no-xvfb', On 2014/07/23 14:23:12, jbudorick wrote: > style nit: continuation lines should be indented 4 spaces or to match the line > above. Acknowledged. https://codereview.chromium.org/410033002/diff/20001/build/android/buildbot/b... build/android/buildbot/bb_device_steps.py:109: '--no-xvfb', On 2014/07/23 18:56:38, qyearsley wrote: > Note: Passing --no-xvfb appears to have no effect in runtest.py if the platform > is not Linux -- If 'test_platform' in factory properties is 'android', then this > is ignored. > > There's no harm in passing it, except that it might suggest that it's necessary. Acknowledged. https://codereview.chromium.org/410033002/diff/20001/build/android/buildbot/b... build/android/buildbot/bb_device_steps.py:148: cmd = ['build/android/test_runner.py', 'gtest', '-s', suite] + args Notice here we use --run-python-script, which makes this necessary. On 2014/07/23 14:23:12, jbudorick wrote: > This seems to be partially duplicated in _MainAndroid in runtest.py.
lgtm, not sure if others still have doubts/questions.
The CQ bit was checked by zty@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/zty@chromium.org/410033002/40001
On 2014/07/23 19:53:22, zty wrote: > Addressed. > > https://codereview.chromium.org/410033002/diff/20001/build/android/buildbot/b... > File build/android/buildbot/bb_device_steps.py (right): > > https://codereview.chromium.org/410033002/diff/20001/build/android/buildbot/b... > build/android/buildbot/bb_device_steps.py:109: '--no-xvfb', > On 2014/07/23 14:23:12, jbudorick wrote: > > style nit: continuation lines should be indented 4 spaces or to match the line > > above. > > Acknowledged. > > https://codereview.chromium.org/410033002/diff/20001/build/android/buildbot/b... > build/android/buildbot/bb_device_steps.py:109: '--no-xvfb', > On 2014/07/23 18:56:38, qyearsley wrote: > > Note: Passing --no-xvfb appears to have no effect in runtest.py if the > platform > > is not Linux -- If 'test_platform' in factory properties is 'android', then > this > > is ignored. > > > > There's no harm in passing it, except that it might suggest that it's > necessary. > > Acknowledged. > > https://codereview.chromium.org/410033002/diff/20001/build/android/buildbot/b... > build/android/buildbot/bb_device_steps.py:148: cmd = > ['build/android/test_runner.py', 'gtest', '-s', suite] + args > Notice here we use --run-python-script, which makes this necessary. > On 2014/07/23 14:23:12, jbudorick wrote: > > This seems to be partially duplicated in _MainAndroid in runtest.py. Ah, I didn't see that option in runtest.py. lgtm
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: chromium_presubmit on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...) linux_chromium_chromeos_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...) linux_chromium_rel_swarming on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel_sw...)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
On 2014/07/24 20:02:13, I haz the power (commit-bot) wrote: > Try jobs failed on following builders: > chromium_presubmit on tryserver.chromium > (http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...) lgtm
The CQ bit was checked by zty@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/zty@chromium.org/410033002/40001
Message was sent while issue was closed.
Change committed as 285667
Message was sent while issue was closed.
Android builders are sad panda: https://build.chromium.org/p/chromium.linux/builders/Android%20Tests/builds/1...
Message was sent while issue was closed.
A revert of this CL has been created in https://codereview.chromium.org/423593002/ by dmichael@chromium.org. The reason for reverting is: Broke a ton of tests: http://build.chromium.org/p/chromium.linux/builders/Android%20Tests/builds/14823.
Message was sent while issue was closed.
A revert of this CL has been created in https://codereview.chromium.org/414333002/ by mpearson@chromium.org. The reason for reverting is: Causes failures: http://build.chromium.org/p/chromium.linux/builders/Android%20Tests/builds/14... error: --results-directory is required with --generate-json-file=True. |
