Create EarlGrey wait utility.

Create EarlGrey wait utility.

EarlGrey typically handles synchronization with the UI, but it is
useful when manipulating non-UI APIs to be able to wait for a certain
state.

BUG=610837
Committed: https://crrev.com/038742bffc36be9c6d4266c3d43ae7fbf5e0a62c
Cr-Commit-Position: refs/heads/master@{#398182}

[baxley, 2016-06-06 21:20:47]

baxley@chromium.org changed reviewers:
+ eugenebut@chromium.org

[baxley, 2016-06-06 21:20:48]

I'm only using this in the simple case right now, where you have a simple
boolean that you are waiting for it to be true/false. For more complex cases
I'll just keep them implemented inline, and if patterns emerge, then refactor.
Currently there is one (which I need to update separately since the logic may be
flawed).

There was also a presubmit warning that I'm looking into...
You might be calling functions intended only for testing from
production code.  It is OK to ignore this warning if you know what
you are doing, as the heuristics used to detect the situation are
not perfect.  The commit queue will not block on this warning.
  ios/testing/earl_grey/wait_util.mm:21
    base::test::ios::SpinRunLoopWithMaxDelay(

https://codereview.chromium.org/2044433003/diff/1/ios/web/public/test/web_vie...
File ios/web/public/test/web_view_interaction_test_util.mm (right):

https://codereview.chromium.org/2044433003/diff/1/ios/web/public/test/web_vie...
ios/web/public/test/web_view_interaction_test_util.mm:32: });
Let me know what you think of the readability here and if a variable for the
block would be useful. It wouldn't shorten the number of lines, though it may
make it more readable.

[Eugene But (OOO till 7-30), 2016-06-06 21:38:59]

lgtm

https://codereview.chromium.org/2044433003/diff/1/ios/testing/earl_grey/wait_...
File ios/testing/earl_grey/wait_util.h (right):

https://codereview.chromium.org/2044433003/diff/1/ios/testing/earl_grey/wait_...
ios/testing/earl_grey/wait_util.h:21: // Wait until |condition| is true, or
induce GREYAssert after |timeout|.
s/Wait/Waits

https://codereview.chromium.org/2044433003/diff/1/ios/testing/earl_grey/wait_...
ios/testing/earl_grey/wait_util.h:21: // Wait until |condition| is true, or
induce GREYAssert after |timeout|.
s/induce/induces

https://codereview.chromium.org/2044433003/diff/1/ios/testing/earl_grey/wait_...
ios/testing/earl_grey/wait_util.h:22: void WaitUntilCondition(NSTimeInterval
timeout, bool (^condition)(void));
Thank you for placing block as the last argument!

https://codereview.chromium.org/2044433003/diff/1/ios/web/public/test/web_vie...
File ios/web/public/test/web_view_interaction_test_util.mm (right):

https://codereview.chromium.org/2044433003/diff/1/ios/web/public/test/web_vie...
ios/web/public/test/web_view_interaction_test_util.mm:30:
testing::WaitUntilCondition(testing::kWaitForJSCompletionTimeout, ^() {
Optional NIT: s/^() {/^{

https://codereview.chromium.org/2044433003/diff/1/ios/web/public/test/web_vie...
ios/web/public/test/web_view_interaction_test_util.mm:32: });
On 2016/06/06 21:20:48, baxley wrote:
> Let me know what you think of the readability here and if a variable for the
> block would be useful. It wouldn't shorten the number of lines, though it may
> make it more readable.
I believe that inline blocks are more readable comparing to using a local
variable for them. This is because Actual code flow is Call Method -> Call block
and local variable reverses this order.

Sometimes however inline blocks make formatting unreadable, but this is
definitely not the case!

https://codereview.chromium.org/2044433003/diff/1/ios/web/shell/test/earl_gre...
File ios/web/shell/test/earl_grey/shell_matchers.mm (right):

https://codereview.chromium.org/2044433003/diff/1/ios/web/shell/test/earl_gre...
ios/web/shell/test/earl_grey/shell_matchers.mm:57:
testing::WaitUntilCondition(testing::kWaitForUIElementTimeout, ^() {
I don't think this will compile on 32-bit platform. BOOL is unsigned there,
while C++ deduces the result of this block as bool and the type won't match. If
that's the case then you can fix it as follows:

testing::WaitUntilCondition(testing::kWaitForUIElementTimeout, ^bool() {
}

or 

testing::WaitUntilCondition(testing::kWaitForUIElementTimeout, ^{
  return text == base::SysNSStringToUTF8(textField.text);
});

[baxley, 2016-06-06 22:38:08]

comments addressed. Thanks for the review!

https://codereview.chromium.org/2044433003/diff/1/ios/testing/earl_grey/wait_...
File ios/testing/earl_grey/wait_util.h (right):

https://codereview.chromium.org/2044433003/diff/1/ios/testing/earl_grey/wait_...
ios/testing/earl_grey/wait_util.h:21: // Wait until |condition| is true, or
induce GREYAssert after |timeout|.
On 2016/06/06 21:38:59, Eugene But wrote:
> s/Wait/Waits

Done.

https://codereview.chromium.org/2044433003/diff/1/ios/testing/earl_grey/wait_...
ios/testing/earl_grey/wait_util.h:21: // Wait until |condition| is true, or
induce GREYAssert after |timeout|.
On 2016/06/06 21:38:59, Eugene But wrote:
> s/induce/induces

Done.

https://codereview.chromium.org/2044433003/diff/1/ios/testing/earl_grey/wait_...
ios/testing/earl_grey/wait_util.h:22: void WaitUntilCondition(NSTimeInterval
timeout, bool (^condition)(void));
On 2016/06/06 21:38:59, Eugene But wrote:
> Thank you for placing block as the last argument!

:) It wasn't even on accident!

https://codereview.chromium.org/2044433003/diff/1/ios/web/public/test/web_vie...
File ios/web/public/test/web_view_interaction_test_util.mm (right):

https://codereview.chromium.org/2044433003/diff/1/ios/web/public/test/web_vie...
ios/web/public/test/web_view_interaction_test_util.mm:30:
testing::WaitUntilCondition(testing::kWaitForJSCompletionTimeout, ^() {
On 2016/06/06 21:38:59, Eugene But wrote:
> Optional NIT: s/^() {/^{

Done.

https://codereview.chromium.org/2044433003/diff/1/ios/web/public/test/web_vie...
ios/web/public/test/web_view_interaction_test_util.mm:32: });
On 2016/06/06 21:38:59, Eugene But wrote:
> On 2016/06/06 21:20:48, baxley wrote:
> > Let me know what you think of the readability here and if a variable for the
> > block would be useful. It wouldn't shorten the number of lines, though it
may
> > make it more readable.
> I believe that inline blocks are more readable comparing to using a local
> variable for them. This is because Actual code flow is Call Method -> Call
block
> and local variable reverses this order.
> 
> Sometimes however inline blocks make formatting unreadable, but this is
> definitely not the case!

Acknowledged.

https://codereview.chromium.org/2044433003/diff/1/ios/web/shell/test/earl_gre...
File ios/web/shell/test/earl_grey/shell_matchers.mm (right):

https://codereview.chromium.org/2044433003/diff/1/ios/web/shell/test/earl_gre...
ios/web/shell/test/earl_grey/shell_matchers.mm:57:
testing::WaitUntilCondition(testing::kWaitForUIElementTimeout, ^() {
On 2016/06/06 21:38:59, Eugene But wrote:
> I don't think this will compile on 32-bit platform. BOOL is unsigned there,
> while C++ deduces the result of this block as bool and the type won't match.
If
> that's the case then you can fix it as follows:
> 
> testing::WaitUntilCondition(testing::kWaitForUIElementTimeout, ^bool() {
> }
> 
> or 
> 
> testing::WaitUntilCondition(testing::kWaitForUIElementTimeout, ^{
>   return text == base::SysNSStringToUTF8(textField.text);
> });

Nice catch! Done.

[baxley, 2016-06-06 22:47:09]

The CQ bit was checked by baxley@chromium.org

[baxley, 2016-06-06 22:47:10]

The patchset sent to the CQ was uploaded after l-g-t-m from
eugenebut@chromium.org
Link to the patchset: https://codereview.chromium.org/2044433003/#ps20001
(title: "fix blocks")

[commit-bot: I haz the power, 2016-06-06 22:48:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2044433003/20001

[commit-bot: I haz the power, 2016-06-07 01:03:54]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-07 01:06:20]

Description was changed from

==========
Create EarlGrey wait utility.

EarlGrey typically handles synchronization with the UI, but it is
useful when manipulating non-UI APIs to be able to wait for a certain
state.

BUG=610837
==========

to

==========
Create EarlGrey wait utility.

EarlGrey typically handles synchronization with the UI, but it is
useful when manipulating non-UI APIs to be able to wait for a certain
state.

BUG=610837

Committed: https://crrev.com/038742bffc36be9c6d4266c3d43ae7fbf5e0a62c
Cr-Commit-Position: refs/heads/master@{#398182}
==========

[commit-bot: I haz the power, 2016-06-07 01:06:21]

Patchset 2 (id:??) landed as
https://crrev.com/038742bffc36be9c6d4266c3d43ae7fbf5e0a62c
Cr-Commit-Position: refs/heads/master@{#398182}