Skip to content

GitLab

T
tangled
Commit bb5659c5 authored by James Cook's avatar James Cook Committed by Commit Bot
Browse files
Options

Revert "cros: Fix SyncConsentTest when SplitSettingsSync is enabled" 

This reverts commit 8f16e273.

Reason for revert:
It doesn't fully fix the test when SplitSettingsSync is enabled.
I missed one case, but I think it's easier to revert and reland.

../../chrome/browser/chromeos/login/test/js_checker.cc:110: Failure
Value of: GetBool(expression)
  Actual: true
Expected: false
document.getElementById('sync-consent-impl').$$('#syncConsentOverviewDialog').hidden
Stack trace:
#0 0x55a3b204a99a chromeos::test::JSChecker::ExpectFalse()
#1 0x55a3b204c317 chromeos::test::JSChecker::ExpectVisiblePath()
#2 0x55a3aff35571 chromeos::(anonymous namespace)::SyncConsentTest::SyncConsentRecorderTestImpl()
#3 0x55a3aff35193 chromeos::(anonymous namespace)::SyncConsentTest_SyncConsentRecorder_Test::RunTestOnMainThread()
#4 0x55a3b1f0f111 content::BrowserTestBase::ProxyRunTestOnMainThreadLoop()
#5 0x55a3b195276c ChromeBrowserMainParts::PreMainMessageLoopRunImpl()
#6 0x55a3b19519f9 ChromeBrowserMainParts::PreMainMessageLoopRun()
#7 0x55a3b1188f84 chromeos::ChromeBrowserMainPartsChromeos::PreMainMessageLoopRun()
#8 0x7fa48e51216a content::BrowserMainLoop::PreMainMessageLoopRun()
#9 0x7fa48eaa2895 content::StartupTaskRunner::RunAllTasksNow()
#10 0x7fa48e5111b3 content::BrowserMainLoop::CreateStartupTasks()
#11 0x7fa48e513ee7 content::BrowserMainRunnerImpl::Initialize()
#12 0x7fa48e50f336 content::BrowserMain()
#13 0x7fa48ef7d613 content::ContentMainRunnerImpl::RunServiceManager()
#14 0x7fa48ef7d19c content::ContentMainRunnerImpl::Run()
#15 0x7fa482f36c44 service_manager::Main()
#16 0x7fa48ef7b5a4 content::ContentMain()
#17 0x55a3b1f0ebc8 content::BrowserTestBase::SetUp()
#18 0x55a3b18dd9b0 InProcessBrowserTest::SetUp()

Original change's description:
> cros: Fix SyncConsentTest when SplitSettingsSync is enabled
> 
> SplitSettingsSync adds some text to the OOBE sync consent dialog.
> Update test expectations to match, otherwise the test will fail when
> we flip the flag.
> 
> Bug: 1067212
> Test: browser_tests --enable-features=SplitSettingsSync
> Change-Id: Iaed6053700c1cd2d5cbaadd29443cf564a7d60b5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2136881
> Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
> Auto-Submit: James Cook <jamescook@chromium.org>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#756667}

TBR=jamescook@chromium.org,xiyuan@chromium.org

Change-Id: Id5fedc0af71bb3e6d221582a61c377bbdbd1a08a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1067212
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2137671Reviewed-by: default avatarJames Cook <jamescook@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756740}
parent f494f22f
Hide whitespace changes
Inline Side-by-side
Showing with 21 additions and 55 deletions
chrome/browser/chromeos/login/screens/sync_consent_browsertest.cc
View file @ bb5659c5
...@@ -97,29 +97,6 @@ class SyncConsentTest : public OobeBaseTest { ...@@ -97,29 +97,6 @@ class SyncConsentTest : public OobeBaseTest {
void SetUpOnMainThread() override { void SetUpOnMainThread() override {
OobeBaseTest::SetUpOnMainThread(); OobeBaseTest::SetUpOnMainThread();
branded_build_override_ = WizardController::ForceBrandedBuildForTesting(); branded_build_override_ = WizardController::ForceBrandedBuildForTesting();
if (features::IsSplitSettingsSyncEnabled()) {
expected_consent_ids_ = {
IDS_LOGIN_SYNC_CONSENT_SCREEN_TITLE,
IDS_LOGIN_SYNC_CONSENT_SCREEN_OS_SYNC_NAME,
IDS_LOGIN_SYNC_CONSENT_SCREEN_OS_SYNC_DESCRIPTION,
IDS_LOGIN_SYNC_CONSENT_SCREEN_CHROME_SYNC_NAME,
IDS_LOGIN_SYNC_CONSENT_SCREEN_CHROME_SYNC_DESCRIPTION,
IDS_LOGIN_SYNC_CONSENT_SCREEN_REVIEW_BROWSER_SYNC_OPTIONS,
IDS_LOGIN_SYNC_CONSENT_SCREEN_PERSONALIZE_GOOGLE_SERVICES_NAME,
IDS_LOGIN_SYNC_CONSENT_SCREEN_PERSONALIZE_GOOGLE_SERVICES_DESCRIPTION,
IDS_LOGIN_SYNC_CONSENT_SCREEN_ACCEPT_AND_CONTINUE,
};
} else {
expected_consent_ids_ = {
IDS_LOGIN_SYNC_CONSENT_SCREEN_TITLE,
IDS_LOGIN_SYNC_CONSENT_SCREEN_CHROME_SYNC_NAME,
IDS_LOGIN_SYNC_CONSENT_SCREEN_CHROME_SYNC_DESCRIPTION,
IDS_LOGIN_SYNC_CONSENT_SCREEN_PERSONALIZE_GOOGLE_SERVICES_NAME,
IDS_LOGIN_SYNC_CONSENT_SCREEN_PERSONALIZE_GOOGLE_SERVICES_DESCRIPTION,
IDS_LOGIN_SYNC_CONSENT_SCREEN_REVIEW_SYNC_OPTIONS_LATER,
IDS_LOGIN_SYNC_CONSENT_SCREEN_ACCEPT_AND_CONTINUE,
};
}
} }
void TearDownOnMainThread() override { void TearDownOnMainThread() override {
...@@ -197,7 +174,7 @@ class SyncConsentTest : public OobeBaseTest { ...@@ -197,7 +174,7 @@ class SyncConsentTest : public OobeBaseTest {
consent_recorded_waiter.consent_description_strings_); consent_recorded_waiter.consent_description_strings_);
EXPECT_EQ(expected_consent_confirmation_string, EXPECT_EQ(expected_consent_confirmation_string,
consent_recorded_waiter.consent_confirmation_string_); consent_recorded_waiter.consent_confirmation_string_);
EXPECT_EQ(expected_consent_ids_, EXPECT_EQ(expected_consent_ids,
consent_recorded_waiter.consent_description_ids_); consent_recorded_waiter.consent_description_ids_);
EXPECT_EQ(expected_consent_confirmation_id, EXPECT_EQ(expected_consent_confirmation_id,
consent_recorded_waiter.consent_confirmation_id_); consent_recorded_waiter.consent_confirmation_id_);
...@@ -205,14 +182,23 @@ class SyncConsentTest : public OobeBaseTest { ...@@ -205,14 +182,23 @@ class SyncConsentTest : public OobeBaseTest {
std::vector<std::string> GetLocalizedExpectedConsentStrings() const { std::vector<std::string> GetLocalizedExpectedConsentStrings() const {
std::vector<std::string> result; std::vector<std::string> result;
for (const int& id : expected_consent_ids_) { for (const int& id : expected_consent_ids) {
result.push_back(GetLocalizedConsentString(id)); result.push_back(GetLocalizedConsentString(id));
} }
return result; return result;
} }
const std::vector<int> expected_consent_ids = {
IDS_LOGIN_SYNC_CONSENT_SCREEN_TITLE,
IDS_LOGIN_SYNC_CONSENT_SCREEN_CHROME_SYNC_NAME,
IDS_LOGIN_SYNC_CONSENT_SCREEN_CHROME_SYNC_DESCRIPTION,
IDS_LOGIN_SYNC_CONSENT_SCREEN_PERSONALIZE_GOOGLE_SERVICES_NAME,
IDS_LOGIN_SYNC_CONSENT_SCREEN_PERSONALIZE_GOOGLE_SERVICES_DESCRIPTION,
IDS_LOGIN_SYNC_CONSENT_SCREEN_REVIEW_SYNC_OPTIONS_LATER,
IDS_LOGIN_SYNC_CONSENT_SCREEN_ACCEPT_AND_CONTINUE,
};
std::unique_ptr<base::AutoReset<bool>> branded_build_override_; std::unique_ptr<base::AutoReset<bool>> branded_build_override_;
std::vector<int> expected_consent_ids_;
FakeGaiaMixin fake_gaia_{&mixin_host_, embedded_test_server()}; FakeGaiaMixin fake_gaia_{&mixin_host_, embedded_test_server()};
private: private:
...@@ -223,35 +209,15 @@ IN_PROC_BROWSER_TEST_F(SyncConsentTest, SyncConsentRecorder) { ...@@ -223,35 +209,15 @@ IN_PROC_BROWSER_TEST_F(SyncConsentTest, SyncConsentRecorder) {
EXPECT_EQ(g_browser_process->GetApplicationLocale(), "en-US"); EXPECT_EQ(g_browser_process->GetApplicationLocale(), "en-US");
LoginToSyncConsentScreen(); LoginToSyncConsentScreen();
// For En-US we hardcode strings here to catch string issues too. // For En-US we hardcode strings here to catch string issues too.
std::vector<std::string> expected_consent_strings; const std::vector<std::string> expected_consent_strings(
if (features::IsSplitSettingsSyncEnabled()) { {"You're signed in!", "Chrome sync",
expected_consent_strings = { "Your bookmarks, history, passwords, and other settings will be synced "
"You're signed in!", "to your Google Account so you can use them on all your devices.",
"Settings sync", "Personalize Google services",
"Your apps, settings, and other customizations will sync across all " "Google may use your browsing history to personalize Search, ads, and "
"Chrome OS devices signed in with your Google Account.", "other Google services. You can change this anytime at "
"Chrome sync", "myaccount.google.com/activitycontrols/search",
"Your bookmarks, history, passwords, and other settings will be synced " "Review sync options following setup", "Accept and continue"});
"to your Google Account so you can use them on all your devices.",
"Review browser sync options following setup",
"Personalize Google services",
"Google may use your browsing history to personalize Search, ads, and "
"other Google services. You can change this anytime at "
"myaccount.google.com/activitycontrols/search",
"Accept and continue"};
} else {
expected_consent_strings = {
"You're signed in!",
"Chrome sync",
"Your bookmarks, history, passwords, and other settings will be synced "
"to your Google Account so you can use them on all your devices.",
"Personalize Google services",
"Google may use your browsing history to personalize Search, ads, and "
"other Google services. You can change this anytime at "
"myaccount.google.com/activitycontrols/search",
"Review sync options following setup",
"Accept and continue"};
}
const std::string expected_consent_confirmation_string = const std::string expected_consent_confirmation_string =
"Accept and continue"; "Accept and continue";
SyncConsentRecorderTestImpl(expected_consent_strings, SyncConsentRecorderTestImpl(expected_consent_strings,
......
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