MultideviceSetup tests failing with Polymer 2 |
||||||
Issue descriptionThe following tests are failing when run with --enable-features=WebUIPolymer2: MultiDeviceSetupBrowserTest.Integration MultiDeviceSetupBrowserTest.StartSetupPage These tests should be fixed to work correctly with Polymer 2 and removed from the exclusions list in webui_polymer2_browser_tests.filter.
,
Aug 24
,
Aug 24
For context, see the CL description of https://chromium-review.googlesource.com/c/chromium/src/+/1184315 which addressed this issue. Note that, in order to see the solution to the Polymer 2 issue, you have to compare the merged version with patchset 1 rather than the base.
,
Sep 5
@jordynass: Is there anything left to do for this bug?
,
Sep 5
They still are not fixed for Polymer 2 but I think we have a number of bugs for high priority blockers. Can it was until we've finished our release blockers for the Unified Setup project?
,
Sep 10
> Can it was until we've finished our release blockers for the Unified Setup project? What is the target date for the release blockers you are mentioning? It is hard to answer this question without specific targets. From the Polymer 2 migration side, the original target for finishing was M71. If we keep postponing ChromeOS fixes, this will most likely slip to M72+. The problem is not only that waiting more delays the Polymer 2 migration, but also that there is potential for new Multidevice code being added that directly conflicts with the Polymer 2 migration. For example the addition of new "/deep/" usage (see [1]), which we are trying to remove across all WebUI as part of [2]. Getting the test passing sooner, rather than later, ensures that newly added Multidevice code does not conflict with the migration. [1] https://cs.chromium.org/chromium/src/chrome/test/data/webui/multidevice_setup/start_setup_page_test.js?l=50,63 [2] https://bugs.chromium.org/p/chromium/issues/detail?id=860069
,
Sep 10
FYI candidate fix for MultiDeviceSetupBrowserTest.StartSetupPage is at https://chromium-review.googlesource.com/c/chromium/src/+/1217026.
,
Sep 10
It seems that MultiDeviceSetupBrowserTest.Integration also fails because of /deep/ usage, see https://cs.chromium.org/chromium/src/chrome/test/data/webui/multidevice_setup/integration_test.js?l=38,40.
,
Sep 10
Our target is now M71, so we can prioritize this in that timeframe. Jordy, consider this an in-between-things task.
,
Sep 10
As I was investigating how hard fixing those tests would be, I ended up fixing both at https://chromium-review.googlesource.com/c/chromium/src/+/1217026. Assigning to myself for now, and will send a review. Thanks!
,
Sep 10
You're awesome, Demetrios :-). Thanks for helping out on this!
,
Sep 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/00f8d38de1290ea5e4556c0110248b19cd64c593 commit 00f8d38de1290ea5e4556c0110248b19cd64c593 Author: dpapad <dpapad@chromium.org> Date: Tue Sep 11 01:58:16 2018 Multidevice: Fix failing MultiDeviceSetupBrowserTest.* for Polymer 2. Removes /deep/ usage, which no longer works in Shadow DOM v1. Also fixes some test timing that caused part of the failures. Bug: 875505 ,860069 Change-Id: I5642105a6df148bdea2d02b8e1af8889f400c89d Reviewed-on: https://chromium-review.googlesource.com/1217026 Reviewed-by: Kyle Horimoto <khorimoto@chromium.org> Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org> Cr-Commit-Position: refs/heads/master@{#590171} [modify] https://crrev.com/00f8d38de1290ea5e4556c0110248b19cd64c593/chrome/test/data/webui/multidevice_setup/integration_test.js [modify] https://crrev.com/00f8d38de1290ea5e4556c0110248b19cd64c593/chrome/test/data/webui/multidevice_setup/start_setup_page_test.js [modify] https://crrev.com/00f8d38de1290ea5e4556c0110248b19cd64c593/testing/buildbot/filters/webui_polymer2_browser_tests.filter
,
Sep 11
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by rbpotter@chromium.org
, Aug 17