Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2747293003:
Added profile deletion browsertests. (Closed)
DescriptionAdded profile deletion browsertests.
BUG=
R=bauerb@chromium.org, anthonyvd@chromium.org, mlerman@chromium.org
Review-Url: https://codereview.chromium.org/2747293003
Cr-Commit-Position: refs/heads/master@{#458719}
Committed: https://chromium.googlesource.com/chromium/src/+/dd76f5f150fbaf1a277b8912c1a15e72e96078b5
Patch Set 1 #
Total comments: 4
Patch Set 2 : Review fixes. Test tweaks. #Patch Set 3 : Resolved object files name conflict. #Patch Set 4 : Added delay for browser activation at OpenNewWindowForProfile test. #Patch Set 5 : Disabled failing check on Macs. #Patch Set 6 : Disabled tests for CrOS. #Patch Set 7 : Rebased patch version. #Patch Set 8 : Added delay till profile BrowsingData is removed. #Patch Set 9 : Fixed ProfileHelperTest test flakiness. #
Messages
Total messages: 62 (35 generated)
Thanks! https://codereview.chromium.org/2747293003/diff/1/chrome/browser/profiles/pro... File chrome/browser/profiles/profile_manager_browsertest.cc (right): https://codereview.chromium.org/2747293003/diff/1/chrome/browser/profiles/pro... chrome/browser/profiles/profile_manager_browsertest.cc:79: keep_alive_.reset(new ScopedKeepAlive(KeepAliveOrigin::PROFILE_HELPER, Can you use base::MakeUnique<ScopedKeepAlive>() and directly assign to |keep_alive_|? https://codereview.chromium.org/2747293003/diff/1/chrome/browser/profiles/pro... chrome/browser/profiles/profile_manager_browsertest.cc:119: void set_on_profile_removal_callback(base::Closure callback) { Nit: Pass the callback by const-ref. https://codereview.chromium.org/2747293003/diff/1/chrome/browser/profiles/pro... chrome/browser/profiles/profile_manager_browsertest.cc:310: // Should be fixed: Crashes/CHECKs. See crbug.com/104851 Does this test still crash? If so, we probably shouldn't reenable it. https://codereview.chromium.org/2747293003/diff/1/chrome/browser/ui/webui/pro... File chrome/browser/ui/webui/profile_helper_browsertest.cc (right): https://codereview.chromium.org/2747293003/diff/1/chrome/browser/ui/webui/pro... chrome/browser/ui/webui/profile_helper_browsertest.cc:121: IN_PROC_BROWSER_TEST_F(ProfileHelperTest, DeleteInctiveProfile) { Nit: DeleteInactiveProfile
On 2017/03/16 16:31:53, Bernhard Bauer wrote: > Does this test still crash? If so, we probably shouldn't reenable it. Nope, test should be fine now. Is was a note to myself. Removed it, as all other nits.
The CQ bit was checked by palar@yandex-team.ru to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: No L-G-T-M from a valid reviewer yet. CQ run can only be started once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer,_not_ a full super star committer. Committers are members of the group "project-chromium-committers". Note that this has nothing to do with OWNERS files.
LGTM, thanks!
leaning on bauerb@, happy to provide profiles LGTM
The CQ bit was checked by palar@yandex-team.ru
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...) chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by palar@yandex-team.ru
The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org, mlerman@chromium.org Link to the patchset: https://codereview.chromium.org/2747293003/#ps40001 (title: "Resolved object files name conflict.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by palar@yandex-team.ru
The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org, mlerman@chromium.org Link to the patchset: https://codereview.chromium.org/2747293003/#ps60001 (title: "Added delay for browser activation at OpenNewWindowForProfile test.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by palar@yandex-team.ru
The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org, mlerman@chromium.org Link to the patchset: https://codereview.chromium.org/2747293003/#ps80001 (title: "Disabled failing check on Macs.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by palar@yandex-team.ru
The CQ bit was checked by palar@yandex-team.ru
The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org, mlerman@chromium.org Link to the patchset: https://codereview.chromium.org/2747293003/#ps100001 (title: "Disabled failing check on Macs.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by palar@yandex-team.ru
The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org, mlerman@chromium.org Link to the patchset: https://codereview.chromium.org/2747293003/#ps120001 (title: "Fixed test name conflict. Tests fixes for CrOS.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Patchset #5 (id:80001) has been deleted
Patchset #6 (id:120001) has been deleted
The CQ bit was checked by palar@yandex-team.ru
The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org, mlerman@chromium.org Link to the patchset: https://codereview.chromium.org/2747293003/#ps140001 (title: "Disabled tests for CrOS.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for chrome/test/BUILD.gn:
While running git apply --index -p1;
error: patch failed: chrome/test/BUILD.gn:1833
error: chrome/test/BUILD.gn: patch does not apply
Patch: chrome/test/BUILD.gn
Index: chrome/test/BUILD.gn
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index
d2612b51ca9b694216848dac35ac0832ef90238c..b003754d0bfc2d2ec6364db9bdea670421ed389c
100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -1833,6 +1833,7 @@ test("browser_tests") {
"../browser/ui/webui/password_manager_internals/password_manager_internals_ui_browsertest.cc",
"../browser/ui/webui/policy_ui_browsertest.cc",
"../browser/ui/webui/print_preview/print_preview_ui_browsertest.cc",
+ "../browser/ui/webui/profile_helper_browsertest.cc",
"../browser/ui/webui/set_as_default_browser_ui_browsertest_win.cc",
"../browser/ui/webui/settings/md_settings_ui_browsertest.cc",
"../browser/ui/webui/signin/inline_login_ui_browsertest.cc",
@@ -2376,6 +2377,7 @@ test("browser_tests") {
# inline login UI is disabled on chromeos
"../browser/ui/views/sync/profile_signin_confirmation_dialog_views_browsertest.cc",
+ "../browser/ui/webui/profile_helper_browsertest.cc",
"../browser/ui/webui/signin/inline_login_ui_browsertest.cc",
# chromeos does not use the desktop user manager
The CQ bit was checked by palar@yandex-team.ru
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for chrome/test/BUILD.gn:
While running git apply --index -p1;
error: patch failed: chrome/test/BUILD.gn:1833
error: chrome/test/BUILD.gn: patch does not apply
Patch: chrome/test/BUILD.gn
Index: chrome/test/BUILD.gn
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index
d2612b51ca9b694216848dac35ac0832ef90238c..b003754d0bfc2d2ec6364db9bdea670421ed389c
100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -1833,6 +1833,7 @@ test("browser_tests") {
"../browser/ui/webui/password_manager_internals/password_manager_internals_ui_browsertest.cc",
"../browser/ui/webui/policy_ui_browsertest.cc",
"../browser/ui/webui/print_preview/print_preview_ui_browsertest.cc",
+ "../browser/ui/webui/profile_helper_browsertest.cc",
"../browser/ui/webui/set_as_default_browser_ui_browsertest_win.cc",
"../browser/ui/webui/settings/md_settings_ui_browsertest.cc",
"../browser/ui/webui/signin/inline_login_ui_browsertest.cc",
@@ -2376,6 +2377,7 @@ test("browser_tests") {
# inline login UI is disabled on chromeos
"../browser/ui/views/sync/profile_signin_confirmation_dialog_views_browsertest.cc",
+ "../browser/ui/webui/profile_helper_browsertest.cc",
"../browser/ui/webui/signin/inline_login_ui_browsertest.cc",
# chromeos does not use the desktop user manager
The CQ bit was checked by palar@yandex-team.ru
The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org, mlerman@chromium.org Link to the patchset: https://codereview.chromium.org/2747293003/#ps160001 (title: "Rebased patch version.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
The CQ bit was checked by palar@yandex-team.ru
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 160001, "attempt_start_ts": 1490180854130840,
"parent_rev": "6fd78003bc0d9f00fb49e195c762eb227a3f2134", "commit_rev":
"dd76f5f150fbaf1a277b8912c1a15e72e96078b5"}
Message was sent while issue was closed.
Description was changed from ========== Added profile deletion browsertests. BUG= R=bauerb@chromium.org, anthonyvd@chromium.org, mlerman@chromium.org ========== to ========== Added profile deletion browsertests. BUG= R=bauerb@chromium.org, anthonyvd@chromium.org, mlerman@chromium.org Review-Url: https://codereview.chromium.org/2747293003 Cr-Commit-Position: refs/heads/master@{#458719} Committed: https://chromium.googlesource.com/chromium/src/+/dd76f5f150fbaf1a277b8912c1a1... ==========
Message was sent while issue was closed.
Committed patchset #7 (id:160001) as https://chromium.googlesource.com/chromium/src/+/dd76f5f150fbaf1a277b8912c1a1...
Message was sent while issue was closed.
A revert of this CL (patchset #7 id:160001) has been created in https://codereview.chromium.org/2772503003/ by thakis@chromium.org. The reason for reverting is: Test is flaky, see https://crbug.com/704601.
Message was sent while issue was closed.
Description was changed from ========== Added profile deletion browsertests. BUG= R=bauerb@chromium.org, anthonyvd@chromium.org, mlerman@chromium.org Review-Url: https://codereview.chromium.org/2747293003 Cr-Commit-Position: refs/heads/master@{#458719} Committed: https://chromium.googlesource.com/chromium/src/+/dd76f5f150fbaf1a277b8912c1a1... ========== to ========== Added profile deletion browsertests. BUG= R=bauerb@chromium.org, anthonyvd@chromium.org, mlerman@chromium.org Review-Url: https://codereview.chromium.org/2747293003 Cr-Commit-Position: refs/heads/master@{#458719} Committed: https://chromium.googlesource.com/chromium/src/+/dd76f5f150fbaf1a277b8912c1a1... ==========
On 2017/03/23 17:22:30, Nico (afk until Tue Apr 4) wrote: > A revert of this CL (patchset #7 id:160001) has been created in > https://codereview.chromium.org/2772503003/ by mailto:thakis@chromium.org. > > The reason for reverting is: Test is flaky, see https://crbug.com/704601. Problem pinpointed. Flakiness is fixed. @bauerb, @mlerman, can you re-review this issue please?
On 2017/03/31 17:51:44, palar wrote: > On 2017/03/23 17:22:30, Nico (afk until Tue Apr 4) wrote: > > A revert of this CL (patchset #7 id:160001) has been created in > > https://codereview.chromium.org/2772503003/ by mailto:thakis@chromium.org. > > > > The reason for reverting is: Test is flaky, see https://crbug.com/704601. > > Problem pinpointed. Flakiness is fixed. > @bauerb, @mlerman, can you re-review this issue please? Please re-land as a separate CL. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
