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

Issue 633333 link

Starred by 4 users
Status: Fixed
Owner:
jbudorick@chromium.org
Closed: Aug 2016
Cc:
agrieve@chromium.org
█ stip@chromium.org
mar...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

We need to make the android build no longer rely on checked-in *.isolate files

Project Member Reported by dpranke@chromium.org, Aug 1 2016 
Now that the GYP build is turned off everywhere, we need to be sure that we're ready to start deleting GYP files, and as part of that we should make sure we're also ready to start deleting .isolate files, so that we don't need to ask devs to keep two different sets of things in sync.

We should be able to generate whatever the instrumentation tests need from just the list of runtime_deps specified in the GN build, so someone just needs to updated the _generate_device_isolate logic accordingly and make sure all of the needed data deps are there.

https://codesearch.chromium.org/chromium/src/testing/test.gni?rcl=0&l=92

Comment 1 by dpranke@chromium.org, Aug 1 2016

Status: Assigned (was: Untriaged)

Comment 2 by jbudorick@chromium.org, Aug 2 2016

Status: Started (was: Assigned)

Comment 3 by agrieve@chromium.org, Aug 2 2016 

Some thoughts on this:

It's best to ensure there's not too many excess files being included via unnecessary data deps.

It might be better to do similar to what mac/ios is doing, and just have an explicit file list (via bundle_data). We could look at using bundle_data directly, or maybe just use a group() with data=[].

Even better would be to change all tests to not require .isolate files beyond the test runner + test.apk. Any other data they require should be included in the apk via android_assets.

Comment 4 by jbudorick@chromium.org, Aug 2 2016 

I'll look at bundle_data. Moving data dependencies into the apk would be substantially more complex, so I don't intend to look at it as part of this bug. We can revisit that later.

Comment 5 by dpranke@chromium.org, Aug 3 2016 

it's not clear to me that bundle_data() will work as is (it was intentionally scoped just to mac/ios needs at the time), but I agree that we should spend some time now trying to see if it can be made to work.

But I also agree that not doing that in this bug makes sense.
Project Member

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

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

commit f5e2b6ac5cf0356d4e3fa25d65a6d1dcf4aaa2ad
Author: jbudorick <jbudorick@chromium.org>
Date: Wed Aug 03 02:02:20 2016

[Android] Move isolate generation into //build/config/android.

The instrumentation_test_apk template can't use the isolate generation
logic if it stays in //testing/test.gni as doing so would introduce a
circular dependency. Android appears to be the only user of the
isolate generation logic anyway.

BUG= 633333 

Review-Url: https://codereview.chromium.org/2204823002
Cr-Commit-Position: refs/heads/master@{#409429}

[rename] https://crrev.com/f5e2b6ac5cf0356d4e3fa25d65a6d1dcf4aaa2ad/build/android/gn/generate_isolate.py
[modify] https://crrev.com/f5e2b6ac5cf0356d4e3fa25d65a6d1dcf4aaa2ad/build/config/android/internal_rules.gni
[modify] https://crrev.com/f5e2b6ac5cf0356d4e3fa25d65a6d1dcf4aaa2ad/testing/test.gni
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 6 2016 

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

commit 84c77363529b707fe226c440a153810cb6dea30d
Author: jbudorick <jbudorick@chromium.org>
Date: Sat Aug 06 01:00:34 2016

[Android] Switch instrumentation tests to generated device isolates.

BUG= 633333 

Review-Url: https://codereview.chromium.org/2203073002
Cr-Commit-Position: refs/heads/master@{#410238}

[delete] https://crrev.com/92e3a706af2e37fa02544787769288d6f5e0c2b8/android_webview/android_webview_test_apk.isolate
[delete] https://crrev.com/92e3a706af2e37fa02544787769288d6f5e0c2b8/android_webview/android_webview_test_data.isolate
[delete] https://crrev.com/92e3a706af2e37fa02544787769288d6f5e0c2b8/android_webview/system_webview_shell_layout_test_apk_run.isolate
[delete] https://crrev.com/92e3a706af2e37fa02544787769288d6f5e0c2b8/android_webview/system_webview_shell_test_apk.isolate
[modify] https://crrev.com/84c77363529b707fe226c440a153810cb6dea30d/android_webview/test/BUILD.gn
[modify] https://crrev.com/84c77363529b707fe226c440a153810cb6dea30d/android_webview/tools/system_webview_shell/BUILD.gn
[modify] https://crrev.com/84c77363529b707fe226c440a153810cb6dea30d/build/android/gn/generate_isolate.py
[modify] https://crrev.com/84c77363529b707fe226c440a153810cb6dea30d/build/config/android/internal_rules.gni
[modify] https://crrev.com/84c77363529b707fe226c440a153810cb6dea30d/build/config/android/rules.gni
[modify] https://crrev.com/84c77363529b707fe226c440a153810cb6dea30d/chrome/android/BUILD.gn
[delete] https://crrev.com/92e3a706af2e37fa02544787769288d6f5e0c2b8/chrome/android/chrome_public_test_apk.isolate
[delete] https://crrev.com/92e3a706af2e37fa02544787769288d6f5e0c2b8/chrome/android/chrome_sync_shell_test_apk.isolate
[delete] https://crrev.com/92e3a706af2e37fa02544787769288d6f5e0c2b8/chrome/chrome_public_test_apk.isolate
[modify] https://crrev.com/84c77363529b707fe226c440a153810cb6dea30d/components/cronet/android/BUILD.gn
[delete] https://crrev.com/92e3a706af2e37fa02544787769288d6f5e0c2b8/components/cronet/android/cronet_test_instrumentation_apk.isolate
[delete] https://crrev.com/92e3a706af2e37fa02544787769288d6f5e0c2b8/content/content_shell_test_apk.isolate
[delete] https://crrev.com/92e3a706af2e37fa02544787769288d6f5e0c2b8/content/content_shell_test_data.isolate
[modify] https://crrev.com/84c77363529b707fe226c440a153810cb6dea30d/content/public/android/BUILD.gn
[modify] https://crrev.com/84c77363529b707fe226c440a153810cb6dea30d/content/shell/android/BUILD.gn
[modify] https://crrev.com/84c77363529b707fe226c440a153810cb6dea30d/mojo/android/BUILD.gn
[delete] https://crrev.com/92e3a706af2e37fa02544787769288d6f5e0c2b8/mojo/mojo_test_apk.isolate
Project Member

Comment 8 by bugdroid1@chromium.org, Aug 6 2016 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/525ea7d37694243dfa12f146ba5489ea9a9f8ee2

commit 525ea7d37694243dfa12f146ba5489ea9a9f8ee2
Author: John Budorick <jbudorick@google.com>
Date: Sat Aug 06 19:43:59 2016
Project Member

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

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/314c7b66502142e925af43bbd31437b0def359c4

commit 314c7b66502142e925af43bbd31437b0def359c4
Author: John Budorick <jbudorick@google.com>
Date: Sun Aug 07 14:10:38 2016
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 10 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra.git/+/741a2001acbbd5da5d884eb01e35bcd644f7bd3d

commit 741a2001acbbd5da5d884eb01e35bcd644f7bd3d
Author: recipe-roller <recipe-roller@chromium.org>
Date: Wed Aug 10 01:16:04 2016

Roll recipe dependencies (trivial).

This is an automated CL created by the recipe roller. This CL rolls recipe
changes from upstream projects (e.g. depot_tools) into downstream projects
(e.g. tools/build).

More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug
(or complain)

build:
  https://crrev.com/12b302b6ed29f15838a9145262d7913d19ae4aba Use iossim/test-without-building for Earlgrey tests. (huangml@chromium.org)
  https://crrev.com/a661277f9f72d06f313dba599b4e6a4a7dd9887f Remove the android_isolate_path option from LocalGTestTest. (jbudorick@chromium.org)

R=huangml@chromium.org,jbudorick@chromium.org
BUG= 633333 

TBR=martiniss@chromium.org,phajdan.jr@chromium.org

Review-Url: https://codereview.chromium.org/2233603002

[modify] https://crrev.com/741a2001acbbd5da5d884eb01e35bcd644f7bd3d/infra/config/recipes.cfg
Project Member

Comment 12 by bugdroid1@chromium.org, Aug 10 2016 

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

commit 411f3a717cba9dbf92413f6dfa59cd1bb6a6bfc4
Author: recipe-roller <recipe-roller@chromium.org>
Date: Wed Aug 10 18:29:51 2016

Roll recipe dependencies (trivial).

This is an automated CL created by the recipe roller. This CL rolls recipe
changes from upstream projects (e.g. depot_tools) into downstream projects
(e.g. tools/build).

More info is at https://goo.gl/zkKdpD. Use https://goo.gl/noib3a to file a bug
(or complain)

build:
  https://crrev.com/1ef1717a1d167fa243e0db9f89b1f715ca90b251 Roll recipe dependencies (trivial). (recipe-roller@chromium.org)
  https://crrev.com/1f32353ff4a7011bbded5898da32981437de16eb [Android] Specify --adb-path when printing perf test results. (jbudorick@chromium.org)
  https://crrev.com/52b73ce928346e5ff2de78658eed76bf048b1d45 Presubmit recipe: rebase Gerrit patches. (tandrii@chromium.org)
  https://crrev.com/ed36f101999bd972cea1b5be8e8e663e4c882a13 [perf] Remove Large Profile Generator. (dtu@chromium.org)
  https://crrev.com/ed494a5ee803bdbb1b0ac7df24cd6d819efc53f5 WebRTC: Disable Linux32 Debug+Release bots for now. (kjellander@chromium.org)
  https://crrev.com/abb024b2505d48e69dd8ba049f7b619677640fe9 Move Flutter engine to src/flutter (abarth@chromium.org)
  https://crrev.com/4e6de3a742791006b6ca41aab6c1bd8868461ce8 Update the path to the travis directory (abarth@chromium.org)
  https://crrev.com/7746bb86402b08a137921c1a2abe07352d95bd74 Roll recipe dependencies (trivial). (recipe-roller@chromium.org)
  https://crrev.com/e6e74c9db3e74f8603acf4d21389439aa20b232c Roll recipe dependencies (trivial). (recipe-roller@chromium.org)
  https://crrev.com/2c78661f30da29ac3d08a6e26b2392ada22dbba4 Check if device disconnects every time the last test of a revision fails (zxiong@google.com)
  https://crrev.com/12b302b6ed29f15838a9145262d7913d19ae4aba Use iossim/test-without-building for Earlgrey tests. (huangml@chromium.org)
  https://crrev.com/a661277f9f72d06f313dba599b4e6a4a7dd9887f Remove the android_isolate_path option from LocalGTestTest. (jbudorick@chromium.org)
depot_tools:
  https://crrev.com/f83fa37b3451a862df10e0a5f8a1265a28e75b1f Add flag to rebase before gclient sync when applying a Gerrit patch (andybons@chromium.org)
  https://crrev.com/1dae95a13f123a16bc1d03bd2c587e770ce2e29f Require internal recipe tryjob (martiniss@chromium.org)
  https://crrev.com/fff0635af2a79778a23be93802f2cabd6c9aedc4 Roll recipe dependencies (trivial). (recipe-roller@chromium.org)
recipe_engine:
  https://crrev.com/87823726a3c8ef0016780ac1b752a758f2191a55 Require internal recipe tryjob (martiniss@chromium.org)

R=dtu@chromium.org,zxiong@google.com,tandrii@chromium.org,huangml@chromium.org,kjellander@chromium.org,jbudorick@chromium.org,andybons@chromium.org,recipe-roller@chromium.org,martiniss@chromium.org,phajdan.jr@chromium.org,ehmaldonado@chromium.org,ianh@google.com,abarth@chromium.org
BUG= 628801 , 633333 ,635975, chromium:635183 , 636080 , 612417 

TBR=martiniss@chromium.org,phajdan.jr@chromium.org

Review-Url: https://codereview.chromium.org/2228243002
Cr-Commit-Position: refs/heads/master@{#411089}

[modify] https://crrev.com/411f3a717cba9dbf92413f6dfa59cd1bb6a6bfc4/infra/config/recipes.cfg
Project Member

Comment 13 by bugdroid1@chromium.org, Aug 15 2016 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/e2013985f5ad57cf9641be6354b737481582dc2b

commit e2013985f5ad57cf9641be6354b737481582dc2b
Author: John Budorick <jbudorick@google.com>
Date: Mon Aug 15 15:05:11 2016
Project Member

Comment 14 by bugdroid1@chromium.org, Aug 22 2016 

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

commit 982ede9f8a7d63f5986110e673b3d4cfaa50fcb6
Author: jbudorick <jbudorick@chromium.org>
Date: Mon Aug 22 21:17:25 2016

[Android] Remove vestigial isolate_file logic from instrumentation_test_apk.

BUG= 633333 

Review-Url: https://codereview.chromium.org/2233613002
Cr-Commit-Position: refs/heads/master@{#413536}

[modify] https://crrev.com/982ede9f8a7d63f5986110e673b3d4cfaa50fcb6/build/config/android/rules.gni

Comment 15 by jbudorick@chromium.org, Aug 29 2016

Status: Fixed (was: Started)
►

Sign in to add a comment