Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1925013002:
👔 Make instrumentation tests declare their runtime_deps (Closed)
|
Created:
4 years, 7 months ago by agrieve Modified:
4 years, 7 months ago CC:
chromium-reviews, darin-cc_chromium.org, jam, android-webview-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionGN: Make instrumentation tests declare their runtime_deps
TBR=vadimsh
BUG=589318
Committed: https://crrev.com/e98a8fbe91dc81f05799389edaca003e29f78d70
Cr-Commit-Position: refs/heads/master@{#390482}
Patch Set 1 #
Total comments: 1
Patch Set 2 : just make isolate changes #Patch Set 3 : fix cronet isolate #Patch Set 4 : #
Messages
Total messages: 36 (16 generated)
Description was changed from ========== Make android instrumentation tests work with "mb run" BUG=589318#c32 ========== to ========== Make android instrumentation tests work with "mb run" BUG=589318#c32 ==========
agrieve@chromium.org changed reviewers: + dpranke@chromium.org, jbudorick@chromium.org, stip@chromium.org
On 2016/04/28 02:31:17, agrieve wrote: > mailto:agrieve@chromium.org changed reviewers: > + mailto:dpranke@chromium.org, mailto:jbudorick@chromium.org, mailto:stip@chromium.org Took a stab at getting the instrumentation tests going. Part of it was missing data, but the truly ugly part is the change I had to make to mb.py that maps android_webview_test -> android_webview_test_apk when compiling. Wondering if it'd be better to do the work to change the recipes so that instrumentation tests are different from gtests, so that you can use "android_webview_test_apk" rather than "android_webview_test" (the latter doesn't actually exist as a target).
I really think we should be working toward using the target name. 'chrome_public_test' isn't the target here or in gyp. https://codereview.chromium.org/1925013002/diff/1/tools/mb/mb.py File tools/mb/mb.py (right): https://codereview.chromium.org/1925013002/diff/1/tools/mb/mb.py#newcode295 tools/mb/mb.py:295: build_target = target + '_apk' :(
On 2016/04/28 03:11:58, jbudorick wrote: > I really think we should be working toward using the target name. > 'chrome_public_test' isn't the target here or in gyp. > > https://codereview.chromium.org/1925013002/diff/1/tools/mb/mb.py > File tools/mb/mb.py (right): > > https://codereview.chromium.org/1925013002/diff/1/tools/mb/mb.py#newcode295 > tools/mb/mb.py:295: build_target = target + '_apk' > :( I have an alternative idea here involving some minor changes to the recipes & the //testing/buildbot jsons. I'll send it out in a bit. I think we'll need the test data changes you have in here regardless, though.
On 2016/04/28 15:06:10, jbudorick wrote: > On 2016/04/28 03:11:58, jbudorick wrote: > > I really think we should be working toward using the target name. > > 'chrome_public_test' isn't the target here or in gyp. > > > > https://codereview.chromium.org/1925013002/diff/1/tools/mb/mb.py > > File tools/mb/mb.py (right): > > > > https://codereview.chromium.org/1925013002/diff/1/tools/mb/mb.py#newcode295 > > tools/mb/mb.py:295: build_target = target + '_apk' > > :( > > I have an alternative idea here involving some minor changes to the recipes & > the //testing/buildbot jsons. I'll send it out in a bit. > > I think we'll need the test data changes you have in here regardless, though. Sounds great! I'll split out the data changes.
Description was changed from ========== Make android instrumentation tests work with "mb run" BUG=589318#c32 ========== to ========== Make instrumentation tests declare their runtime_deps BUG=589318 ==========
agrieve@chromium.org changed reviewers: + michaelbai@chromium.org, vadimsh@chromium.org
michaelbai & vadimsh for .isolate files
The CQ bit was checked by agrieve@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1925013002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1925013002/20001
lgtm
On 2016/04/28 18:54:58, jbudorick wrote: > lgtm Andrew, You probably got wrong name, I don't know the isolate file
rs-lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_android on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
agrieve@chromium.org changed reviewers: + mmenke@chromium.org
mmenke@chromium.org: Please review changes in cronet
On 2016/04/28 19:04:05, michaelbai wrote: > On 2016/04/28 18:54:58, jbudorick wrote: > > lgtm > > Andrew, > You probably got wrong name, I don't know the isolate file Not actually changing the isolate, just removing the OS=Android condition (which should always true for an Android instrumentation test).
The CQ bit was checked by agrieve@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1925013002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1925013002/40001
lgtm
On 2016/04/28 19:35:47, Dirk Pranke wrote: > lgtm cronet LGTM
OK, you actually meant android_webview, then lgtm
Description was changed from ========== Make instrumentation tests declare their runtime_deps BUG=589318 ========== to ========== GN: Make instrumentation tests declare their runtime_deps BUG=589318 ==========
The CQ bit was checked by agrieve@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1925013002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1925013002/60001
Description was changed from ========== GN: Make instrumentation tests declare their runtime_deps BUG=589318 ========== to ========== GN: Make instrumentation tests declare their runtime_deps TBR=vadimsh BUG=589318 ==========
The CQ bit was unchecked by agrieve@chromium.org
The CQ bit was checked by agrieve@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jbudorick@chromium.org, stip@chromium.org, dpranke@chromium.org, michaelbai@chromium.org, mmenke@chromium.org Link to the patchset: https://codereview.chromium.org/1925013002/#ps60001 (title: " ")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1925013002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1925013002/60001
Message was sent while issue was closed.
Description was changed from ========== GN: Make instrumentation tests declare their runtime_deps TBR=vadimsh BUG=589318 ========== to ========== GN: Make instrumentation tests declare their runtime_deps TBR=vadimsh BUG=589318 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/e98a8fbe91dc81f05799389edaca003e29f78d70 Cr-Commit-Position: refs/heads/master@{#390482}
Message was sent while issue was closed.
Description was changed from ========== GN: Make instrumentation tests declare their runtime_deps TBR=vadimsh BUG=589318 ========== to ========== GN: Make instrumentation tests declare their runtime_deps TBR=vadimsh BUG=589318 Committed: https://crrev.com/e98a8fbe91dc81f05799389edaca003e29f78d70 Cr-Commit-Position: refs/heads/master@{#390482} ========== |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
