New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 718043 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
hobby only
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug



Sign in to add a comment

Fix PasswordsSettingsTestCase.testCopyPasswordToast on iOS.

Project Member Reported by jif@chromium.org, May 3 2017

Issue description

Comment 1 by vabr@chromium.org, May 3 2017

Cc: -vabr@chromium.org
Owner: vabr@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, May 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/692751dc80943ee7094ee1027d9cdc8c8f3d5170

commit 692751dc80943ee7094ee1027d9cdc8c8f3d5170
Author: jif <jif@chromium.org>
Date: Wed May 03 15:47:35 2017

Disable PasswordsSettingsTestCase.testCopyPasswordToast on iOS.

TBR=sdefresne
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 718043 

Review-Url: https://codereview.chromium.org/2861773002
Cr-Commit-Position: refs/heads/master@{#468985}

[modify] https://crrev.com/692751dc80943ee7094ee1027d9cdc8c8f3d5170/ios/chrome/browser/ui/settings/passwords_settings_egtest.mm

Comment 3 by vabr@chromium.org, May 3 2017

Status: Started (was: Assigned)
The EG test tries to tap on a button which ends up covered by the toast popped up earlier. I will add some scrolling.

Sorry to have missed this, I only tested iPhone 6S Plus, which is too big for that issue.

Comment 4 by vabr@chromium.org, May 3 2017

The patch is in https://codereview.chromium.org/2860453004/
Project Member

Comment 5 by bugdroid1@chromium.org, May 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6d0700c264fabe9d1b6857b2f860759544c4e640

commit 6d0700c264fabe9d1b6857b2f860759544c4e640
Author: vabr <vabr@chromium.org>
Date: Thu May 04 16:53:14 2017

Fix PasswordsSettingsTestCase.testCopyPasswordToast

The tested settings have a button just at the bottom end of the screen on small devices. Tapping the button results in showing a snackbar, which covers the button on small devices. testCopyPasswordToast in particular needs to tap the copy button twice. The second tap was impossible on small devices because the snackbar was still around, blocking the button.

This CL adds two actions to prevent this issue:

(1) While checking for the snackbar, the test also taps it, causing it to close. The test waits for the fade-out animation to finish.

(2) The test also uses usingSearchAction:grey_scrollInDirection to ask EarlGrey to scroll until the control is clickable.

Even though either single option from the above would fix the current issue, the CL contains both: (2) because it will also be useful on smaller devices, e.g., should we start testing in landscape mode (see https://crbug.com/717548 and also  https://crbug.com/717163  for another blocker for landscape mode), where the buttons will be off the screen instead of just being covered. It also includes (1), because cleaning up the snackbar explicitly seems like a reasonable thing to do. Speaking of cleaning up, the CL also:

(3) Forcibly closes all passwords-related snackbars on teardown. This will be useful when the test fails in the middle, leaving a snackbar up. If not foce-closed, such snackbar would remain visible for (currently) 4 seconds, which might be enough to interfere with the next test.

The CL also applied (1) and (2) to two other tests in the same suite as well. Those deal with similar buttons, which are currently high enough not to experience the problem with being blocked. This secures them in case that, e.g., the layout changes in the future.

Note that while (2) is clearly beneficial, it also has a drawback: the search action takes a considerable amount of time (about 2 seconds per one scrolling step on my computer). With it, the tests are more likely to time out. If there is an issue with timing out in the future, we should increase kScrollAmount to speed up the scrolling, or split long tests (testCopyPasswordToast is a particular candidate).

BUG= 718043 

Review-Url: https://codereview.chromium.org/2860453004
Cr-Commit-Position: refs/heads/master@{#469361}

[modify] https://crrev.com/6d0700c264fabe9d1b6857b2f860759544c4e640/ios/chrome/browser/ui/settings/password_details_collection_view_controller.mm
[modify] https://crrev.com/6d0700c264fabe9d1b6857b2f860759544c4e640/ios/chrome/browser/ui/settings/passwords_settings_egtest.mm

Comment 6 by vabr@chromium.org, May 7 2017

Status: Fixed (was: Started)
Looks like r469361 stuck, so marking as fixed.

Sign in to add a comment