Run android webview tests on swarming |
|||||||||
Issue descriptionI was able to run android webview tests on swarming: https://chromium-swarm.appspot.com/task?id=365038abad87c310&refresh=10&show_raw=1 This is an isolate from a local release build, so it might be wrong. Any idea why this isn't passing?
,
May 23 2017
Is this test currently run on Webview. The failure is a timed out, so it could just be you have a bad webview build that fails this particular test.
,
May 23 2017
Yeah, I'm not sure how to handle that error. Let me run a test which is passing on the waterfall though.
,
May 24 2017
Ok, https://chromium-swarm-dev.appspot.com/task?id=3650b63297f14510&refresh=10&show_raw=1 succeeded, as did the test on the main waterfall. So we might be good. What's a good set of benchmarks I should run on webview, to make sure everything works ok?
,
May 24 2017
Also, I'm not sure if this is correct, but the tests seem to take a lot longer on webview. I could be wrong, or my build of chromium could be unoptimized. I'm not sure.
,
May 24 2017
I would also suggest running the isolate test with your local phone to see what goes wrong. Randy can help with debugging.
,
May 26 2017
,
May 30 2017
Ok, so I've been able to run the tests on some phones locally. I need to run this before run_benchmark succeeds though: python build/android/adb_install_apk.py out/64bit/apks/SystemWebView.apk I probably need to make telemetry run this command as well. I need both SystemWebView.apk and SystemWebViewShell.apk installed on the device to run webview tests, right? Anyone have an idea of how to do this? I looked at the code and it looks doable, if a bit complicated.
,
May 30 2017
cc-ing perezju@ if you have any input.
,
May 30 2017
boliu@: do you know if we need both SystemWebView.apk & SystemWebViewShell.apk to run webview tests?
,
May 30 2017
These are perf tests? Then yes, you need both. SystemWebView.apk is the (public version of) the thing we ship to the play store. Note the public version is only meant to run on aosp devices though. The actual production version is under a different package name, and the name of package used as the webview implementation is semi-hardcoded. So just installing SystemWebView.apk on device does not mean it's actually being used as the webview implementation. SystemWebViewShell.apk is a small app that uses the system webview, whatever implementation it is. I think this is the thing telemetry talks to.
,
May 30 2017
Thanks for the answer, Bo Liu@! #8: for Telemetry to install both apk, you can update the code in https://github.com/catapult-project/catapult/blob/master/telemetry/telemetry/internal/backends/chrome/android_browser_finder.py#L152 Probably an optional _embedder_apk field, so that PossibleAndroidBrowser .UpdateExecutableIfNeeded works like this: if HaveLocalAPK(): self.platform.InstallApplication(self._local_apk) if HaveEmbedderAPK(): self.platform.InstallApplication(self._local_embedder_apk)
,
May 30 2017
Sorry for the back n forth on direction, now I think about it, we just happen to use SystemWebViewShell.apk for our benchmark. Theoretically, if people want to use SystemWebView.apk with AndroidSearchApp.apk or GmailApp.apk, it should also be possible. So the way we implement this in Telemetry should be it support a --webview-embedder-apk-path flag, and have PossibleAndroidBrowser installs finder_options.webview_embedder_apk_path when its browser type is webview.
,
May 31 2017
It's a bit more complicated than that, mikecase started a discussion at issue 711073 to improve on the current situation. As far as I understand, setting up WebView on a device consists of: 1. Removing the stock webview implementation from the device (involves restarting the device) e.g.: https://github.com/catapult-project/catapult/blob/deb4a2aa8f11cb520e9b8bc7413296d899a3af47/devil/devil/android/tools/provision_devices.py#L375 2. Install apks: SystemWebView.apk (or SystemWebViewGoogle.apk on internal bots) and SystemWebViewShell.apk. 3. Select the correct webview implementation e.g. https://goto.google.com/rnokl (internal link) As I mentioned in that issue, I'm not sure how much of this should be responsibility of Telemetry, and how much should be done on some previous device provisioning step.
,
May 31 2017
Wow, this seems a lot harder than I thought it gonna be. How does webview correctness testing on Android swarming work now? Probably best to mimic the solution there. +Ben if you know the answer to this.
,
May 31 2017
> How does webview correctness testing on Android swarming work now? Probably best to mimic the solution there. That's based on the webview instrumentation test shell, target name webview_instrumentation_apk. That's a standalone apk that includes most (but not all) webview production code, plus fake implementations of private APIs that webview needs. It's fine for correctness testing (for code that's in there anyway), but not ideal for end-to-end perf testing, because part of that reimplemented private API is android compositor API, so it doesn't necessarily reflect production behavior.
,
May 31 2017
Oh I see. For the comment in #14: does the step of removing the stock webview implementation only need to be done once, or do we need to do it for every test that uses a new webview apk? If that's the former, it's probably be handled by infra, and not Telemetry. (2) & (3) seems to be in Telemetry realm to me.
,
May 31 2017
We could make the infra/swarming side of things remove the default webview. (A lot of what swarming does is just a subset of provision_devices, so it would make sense to add the logic there.) As for replacing it with a custom SystemWebView.apk, that should probably be done by the tests, yeah?
,
May 31 2017
I talked with jbudorick@ about this offline. We thought that it would be good to make a small wrapper script which would do uninstallation, and optional re-installation of particular packages, and add that to devil. Then our tests would call that wrapper, and have them not reinstall the package (to save on cycle time) This is also useful for other teams, AFAIK. I'm writing a small design doc about this. I'll post the link here in a bit when I have a first draft.
,
May 31 2017
martiniss and I talked about this earlier today -- we need a generic mechanism for disabling/removing/hiding system packages in the scope of a task. (The VR folks want to do something similar for some of their functional tests.) I don't think this is something we want to bake into swarming's provisioning logic, as it's not something we always want to do. (provision_devices currently handles it w/ a flag.)
,
May 31 2017
I am supportive of making it general for other tests! However, can you file a another bug & mark it as blocker for this bug?
,
May 31 2017
,
Jun 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9ff9c3196bb331d8b97089e61b19888bbe75cfb5 commit 9ff9c3196bb331d8b97089e61b19888bbe75cfb5 Author: catapult-deps-roller@chromium.org <catapult-deps-roller@chromium.org> Date: Tue Jun 13 00:38:37 2017 Roll src/third_party/catapult/ e663166b9..ceedebe21 (3 commits) https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/e663166b9cf3..ceedebe21711 $ git log e663166b9..ceedebe21 --date=short --no-merges --format='%ad %ae %s' 2017-06-12 martiniss Add webview embedded apk flag. 2017-06-12 mikecase Pass output-dir argument to stack script. 2017-06-12 mikecase Adds option to not have SetLogLevel add a log handler. Created with: roll-dep src/third_party/catapult BUG= 725695 ,721684 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, see: http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel TBR=sullivan@chromium.org Change-Id: I0eba150a1a220ca44e6ef346dd6c1ec08df3526b Reviewed-on: https://chromium-review.googlesource.com/531837 Reviewed-by: <catapult-deps-roller@chromium.org> Commit-Queue: <catapult-deps-roller@chromium.org> Cr-Commit-Position: refs/heads/master@{#478843} [modify] https://crrev.com/9ff9c3196bb331d8b97089e61b19888bbe75cfb5/DEPS
,
Jun 14 2017
The following revision refers to this bug: https://chrome-internal.googlesource.com/infradata/master-manager/+/4a022df7daf26ff6b5a2dbdaadc945cf32f0686d commit 4a022df7daf26ff6b5a2dbdaadc945cf32f0686d Author: Stephen Martinis <martiniss@google.com> Date: Wed Jun 14 00:36:33 2017
,
Jun 14 2017
The following revision refers to this bug: https://chrome-internal.googlesource.com/infradata/master-manager/+/4a022df7daf26ff6b5a2dbdaadc945cf32f0686d commit 4a022df7daf26ff6b5a2dbdaadc945cf32f0686d Author: Stephen Martinis <martiniss@google.com> Date: Wed Jun 14 00:36:33 2017
,
Jun 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/76218ac69794372fe5d575fffcd5b60dd6596688 commit 76218ac69794372fe5d575fffcd5b60dd6596688 Author: Stephen Martinis <martiniss@chromium.org> Date: Thu Jun 15 01:29:05 2017 //tools/perf: Fix typo-ed device name I misread the bot name in http://crbug.com/732898. This CL fixes the typo Bug: 725695 Change-Id: I516a8732b7ec7a734e94ef24b7f62288d87ae822 Reviewed-on: https://chromium-review.googlesource.com/536174 Commit-Queue: Stephen Martinis <martiniss@chromium.org> Commit-Queue: Ned Nguyen <nednguyen@google.com> Reviewed-by: Ned Nguyen <nednguyen@google.com> Cr-Commit-Position: refs/heads/master@{#479569} [modify] https://crrev.com/76218ac69794372fe5d575fffcd5b60dd6596688/testing/buildbot/chromium.perf.fyi.json [modify] https://crrev.com/76218ac69794372fe5d575fffcd5b60dd6596688/tools/perf/core/benchmark_sharding_map.json [modify] https://crrev.com/76218ac69794372fe5d575fffcd5b60dd6596688/tools/perf/core/perf_data_generator.py
,
Jun 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b2846d28f16b5d28625b4efbd66cf01ce9bf8d37 commit b2846d28f16b5d28625b4efbd66cf01ce9bf8d37 Author: Stephen Martinis <martiniss@chromium.org> Date: Fri Jun 16 01:45:49 2017 //tools/perf: Fix pool dimension for fyi bot I need to add a check for this as well :/ TBR=nednguyen@google.com NOTRY=true Bug: 725695 Change-Id: Ibf1cee2c5ca0074e6b1b3d54c7bcc64a8218ad5e Reviewed-on: https://chromium-review.googlesource.com/537733 Commit-Queue: Stephen Martinis <martiniss@chromium.org> Reviewed-by: Ned Nguyen <nednguyen@google.com> Cr-Commit-Position: refs/heads/master@{#479888} [modify] https://crrev.com/b2846d28f16b5d28625b4efbd66cf01ce9bf8d37/testing/buildbot/chromium.perf.fyi.json [modify] https://crrev.com/b2846d28f16b5d28625b4efbd66cf01ce9bf8d37/tools/perf/core/perf_data_generator.py
,
Jun 22 2017
The webview builder is cycling green here: https://build.chromium.org/p/chromium.perf.fyi/builders/Android%20Nexus5X%20WebView%20Perf/builds/170 Awesome work!
,
Jun 23 2017
The webview bot is indeed green, per #28. I'll plan to convert the webview bots on Monday. I'll make up the CLs tomorrow. Do we want to compare any data between the existing webview bot and the new swarming bot? Also, I don't see the new fyi webview bot on the perf dashboard :/ I can click on links from the buildbot log, but I don't see it in the auto complete.
,
Jun 23 2017
I'm seeing it in the auto complete:
,
Jun 23 2017
#29: I think we can just compare: 1) The runtime of system_health or loading 2) The result of system_health.memory_mobile
,
Jun 26 2017
1) The run time is comparable Graphs: https://chromeperf.appspot.com/report?sid=10a0399f2d19eec7feef61831e2d7b6a28b9446e2e8299d4f5cd5d0a1b65f615 2) The memory measurement is not comparable. In fact, there is *NO* memory metric output on Swarming Webview Nexus 5X at the moment! browse:news:cnn trace of Android Webview Nexus 5X: https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/trace-file-id_13-2017-04-18_03-10-32-10769.html browse:news:cnn trace of FYI Android Webview Nexus 5X Swarming: https://console.developers.google.com/m/cloudstorage/b/chrome-telemetry-output/o/trace-file-id_9-2017-06-26_12-42-26-9270.html -> there is only memory data on Browser process? Juan: can you help with investigating what could have been wrong here?
,
Jun 26 2017
That is expected, webview typically has a single process so you should only see a single process (i.e. browser). Also, on chromeperf, you should be looking for metrics named "memory:webview:*". For example: https://chromeperf.appspot.com/report?sid=335afd92dc4b6a2b9c1b6ef16cb76a98f64722691ac1da34890cd044a38c9b57 shows that the metrics on two bots are comparable. I guess we could ask for all "memory:chrome:*" metrics on webview bots to be removed to avoid confusion. +Annie, would that be possible?
,
Jun 26 2017
Juan: I guess that make sense. Though how do we explain that the 1st trace and 2nd trace in #32 are so different?
,
Jun 27 2017
I'm guessing that the first trace comes from a time where we were (wrongly) running tests using Chrome on that Webview bot.
,
Jun 28 2017
#35: ah, that makes sense. Thanks Juan! Stephen: I think it's safe to move forward with webview swarming now. :-)
,
Jun 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/40c248d5a7227fc63bb4203c22ae4947f6d5bf30 commit 40c248d5a7227fc63bb4203c22ae4947f6d5bf30 Author: Stephen Martinis <martiniss@chromium.org> Date: Wed Jun 28 23:02:23 2017 //tools/perf: Convert webview bots to swarming This CL also removes the existing FYI webview bot, since there's a name conflict between the FYI bot and the regular bot. NOTRY=true Bug: 725695 Change-Id: I190a593186b3044cc2531cb496974fef91a28b41 Reviewed-on: https://chromium-review.googlesource.com/548997 Commit-Queue: Stephen Martinis <martiniss@chromium.org> Reviewed-by: Ned Nguyen <nednguyen@google.com> Cr-Commit-Position: refs/heads/master@{#483183} [modify] https://crrev.com/40c248d5a7227fc63bb4203c22ae4947f6d5bf30/testing/buildbot/chromium.perf.fyi.json [modify] https://crrev.com/40c248d5a7227fc63bb4203c22ae4947f6d5bf30/testing/buildbot/chromium.perf.json [modify] https://crrev.com/40c248d5a7227fc63bb4203c22ae4947f6d5bf30/tools/perf/core/benchmark_sharding_map.json [modify] https://crrev.com/40c248d5a7227fc63bb4203c22ae4947f6d5bf30/tools/perf/core/perf_data_generator.py
,
Jul 6 2017
This works now!
,
Jul 6 2017
\o/
,
Jul 7 2017
Awesome! |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by jbudorick@chromium.org
, May 23 2017