Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Issue 721691 Leftover registry entries on bots cause subsequent tests to fail
Starred by 1 user Project Member Reported by chromium...@appspot.gserviceaccount.com, May 12 Back to list
Status: Fixed
Owner:
Closed: May 19
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 0
Type: Bug



Sign in to add a comment
"CloudPolicyManagerTest.RegisterFails" is flaky.

This issue was created automatically by the chromium-try-flakes app. Please find the right owner to fix the respective test/step and assign this issue to them. If the step/test is infrastructure-related, please add Infra-Troopers label and change issue status to Untriaged. When done, please remove the issue from Sheriff Bug Queue by removing the Sheriff-Chromium label.

We have detected 3 recent flakes. List of all flakes can be found at https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyLwsSBUZsYWtlIiRDbG91ZFBvbGljeU1hbmFnZXJUZXN0LlJlZ2lzdGVyRmFpbHMM.

Flaky tests should be disabled within 30 minutes unless culprit CL is found and reverted. Please see more details here: https://sites.google.com/a/chromium.org/dev/developers/tree-sheriffs/sheriffing-bug-queues#triaging-auto-filed-flakiness-bugs
 
Labels: -Sheriff-Chromium
Owner: joaodasilva@chromium.org
Status: Assigned
Cc: tnagel@chromium.org bartfab@chromium.org
Owner: atwilson@chromium.org
Cc: pastarmovj@chromium.org
This is due to a misconfigured bot that has policies configured on the machine for some reason. I'm reaching out to troopers to fix it.
Error is:

[ RUN      ] CloudPolicyManagerTest.RegisterFails
[4660:5036:0512/062713.566:ERROR:configuration_policy_handler_list.cc(92)] Unknown policy: StringPolicy
[4660:5036:0512/062713.616:ERROR:configuration_policy_handler_list.cc(92)] Unknown policy: StringPolicy
[4356:4724:0512/062713.717:INFO:media_foundation_video_encode_accelerator_win.cc(329)] Windows versions earlier than 8 are not supported.
[4660:5036:0512/062713.868:WARNING:policy_test_utils.cc(52)] There are pre-existing policies in this machine: {
   "StringPolicy": "hklm"
}
e:\b\c\b\win\src\chrome\browser\policy\cloud\cloud_policy_manager_browsertest.cc(65): error: Value of: PolicyServiceIsEmpty(g_browser_process->policy_service())
  Actual: false
Expected: true
Pre-existing policies in this machine will make this test fail.
Backtrace:
	policy::CloudPolicyManagerTest_RegisterFails_Test::RunTestOnMainThread [0x006B3799+41]
	content::BrowserTestBase::ProxyRunTestOnMainThreadLoop [0x01C34E69+233]
	ChromeBrowserMainParts::PreMainMessageLoopRunImpl [0x02381B67+3320]
	ChromeBrowserMainParts::PreMainMessageLoopRun [0x02380E37+167]
	content::BrowserMainLoop::PreMainMessageLoopRun [0x00DEB8D0+112]
	content::StartupTaskRunner::RunAllTasksNow [0x0104EFC9+35]
	content::BrowserMainLoop::CreateStartupTasks [0x00DE986E+394]
	content::BrowserMainRunnerImpl::Initialize [0x00DECAB0+704]
	content::BrowserMain [0x00DE83A5+117]
	content::RunNamedProcessTypeMain [0x01AAD43C+206]
	content::ContentMainRunnerImpl::Run [0x01AAD33D+413]
	service_manager::Main [0x02A12696+524]
	content::ContentMain [0x01AAC9E8+39]
	content::BrowserTestBase::SetUp [0x01C35392+866]
	InProcessBrowserTest::SetUp [0x01B7EF36+310]
	testing::internal::HandleExceptionsInMethodIfSupported<testing::TestCase,void> [0x00A43FAD+32]
	testing::Test::Run [0x00A4B09F+51]
	testing::TestInfo::Run [0x00A4B24A+126]
	testing::TestCase::Run [0x00A4B175+133]
	testing::internal::UnitTestImpl::RunAllTests [0x00A4B501+433]
	testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,bool> [0x00A43FF1+32]
	testing::UnitTest::Run [0x00A4B31E+133]
	base::TestSuite::Run [0x01B8E786+95]
	ChromeTestSuiteRunner::RunTestSuite [0x040958F8+40]
	content::LaunchTests [0x01C2DFC0+583]
	LaunchChromeTests [0x040958B4+62]
	main [0x040956C9+63]
	__scrt_common_main_seh [0x0404534B+249] (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:253)
	BaseThreadInitThunk [0x7705337A+18]
	RtlInitializeExceptionChain [0x775892B2+99]
	RtlInitializeExceptionChain [0x77589285+54]
Cc: emaxx@chromium.org
 Issue 721694  has been merged into this issue.
Components: Infra>Labs
This is a request to check os:Windows-7-SP1, gpu:none Swarming bots for consistency.
It looks like the offending registry key is  HKEY_LOCAL_MACHINE\SOFTWARE\Policies\Chromium /v StringPolicy

It was found and deleted on 4 swarming slaves: vm33-m4 vm84-m4 vm359-m4 vm438-m4

Labels: -Pri-1 Pri-0
This is causing major flakes on the CQ and makes the diagnosis of another P0 (issue 721245) harder.

Latest occurence: https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%281%29/builds/67193
Cc: pmarko@chromium.org atwilson@chromium.org
Owner: pschmidt@chromium.org
Summary: Leftover registry entries on bots cause subsequent tests to fail (was: "CloudPolicyManagerTest.RegisterFails" is flaky)
Reassigning to pschmidt to clean up this bot too.

Anyone have any idea where these reg entries are coming from? I have to presume it's some test not getting cleaned up (maybe after a failure) - what's the mechanism by which we clean up leftover registry entries after tests?
Cc: -pastarmovj@chromium.org pschmidt@chromium.org
Owner: pastarmovj@chromium.org
Julian, looks like it's the policy_loader_win_unittest.cc class leaving this behind:

https://cs.chromium.org/chromium/src/components/policy/core/common/policy_loader_win_unittest.cc?rcl=e1fa3b55db0f380541270431929002efd8b4ebb5&l=807

What's supposed to clean up after this test?
Hmm good question. I will disable this test for now and refernce this bug until we find a better way to do that. I think the installer tests have some good way to virtualize the registry operations.
Cc: grt@chromium.org
Hrm, policy_loader_win_unittest.cc seems to use RegOverridePredefKey to re-map the HKLM / HKCU registry keys to temporary registry hives created for that unittast. MSDN says that this is a per-process override.

I've seen these flaky tests in browser_tests, but the override only happens in unit_tests - how could this be? Would it mean that the original re-direction fails so the test actually makes real changes to registry which persist on the machine?

BTW, it seems a bit similar to  bug 688878 .
+grt who had a CL to improve the registry key override logic there.
Owner: pmarko@chromium.org
After offline discussion with pastarmovj@, we think we found the reason, same as in  bug 688878 :
The tests just wrap the registry key overriding mechanism in ASSERT_* macros, but this happens in subroutines, so if it fails, we continue writing to the realy registry keys.

Secondly, there is the possibility that if deleting the "temp" keys fails, these will be re-used by another tests, because the temp keys are keyed by process ID.

So, we will change two things:
(1) ensure that nothing is written (tests fail hard) if registry key override fails (ASSERT_NO_FATAL_FAILURE like in grt@'s CL referenced above)
(2) if temp registry keys exist from a previous test, first delete them before re-creating them.
Status: Started
Project Member Comment 15 by bugdroid1@chromium.org, May 19
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1059e59ba99809a03a245e7f55a98efba63567d6

commit 1059e59ba99809a03a245e7f55a98efba63567d6
Author: pmarko <pmarko@chromium.org>
Date: Fri May 19 13:23:01 2017

Fail tests fast if Windows registry key override fails

policy_loader_win_unittest uses RegOverridePredefKey to remap registry
keys to a temporary key for the duration of the test. If this remapping
fails, the test should fail fast to avoid accidentally creating
persistent changes in the machine's registry.
This is done by moving the remap step into the test's SetUp() method.
gtest documentation says that the test will not continue if an ASSERT
fails in SetUp()[1][2].
Also, we now delete the temporary registry keys if they already exist.
These are keyed by the process id, so a subsequent test could reuse an
existing temporary key if the previous test (with the same process id)
crashed and failed to clean up.

[1] https://github.com/google/googletest/blob/master/googletest/docs/Primer.md#invoking-the-tests
[2] https://cs.chromium.org/chromium/src/third_party/mesa/src/src/gtest/src/gtest.cc?rcl=ef811c6bd4de74e13e7035ca882cc77f85793fef&l=2159

BUG= 721691 

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

[modify] https://crrev.com/1059e59ba99809a03a245e7f55a98efba63567d6/components/policy/core/common/configuration_policy_provider_test.cc
[modify] https://crrev.com/1059e59ba99809a03a245e7f55a98efba63567d6/components/policy/core/common/policy_loader_win_unittest.cc

Status: Fixed
 Issue 722246  has been merged into this issue.
Sign in to add a comment