"PasswordManagerBrowserTestBase.InternalsPage_Renderer" is flaky |
|||||
Issue description"PasswordManagerBrowserTestBase.InternalsPage_Renderer" 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=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyQAsSBUZsYWtlIjVQYXNzd29yZE1hbmFnZXJCcm93c2VyVGVzdEJhc2UuSW50ZXJuYWxzUGFnZV9SZW5kZXJlcgw. 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
,
Jan 17 2018
,
Jan 18 2018
,
Jan 18 2018
This line from the log seems relevant: [17046:17046:0117/091748.423427:ERROR:test_password_store.cc(203)] Not implemented reached in virtual std::vector<InteractionsStats> password_manager::TestPasswordStore::GetSiteStatsImpl(const GURL &)
,
Jan 18 2018
Nice, the start of the "flakes" coincides with landing of my e7acbb190061307931922e4441931e7d06bf56ba which made the contract of TestPasswordStore explicit and DCHECKED. After I verify that the test should indeed be calling GetSiteStatsImpl, then the fix is just to implement it for TestPasswordStore.
,
Jan 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cdb2f9649d924f7cb4b0866ca6fbf7ea55c08b45 commit cdb2f9649d924f7cb4b0866ca6fbf7ea55c08b45 Author: Vaclav Brozek <vabr@chromium.org> Date: Fri Jan 19 13:51:57 2018 Re-enable PasswordManagerBrowserTestBase.InternalsPage_Renderer. This CL extends the implementation of TestPasswordStore to support the needs of PasswordManagerBrowserTestBase.InternalsPage_Renderer. After non-implemented parts of TestPasswordStore were marked with NOTREACHED in https://crrev.com/516737, the ensuing DCHECKS highlighted that PasswordManagerBrowserTestBase.InternalsPage_Renderer is using some of those non-implemented parts. Therefore this CL provides a simplistic implementations of FillLoginsForSameOrganizationName and GetSiteStatsImpl and re-enables PasswordManagerBrowserTestBase.InternalsPage_Renderer. The test now passes without DCHECKS locally in ASAN build, while it was observed to fail with a DCHECK before the fix. The flakiness is likely due to the test often finishing successfully before the posted tasks of the TestPasswordStore were able to hit the NOTREACHED. Bug: 803155 Change-Id: I2b850a7fc24d847aa852675c71a0c5fb575d60d5 Reviewed-on: https://chromium-review.googlesource.com/875942 Commit-Queue: Vaclav Brozek <vabr@chromium.org> Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org> Cr-Commit-Position: refs/heads/master@{#530503} [modify] https://crrev.com/cdb2f9649d924f7cb4b0866ca6fbf7ea55c08b45/chrome/browser/password_manager/password_manager_browsertest.cc [modify] https://crrev.com/cdb2f9649d924f7cb4b0866ca6fbf7ea55c08b45/components/password_manager/core/browser/test_password_store.cc [modify] https://crrev.com/cdb2f9649d924f7cb4b0866ca6fbf7ea55c08b45/components/password_manager/core/browser/test_password_store.h
,
Jan 19 2018
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by bugdroid1@chromium.org
, Jan 17 2018