browser_tests WebDriverSitePerProcessPolicyBrowserTest.Simple failing in official builds |
||||||
Issue descriptionExample builds: https://ci.chromium.org/buildbot/chromium.clang/ToTWin/1405 https://uberchromegw.corp.google.com/i/official.desktop.continuous/builders/win%20trunk/builds/83245 [ RUN ] WebDriverSitePerProcessPolicyBrowserTest.Simple [6000:5256:0424/050648.157:WARNING:discovery_network_list_win.cc(195)] Failed to open Wlan client handle: 1062 [6000:3884:0424/050648.165:WARNING:chrome_browser_main_win.cc(630)] Command line too long for RegisterApplicationRestart: --brave-new-test-launcher --cfi-diag=0 --gtest_also_run_disabled_tests --gtest_filter=WebDriverSitePerProcessPolicyBrowserTest.Simple --single_process --test-launcher-bot-mode --test-launcher-output="C:\Users\CHROME~2\AppData\Local\Temp\scoped_dir3220_20783\results3220_26115\test_results.xml" --test-launcher-summary-output="e:\b\swarm_slave\w\ioxxow4e\output.json" --user-data-dir="C:\Users\CHROME~2\AppData\Local\Temp\scoped_dir3220_20783\d3220_14743" --disable-offline-auto-reload --no-first-run --no-default-browser-check --enable-logging=stderr --disable-default-apps --wm-window-animations-disabled --disable-component-update --test-type=browser --force-color-profile=srgb --disable-zero-browsers-open-for-tests --ipc-connection-timeout=30 --allow-file-access-from-files --dom-automation --log-gpu-control-list-decisions --disable-backgrounding-occluded-windows --disable-gl-drawing-for-tests --override-use-software-gl-for-tests --force-color-profile=srgb --disable-compositor-ukm-for-tests --enable-automation --enable-features=TestFeatureForBrowserTest1 --disable-features=NetworkPrediction,TestFeatureForBrowserTest2 --flag-switches-begin --flag-switches-end --restore-last-session about:blank ../../chrome/browser/policy/site_isolation_policy_browsertest.cc(40): error: Expected equality of these values: expectations[i].isolated Which is: true instance->RequiresDedicatedProcess() Which is: false Stack trace: Backtrace: StackTraceGetter::CurrentStackTrace [0x00F412F8+40] testing::internal::UnitTestImpl::CurrentOsStackTraceExceptTop [0x00F49F53+69] testing::internal::AssertHelper::operator= [0x00F49BB8+48] SiteIsolationPolicyBrowserTest::CheckExpectations [0x0069483A+234] WebDriverSitePerProcessPolicyBrowserTest_Simple_Test::RunTestOnMainThread [0x00694B04+52] content::BrowserTestBase::ProxyRunTestOnMainThreadLoop [0x025B16B5+357] ChromeBrowserMainParts::PreMainMessageLoopRunImpl [0x02B38F22+3300] ChromeBrowserMainParts::PreMainMessageLoopRun [0x02B38181+145] content::BrowserMainLoop::PreMainMessageLoopRun [0x016CF5B4+68] content::StartupTaskRunner::RunAllTasksNow [0x01943675+23] content::BrowserMainLoop::CreateStartupTasks [0x016CE669+615] content::BrowserMainRunnerImpl::Initialize [0x016D1384+100] content::BrowserMain [0x016CCC3A+138] content::RunNamedProcessTypeMain [0x0243A493+123] content::ContentMainRunnerImpl::Run [0x0243A88E+142] service_manager::Main [0x035A69CA+622] content::ContentMain [0x0243A3BF+51] content::BrowserTestBase::SetUp [0x025B148A+1498]
,
Apr 24 2018
Disabling the test here: https://chromium-review.googlesource.com/#/c/chromium/src/+/1026113
,
Apr 24 2018
Oops, I put in the wrong bug number in the disabling CL, but it landed here: The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/28f6d78122d4499b85ae5db4dd2962ee24048165 commit 28f6d78122d4499b85ae5db4dd2962ee24048165 Author: Hans Wennborg <hans@chromium.org> Date: Tue Apr 24 17:07:03 2018 Disable WebDriverSitePerProcessPolicyBrowserTest.Simple in official builds The test fails in official builds (see bug). Probably this is due to fieldtrial_testing_config.json not affecting official builds, and so the feature needs to be explicitly enabled in the test fixture, perhaps similarly to https://crrev.com/452026 Bug: 834010 Change-Id: If0909e90d03f05596cc44f3a1022a0f5eca66762 Reviewed-on: https://chromium-review.googlesource.com/1026113 Reviewed-by: Nico Weber <thakis@chromium.org> Commit-Queue: Hans Wennborg <hans@chromium.org> Cr-Commit-Position: refs/heads/master@{#553173} [modify] https://crrev.com/28f6d78122d4499b85ae5db4dd2962ee24048165/chrome/browser/policy/site_isolation_policy_browsertest.cc Assigning to lukasza.
,
Apr 24 2018
pastarmovj@, could you PTAL? The trouble is that when WebDriverSitePerProcessPolicyBrowserTest::SetUpInProcessBrowserTestFixture runs the trial simulations from testing/variations/fieldtrial_testing_config.json have not yet kicked in (for example at that point base::FeatureList::IsEnabled(features::kSitePerProcess) would return false). This makes it difficult to distinguish at that point between the unofficial/ToT build (where site-per-process is turned on by default) and the official build (where site-per-process is supposed to be off). I can't think of any solution other than just disabling the test on official builds (as done in #c3 - thanks hans@!).
,
Apr 25 2018
To be honest I don't have a better idea how to do this. It is a little confusing that we run test with forced site isolation mode and now on top of this changing the default might make this hard. If there is a way to know we are running on official builders inside the code instead of disabling the test we can change the expectation maybe. Do you know if this is possible?
,
Apr 25 2018
RE: is there a way to know we are running on official builders inside the code [then] instead of disabling the test we can change the expectation Hmmm... we could use #if defined(OFFICIAL_BUILD) to change how WebDriverSitePerProcessPolicyBrowserTest::are_sites_isolated_for_testing_ is calculated. I've put together https://crbug.com/c/1028227. The tricky part here is that CQ doesn't cover official builds so we probably have to 1) be careful during code review and 2) monitor official builders once the CL lands.
,
Apr 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/953d7f91ca9ba8ff413482a425b0eaea5c45856b commit 953d7f91ca9ba8ff413482a425b0eaea5c45856b Author: Lukasz Anforowicz <lukasza@chromium.org> Date: Thu Apr 26 11:12:46 2018 Default state of site isolation needs to consider official builds. WebDriverSitePerProcessPolicyBrowserTest attempts to calculate the default state of site isolation before the test starts. This calculation needs to take into account the official builds, where testing/variations/fieldtrial_testing_config.json does not apply. Bug: 836261 Change-Id: I0526969892a39a54a88258d00ee2e66625aa35f8 Reviewed-on: https://chromium-review.googlesource.com/1028227 Commit-Queue: Julian Pastarmov <pastarmovj@chromium.org> Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org> Cr-Commit-Position: refs/heads/master@{#553977} [modify] https://crrev.com/953d7f91ca9ba8ff413482a425b0eaea5c45856b/chrome/browser/policy/site_isolation_policy_browsertest.cc
,
Apr 26 2018
The change from #c7 has been included in https://uberchromegw.corp.google.com/i/official.desktop.continuous/builders/win%20trunk/builds/83376 and there were no failures in WebDriverSitePerProcessPolicyBrowserTest in this and subsequent builds. So - I think it is okay to mark this bug as fixed.
,
May 1 2018
,
May 1 2018
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by h...@chromium.org
, Apr 24 2018