Making the gradient click-through in ChromeSigninViewController

ios/chrome/browser/ui/authentication/signin_interaction_controller_egtest.mm

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import <EarlGrey/EarlGrey.h>
 #import <XCTest/XCTest.h>
 
 #include "base/strings/sys_string_conversions.h"
 #import "base/test/ios/wait_util.h"
 #include "components/signin/core/browser/signin_manager.h"
@@ skipping 341 matching lines @@
   // Check that the settings home screen is shown.
   WaitForMatcher(grey_accessibilityID(kSettingsSignInCellId));
 
   [[EarlGrey selectElementWithMatcher:NavigationBarDoneButton()]
       performAction:grey_tap()];
 
   // Check that there is no signed in user.
   AssertAuthenticatedIdentityInActiveProfile(nil);
 }
 
-// Tests that signing in, tapping the Settings link on the confirmation screen
-// and closing the Settings correctly leaves the user signed in without any
-// Settings shown.
-// TODO(crbug.com/718023): Re-enable test.
-- (void)DISABLED_testSignInOpenSettings {
+- (void)testSignInOpenSettings {
   ChromeIdentity* identity = GetFakeIdentity1();
   ios::FakeChromeIdentityService::GetInstanceFromChromeProvider()->AddIdentity(
       identity);
 
   // Sign in to |identity|.
   OpenSignInFromSettings();
   TapButtonWithAccessibilityLabel(identity.userEmail);
   TapButtonWithLabelId(IDS_IOS_ACCOUNT_CONSISTENCY_SETUP_SIGNIN_BUTTON);
 
+  // Short delay to allow animation to complete and let time for the "Settings"
+  // label to arrive (issue with flakyness, see crbug.com/718023).
+  base::test::ios::SpinRunLoopWithMaxDelay(base::TimeDelta::FromSecondsD(0.5));

[baxley, 2017/05/31 15:51:29]

delays are bad to use in tests. In the best case, the test is slowed down for
0.5 every run. In the worst case, there are times where this delay is not long
enough and it is still flaky.

I understand there are cases where we cannot synchronize for some actions.
Ideally, we'd make it synchronized, but if not it is better to poll for a
condition, with timeout.

Could you useWaitForMatcher on line 96 in this file? Instead of a delay, it
polls for the matcher to succeed. If it doesn't find it in 4 seconds, then it
fails.

[jlebel, 2017/05/31 16:16:40]

This test doesn't fail to find "Settings", neither to tap on it. But it fails
find the "Done" button in the next screen. According to the crash:
https://paste.googleplex.com/5483254505275392
we believe link is found while the screen is still moving. So either the
button is disabled while moving or the NSEvent is missing the "Settings"
button since it is still moving.

I guess another way to fix it (if this is really the problem), is to create
NSNotification from my view controller once the animation is finished.
But I don't think it is a good idea to create code just for testing.

If I turn on the slow animation, the test fails to find "Settings" button (for
this test, for the other one, it fails at the beginning).

I don't know if there is any better solution. I'm very new to EarlGrey.

[baxley, 2017/05/31 16:29:39]

What about if you add more matcher constraints by using grey_allOf(matcher1,
matcher2, ... , nil);

Perhaps matching "Done", with "grey_interactable()"? If you did this, then maybe
no delay would be necessary? This was a problem we had with scrolling and
tapping menu items, that they are sometimes present but not tappable since it's
not interactable.
Yeah, that may be overkill. The examples I alluded to were cases where a
background animation was running, that should not have been.

[jlebel, 2017/06/01 15:01:45]

Since according to the log, the UI is still on the previous view with the
"Settings" button. So one possibility is that the tap failed. This could happen
if EarlGrey tries to tap it while its view appears with animation. That would
explain why the element is found but why the next screen is not displayed.

Unfortunately, I can't reproduce it on my computer. When I activate slow
animation, "Settings" is not even found. But I followed your advice by adding
WaitForMatcher() (and I also added more matcher). Now I can make this test
working with slow animation.

I would be interested to try if it solve the flakiness.

+
   // Tap Settings link.
   id<GREYMatcher> settings_link_matcher = grey_allOf(
       grey_accessibilityLabel(@"Settings"), grey_sufficientlyVisible(), nil);
   [[EarlGrey selectElementWithMatcher:settings_link_matcher]
       performAction:grey_tap()];
 
   [[EarlGrey selectElementWithMatcher:NavigationBarDoneButton()]
       performAction:grey_tap()];
 
   // All Settings should be gone and user signed in.
@@ skipping 203 matching lines @@
 
   // Close sign-in screen and Bookmarks.
   TapButtonWithLabelId(IDS_IOS_ACCOUNT_CONSISTENCY_SETUP_SKIP_BUTTON);
   if (!IsIPadIdiom()) {
     [[EarlGrey selectElementWithMatcher:NavigationBarDoneButton()]
         performAction:grey_tap()];
   }
 }
 
 @end