Make WaitForInfobarCount() robust

Make WaitForInfobarCount() robust

Make WaitForInfobarCount() robust to failures in the case of missing tabs or
windows.

BUG=
TEST=

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=62307

[Nirnimesh, 2010-10-06 19:18:41]

http://codereview.chromium.org/3529016/diff/1/2
File chrome/test/pyautolib/pyauto.py (left):

http://codereview.chromium.org/3529016/diff/1/2#oldcode645
chrome/test/pyautolib/pyauto.py:645: cmd_dict = {
This was unused

[John Grabowski, 2010-10-06 19:40:54]

Would prefer descriptive text that can be used for debugging to be printed with
this fails.

http://codereview.chromium.org/3529016/diff/1/2
File chrome/test/pyautolib/pyauto.py (right):

http://codereview.chromium.org/3529016/diff/1/2#newcode656
chrome/test/pyautolib/pyauto.py:656: return self.WaitUntil(_CountMatched)
Food for thought: if this fails we might like something descriptive printed. 
Perhaps WaitUntil can have a 2nd arg, a function, which returns text to print
out when WaitUntil fails.  For this case we can have it be

def _CountFailString():
  result = 'waiting for ' + str(count) + ' infobars but found ' + ...

return self.WaitUntil(_CountMatched, _CountFailString)

This might warrant some refactoring so both _CountMatched and _CountFailString
can use a common "get infobar count" routine (which returns an int or false)

[Nirnimesh, 2010-10-07 02:42:25]

John, this is a suggestive update.

I've renamed WaitForInfobarCount to assertWaitForInfobarCount so that I can
assert in it. In it I construction a return_dict which I pass to waitUntl func
which passes it to the functional. The functional can add in attributes like
error messages, or #attempts tried, which can be emitted out in the case of
assertion failure. I handle the case in which the functional is the simple form,
ie does not want to add in error messages.

It has gotten more complicated. Does this look reasonable?

I've updated one test with this. I'll need to update all occurrences of
WaitForInfobarCount

Thanks

[John Grabowski, 2010-10-07 05:02:51]

I agree this is more complicated.  It seems a little overkill.

I like the method named waitForInfoBarCount, since a failure is obvious (even if
you only have the stack).  Perhaps you can simply print more info in waitUntil. 
E.g. 
  print 'Function ' + function + ' failed; last return value: ' + blah + '
expected: " + ...
Printing the function would look something like
  <function _GetInfoBarCount at 0x70170>

Which may be helpful enough.  That, plus saving the last return value, and
printing it with the expected return value, would work.

http://codereview.chromium.org/3529016/diff/5001/6002
File chrome/test/pyautolib/pyauto.py (right):

http://codereview.chromium.org/3529016/diff/5001/6002#newcode292
chrome/test/pyautolib/pyauto.py:292: fn_accepts_return_attrs = 'return_attrs' in
inspect.getargspec(function)[0]
yuk.

[Nirnimesh, 2010-10-07 21:33:34]

Added logging. Also, added |expect_retval| (defaulting to True) so that
WaitUntil log last returned vs expected. This is looking much better now.

[John Grabowski, 2010-10-07 21:49:42]

LGTM

http://codereview.chromium.org/3529016/diff/11001/12001
File chrome/test/pyautolib/pyauto.py (right):

http://codereview.chromium.org/3529016/diff/11001/12001#newcode296
chrome/test/pyautolib/pyauto.py:296: if (type(expect_retval) ==
types.BooleanType and bool(retval)) or \
Is the bool(retval) really necessary? 
The only difference between "if bool(retval)" and "if retval" is if it's a
negative number.  We don't care.  How about expected_retval=None.  Then

if (expected_retval == None) and retval:
  return True
if (expected_retval == retval:
  return True
...

http://codereview.chromium.org/3529016/diff/11001/12001#newcode299
chrome/test/pyautolib/pyauto.py:299: logging.debug('WaitUntil: Function %s still
waiting. '
This could be a LOT of logging.

I would prefer if this only logged (and always logged) if this function returns
False.