Commit 88c95c21 authored by Michael Ershov's avatar Michael Ershov Committed by Commit Bot

Fix flaky test DeviceCloudPolicyManagerChromeOSEnrollmentTest

The test failed because sometimes RemoteCommandsService generated
an additional request that was not expected by a mock object.

Fixed by reordering AllowUninterestingRemoteCommandFetches function
calls and EXPECT_CALL(..., StartJob(_)) calls. It looks like the
latter one overrode the former one.

Bug: 1014318
Test: DeviceCloudPolicyManagerChromeOSEnrollmentTest.Success
Change-Id: I9cd8f13c9346623faaabb3c850b292ec10bdd5f6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1924431Reviewed-by: default avatarMaksim Ivanov <emaxx@chromium.org>
Commit-Queue: Michael Ershov <miersh@google.com>
Cr-Commit-Position: refs/heads/master@{#717690}
parent e498edb0
...@@ -261,6 +261,8 @@ class DeviceCloudPolicyManagerChromeOSTest ...@@ -261,6 +261,8 @@ class DeviceCloudPolicyManagerChromeOSTest
EXPECT_TRUE(manager_->policies().Equals(bundle)); EXPECT_TRUE(manager_->policies().Equals(bundle));
} }
// Should be called after EXPECT_CALL(..., StartJob(_)) so "any" case does
// not override this one.
void AllowUninterestingRemoteCommandFetches() { void AllowUninterestingRemoteCommandFetches() {
// We are not interested in remote command fetches that the client initiates // We are not interested in remote command fetches that the client initiates
// automatically. Make them fail and ignore them otherwise. // automatically. Make them fail and ignore them otherwise.
...@@ -338,8 +340,8 @@ TEST_F(DeviceCloudPolicyManagerChromeOSTest, EnrolledDevice) { ...@@ -338,8 +340,8 @@ TEST_F(DeviceCloudPolicyManagerChromeOSTest, EnrolledDevice) {
.WillOnce( .WillOnce(
DoAll(device_management_service_.CaptureJobType(&job_type), DoAll(device_management_service_.CaptureJobType(&job_type),
device_management_service_.StartJobFullControl(&policy_job))); device_management_service_.StartJobFullControl(&policy_job)));
ConnectManager();
AllowUninterestingRemoteCommandFetches(); AllowUninterestingRemoteCommandFetches();
ConnectManager();
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
Mock::VerifyAndClearExpectations(&device_management_service_); Mock::VerifyAndClearExpectations(&device_management_service_);
ASSERT_TRUE(policy_job); ASSERT_TRUE(policy_job);
...@@ -377,8 +379,8 @@ TEST_F(DeviceCloudPolicyManagerChromeOSTest, UnmanagedDevice) { ...@@ -377,8 +379,8 @@ TEST_F(DeviceCloudPolicyManagerChromeOSTest, UnmanagedDevice) {
.WillOnce( .WillOnce(
DoAll(device_management_service_.CaptureJobType(&job_type), DoAll(device_management_service_.CaptureJobType(&job_type),
device_management_service_.StartJobFullControl(&policy_job))); device_management_service_.StartJobFullControl(&policy_job)));
ConnectManager();
AllowUninterestingRemoteCommandFetches(); AllowUninterestingRemoteCommandFetches();
ConnectManager();
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
Mock::VerifyAndClearExpectations(&device_management_service_); Mock::VerifyAndClearExpectations(&device_management_service_);
ASSERT_TRUE(policy_job); ASSERT_TRUE(policy_job);
...@@ -432,9 +434,9 @@ TEST_F(DeviceCloudPolicyManagerChromeOSTest, ConnectAndDisconnect) { ...@@ -432,9 +434,9 @@ TEST_F(DeviceCloudPolicyManagerChromeOSTest, ConnectAndDisconnect) {
DeviceManagementService::JobControl* policy_job = nullptr; DeviceManagementService::JobControl* policy_job = nullptr;
EXPECT_CALL(device_management_service_, StartJob(_)) EXPECT_CALL(device_management_service_, StartJob(_))
.WillOnce(device_management_service_.StartJobFullControl(&policy_job)); .WillOnce(device_management_service_.StartJobFullControl(&policy_job));
AllowUninterestingRemoteCommandFetches();
EXPECT_CALL(*this, OnDeviceCloudPolicyManagerConnected()); EXPECT_CALL(*this, OnDeviceCloudPolicyManagerConnected());
ConnectManager(); ConnectManager();
AllowUninterestingRemoteCommandFetches();
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
Mock::VerifyAndClearExpectations(&device_management_service_); Mock::VerifyAndClearExpectations(&device_management_service_);
Mock::VerifyAndClearExpectations(this); Mock::VerifyAndClearExpectations(this);
...@@ -523,6 +525,7 @@ class DeviceCloudPolicyManagerChromeOSEnrollmentTest ...@@ -523,6 +525,7 @@ class DeviceCloudPolicyManagerChromeOSEnrollmentTest
device_management_service_.CaptureQueryParams(&query_params_), device_management_service_.CaptureQueryParams(&query_params_),
device_management_service_.CaptureRequest(&register_request_), device_management_service_.CaptureRequest(&register_request_),
device_management_service_.StartJobFullControl(&register_job))); device_management_service_.StartJobFullControl(&register_job)));
AllowUninterestingRemoteCommandFetches();
chromeos::OwnerSettingsServiceChromeOS* owner_settings_service = chromeos::OwnerSettingsServiceChromeOS* owner_settings_service =
chromeos::OwnerSettingsServiceChromeOSFactory::GetForBrowserContext( chromeos::OwnerSettingsServiceChromeOSFactory::GetForBrowserContext(
...@@ -544,7 +547,6 @@ class DeviceCloudPolicyManagerChromeOSEnrollmentTest ...@@ -544,7 +547,6 @@ class DeviceCloudPolicyManagerChromeOSEnrollmentTest
initializer_->StartEnrollment(); initializer_->StartEnrollment();
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
Mock::VerifyAndClearExpectations(&device_management_service_); Mock::VerifyAndClearExpectations(&device_management_service_);
AllowUninterestingRemoteCommandFetches();
if (done_) if (done_)
return; return;
...@@ -564,13 +566,13 @@ class DeviceCloudPolicyManagerChromeOSEnrollmentTest ...@@ -564,13 +566,13 @@ class DeviceCloudPolicyManagerChromeOSEnrollmentTest
.WillOnce( .WillOnce(
DoAll(device_management_service_.CaptureJobType(&fetch_job_type), DoAll(device_management_service_.CaptureJobType(&fetch_job_type),
device_management_service_.StartJobFullControl(&fetch_job))); device_management_service_.StartJobFullControl(&fetch_job)));
AllowUninterestingRemoteCommandFetches();
device_management_service_.DoURLCompletion( device_management_service_.DoURLCompletion(
&register_job, &register_job,
register_status_ == DM_STATUS_SUCCESS ? net::OK : net::ERR_FAILED, register_status_ == DM_STATUS_SUCCESS ? net::OK : net::ERR_FAILED,
DeviceManagementService::kSuccess, register_response_); DeviceManagementService::kSuccess, register_response_);
EXPECT_EQ(nullptr, register_job); EXPECT_EQ(nullptr, register_job);
Mock::VerifyAndClearExpectations(&device_management_service_); Mock::VerifyAndClearExpectations(&device_management_service_);
AllowUninterestingRemoteCommandFetches();
if (done_) if (done_)
return; return;
...@@ -597,9 +599,9 @@ class DeviceCloudPolicyManagerChromeOSEnrollmentTest ...@@ -597,9 +599,9 @@ class DeviceCloudPolicyManagerChromeOSEnrollmentTest
DoAll(device_management_service_.CaptureJobType(&robot_job_type), DoAll(device_management_service_.CaptureJobType(&robot_job_type),
device_management_service_.StartJobFullControl( device_management_service_.StartJobFullControl(
&robot_auth_fetch_job))); &robot_auth_fetch_job)));
AllowUninterestingRemoteCommandFetches();
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
Mock::VerifyAndClearExpectations(&device_management_service_); Mock::VerifyAndClearExpectations(&device_management_service_);
AllowUninterestingRemoteCommandFetches();
if (done_) if (done_)
return; return;
...@@ -616,7 +618,6 @@ class DeviceCloudPolicyManagerChromeOSEnrollmentTest ...@@ -616,7 +618,6 @@ class DeviceCloudPolicyManagerChromeOSEnrollmentTest
DeviceManagementService::kSuccess, robot_auth_fetch_response_); DeviceManagementService::kSuccess, robot_auth_fetch_response_);
EXPECT_EQ(nullptr, robot_auth_fetch_job); EXPECT_EQ(nullptr, robot_auth_fetch_job);
Mock::VerifyAndClearExpectations(&device_management_service_); Mock::VerifyAndClearExpectations(&device_management_service_);
AllowUninterestingRemoteCommandFetches();
if (done_) if (done_)
return; return;
...@@ -631,6 +632,7 @@ class DeviceCloudPolicyManagerChromeOSEnrollmentTest ...@@ -631,6 +632,7 @@ class DeviceCloudPolicyManagerChromeOSEnrollmentTest
device_management_service_.CaptureJobType(&component_job_type), device_management_service_.CaptureJobType(&component_job_type),
device_management_service_.StartJobFullControl( device_management_service_.StartJobFullControl(
&component_fetch_job))); &component_fetch_job)));
AllowUninterestingRemoteCommandFetches();
// Process robot refresh token fetch if the auth code fetch succeeded. // Process robot refresh token fetch if the auth code fetch succeeded.
// DeviceCloudPolicyInitializer holds an EnrollmentHandlerChromeOS which // DeviceCloudPolicyInitializer holds an EnrollmentHandlerChromeOS which
...@@ -675,6 +677,7 @@ class DeviceCloudPolicyManagerChromeOSEnrollmentTest ...@@ -675,6 +677,7 @@ class DeviceCloudPolicyManagerChromeOSEnrollmentTest
.WillRepeatedly(device_management_service_.StartJobAsync( .WillRepeatedly(device_management_service_.StartJobAsync(
net::OK, DeviceManagementService::kSuccess, net::OK, DeviceManagementService::kSuccess,
em::DeviceManagementResponse())); em::DeviceManagementResponse()));
AllowUninterestingRemoteCommandFetches();
ReloadDeviceSettings(); ReloadDeviceSettings();
...@@ -732,8 +735,7 @@ class DeviceCloudPolicyManagerChromeOSEnrollmentTest ...@@ -732,8 +735,7 @@ class DeviceCloudPolicyManagerChromeOSEnrollmentTest
DISALLOW_COPY_AND_ASSIGN(DeviceCloudPolicyManagerChromeOSEnrollmentTest); DISALLOW_COPY_AND_ASSIGN(DeviceCloudPolicyManagerChromeOSEnrollmentTest);
}; };
// Flaky. https://crbug.com/1014318 TEST_P(DeviceCloudPolicyManagerChromeOSEnrollmentTest, Success) {
TEST_P(DeviceCloudPolicyManagerChromeOSEnrollmentTest, DISABLED_Success) {
RunTest(); RunTest();
ExpectSuccessfulEnrollment(); ExpectSuccessfulEnrollment();
} }
...@@ -835,6 +837,7 @@ TEST_P(DeviceCloudPolicyManagerChromeOSEnrollmentTest, ...@@ -835,6 +837,7 @@ TEST_P(DeviceCloudPolicyManagerChromeOSEnrollmentTest,
EXPECT_CALL(device_management_service_, StartJob(_)) EXPECT_CALL(device_management_service_, StartJob(_))
.WillOnce(DoAll(device_management_service_.CaptureJobType(&job_type), .WillOnce(DoAll(device_management_service_.CaptureJobType(&job_type),
device_management_service_.StartJobOKAsync(response))); device_management_service_.StartJobOKAsync(response)));
AllowUninterestingRemoteCommandFetches();
EXPECT_CALL(*this, OnUnregistered(true)); EXPECT_CALL(*this, OnUnregistered(true));
// Start unregistering. // Start unregistering.
...@@ -858,6 +861,7 @@ TEST_P(DeviceCloudPolicyManagerChromeOSEnrollmentTest, UnregisterFails) { ...@@ -858,6 +861,7 @@ TEST_P(DeviceCloudPolicyManagerChromeOSEnrollmentTest, UnregisterFails) {
.WillOnce(DoAll(device_management_service_.CaptureJobType(&job_type), .WillOnce(DoAll(device_management_service_.CaptureJobType(&job_type),
device_management_service_.StartJobAsync( device_management_service_.StartJobAsync(
net::ERR_FAILED, DeviceManagementService::kSuccess))); net::ERR_FAILED, DeviceManagementService::kSuccess)));
AllowUninterestingRemoteCommandFetches();
EXPECT_CALL(*this, OnUnregistered(false)); EXPECT_CALL(*this, OnUnregistered(false));
// Start unregistering. // Start unregistering.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment