Commit 2819cbd0 authored by Vaclav Brozek's avatar Vaclav Brozek Committed by Commit Bot

Fix PasswordsSettingsTestCase/testDeletion and testStoredEntriesAlwaysShown

testStoredEntriesAlwaysShown was introduced in
https://chromium-review.googlesource.com/c/573381/. The functionality it
tests exists both when the "view passwords" feature is enabled or not.
However, there is a subtle difference in the accessibility properties of
items inspected by the test: with "view passwords", those items are
buttons, without "view passwords" those items are just text cells. The
test is written with helper matchers designed for "view passwords"
enabled, however the test itself was written without enabling "view
passwords". The helper matchers required the "button" accessibility
trait to be set, and hence failed to locate the items with the "view
passwords" feature disabled.

As a result, the test was failing on bots (when I checked before
landing, it ran OK on my own machine, where I accidentally enabled the
"view passwords" feature).

An unrelated test, testDeletion, also always failed when
testStoredEntriesAlwaysShown failed. I have not investigated why, but it
was probably due to failure of the latter to clear the PasswordStore on
failure. This will be addressed in
https://chromium-review.googlesource.com/c/573542/. There is good
evidence that testDeletion indeed failed as a result of
testStoredEntriesAlwaysShown failing: while at
https://build.chromium.org/p/chromium.fyi/builders/EarlGreyiOS/builds/29816
they both failed,
https://build.chromium.org/p/chromium.fyi/builders/EarlGreyiOS/builds/29817
which only had testStoredEntriesAlwaysShown disabled, passed with no
failure. I observed the same on a downstream bot iphone10-simulator-x64,
where I could reproduce both tests failing before this patch and both
passing with only testStoredEntriesAlwaysShown changed and both tests
enabled, as in this CL.

The fix in this CL is to enable the "view passwords" feature during
testStoredEntriesAlwaysShown. An alternative would be to change the
matchers used to not require the "button" accessibility trait when the
feature is disabled. However, the feature is going to be enabled as soon
as version 61 branches, so using the existing helper matchers will keep
the code simple with minimal effect beyond tomorrow.

Bug: 515462
Change-Id: I90ac1d3f1c42402cc73293eb3e5ba4f49475926a
Reviewed-on: https://chromium-review.googlesource.com/579195Reviewed-by: default avatarSylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Vaclav Brozek <vabr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488250}
parent e74d8610
......@@ -553,10 +553,7 @@ MockReauthenticationModule* SetUpAndReturnMockReauthenticationModule() {
// Checks that deleting a password from password details view goes back to the
// list-of-passwords view.
// TODO(crbug.com/515462) This test stopped working after
// https://chromium-review.googlesource.com/c/573381/ landed. Needs to be
// investigated and fixed.
- (void)DISABLED_testDeletion {
- (void)testDeletion {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(
password_manager::features::kViewPasswords);
......@@ -1066,8 +1063,11 @@ MockReauthenticationModule* SetUpAndReturnMockReauthenticationModule() {
// Check that stored entries are shown no matter what the preference for saving
// passwords is.
// TODO(crbug.com/515462): Fix the scrolling.
- (void)DISABLED_testStoredEntriesAlwaysShown {
- (void)testStoredEntriesAlwaysShown {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature(
password_manager::features::kViewPasswords);
SaveExamplePasswordForm();
PasswordForm blacklisted;
......
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