rebaseline.py: add --keep-going-on-failure option, off by default

rebaseline.py: add --keep-going-on-failure option, off by default

R=borenet@google.com

Committed: https://code.google.com/p/skia/source/detail?r=10109

[epoger, 2013-07-12 19:34:52]

Ready for review at patchset 1

https://codereview.chromium.org/18092004/diff/1/tools/rebaseline_imagefiles.py
File tools/rebaseline_imagefiles.py (right):

https://codereview.chromium.org/18092004/diff/1/tools/rebaseline_imagefiles.p...
tools/rebaseline_imagefiles.py:2: 
Verified that the bug is gone by rebaselining all new tests using the script
after this CL...

tools/rebaseline.py --expectations-root ~/src/skia/gm-expected/ --add-new \
  && echo READY TO COMMIT!

[borenet, 2013-07-12 19:51:36]

LGTM with question

https://codereview.chromium.org/18092004/diff/1/tools/rebaseline_imagefiles.py
File tools/rebaseline_imagefiles.py (right):

https://codereview.chromium.org/18092004/diff/1/tools/rebaseline_imagefiles.p...
tools/rebaseline_imagefiles.py:2: 
On 2013/07/12 19:34:53, epoger wrote:
> Verified that the bug is gone by rebaselining all new tests using the script
> after this CL...
> 
> tools/rebaseline.py --expectations-root ~/src/skia/gm-expected/ --add-new \
>   && echo READY TO COMMIT!

Do we ever expect this type of thing to happen again?  If so, should this script
fail, or should we warn loudly and have a script (like
http://skia.googlecode.com/svn/trunk/tools/verify_images_for_gm_results.py) to
turn something red?

[epoger, 2013-07-12 20:01:28]

On 2013/07/12 19:51:36, borenet wrote:
> LGTM with question
> 
> https://codereview.chromium.org/18092004/diff/1/tools/rebaseline_imagefiles.py
> File tools/rebaseline_imagefiles.py (right):
> 
>
https://codereview.chromium.org/18092004/diff/1/tools/rebaseline_imagefiles.p...
> tools/rebaseline_imagefiles.py:2: 
> On 2013/07/12 19:34:53, epoger wrote:
> > Verified that the bug is gone by rebaselining all new tests using the script
> > after this CL...
> > 
> > tools/rebaseline.py --expectations-root ~/src/skia/gm-expected/ --add-new \
> >   && echo READY TO COMMIT!
> 
> Do we ever expect this type of thing to happen again?  If so, should this
script
> fail, or should we warn loudly and have a script (like
> http://skia.googlecode.com/svn/trunk/tools/verify_images_for_gm_results.py) to
> turn something red?

Murphy's Law says that it will happen again, eventually.

The problem is that warnings are so easy to ignore.  How about this idea:

By default, quit as soon as we encounter one of these errors, with an error
message saying "run again with --keep-going-on-failure if you want to get
partial results".

[borenet, 2013-07-12 20:02:13]

On 2013/07/12 20:01:28, epoger wrote:
> On 2013/07/12 19:51:36, borenet wrote:
> > LGTM with question
> > 
> >
https://codereview.chromium.org/18092004/diff/1/tools/rebaseline_imagefiles.py
> > File tools/rebaseline_imagefiles.py (right):
> > 
> >
>
https://codereview.chromium.org/18092004/diff/1/tools/rebaseline_imagefiles.p...
> > tools/rebaseline_imagefiles.py:2: 
> > On 2013/07/12 19:34:53, epoger wrote:
> > > Verified that the bug is gone by rebaselining all new tests using the
script
> > > after this CL...
> > > 
> > > tools/rebaseline.py --expectations-root ~/src/skia/gm-expected/ --add-new
\
> > >   && echo READY TO COMMIT!
> > 
> > Do we ever expect this type of thing to happen again?  If so, should this
> script
> > fail, or should we warn loudly and have a script (like
> > http://skia.googlecode.com/svn/trunk/tools/verify_images_for_gm_results.py)
to
> > turn something red?
> 
> Murphy's Law says that it will happen again, eventually.
> 
> The problem is that warnings are so easy to ignore.  How about this idea:
> 
> By default, quit as soon as we encounter one of these errors, with an error
> message saying "run again with --keep-going-on-failure if you want to get
> partial results".

SGTM

[epoger, 2013-07-16 16:55:39]

OK, I think this gives us the flexibility we want... PTAL at patchset 2.

[epoger, 2013-07-16 16:56:56]

On 2013/07/16 16:55:39, epoger wrote:
> OK, I think this gives us the flexibility we want... PTAL at patchset 2.

I forgot to say... tested as follows:

$ tools/rebaseline.py --expectations-root ~/src/skia/gm-expectedfff/
--actuals-base-url bogusurl   ; echo $?
Could not find "base-android-galaxy-nexus" subdir within expectations_root
"/usr/local/google/home/epoger/src/skia/gm-expectedfff/".  Are you sure
--expectations-root is pointing at a valid gm-expected directory?
Halting at first exception; to keep going, re-run with the
--keep-going-on-failure option set.
1

$ tools/rebaseline.py --expectations-root ~/src/skia/gm-expectedfff/
--actuals-base-url bogusurl  --keep-going-on-failure ; echo $?
WARNING: swallowing exception Could not find "base-android-galaxy-nexus" subdir
within expectations_root
"/usr/local/google/home/epoger/src/skia/gm-expectedfff/".  Are you sure
--expectations-root is pointing at a valid gm-expected directory?
WARNING: swallowing exception Could not find "base-android-nexus-10" subdir
within expectations_root
"/usr/local/google/home/epoger/src/skia/gm-expectedfff/".  Are you sure
--expectations-root is pointing at a valid gm-expected directory?
WARNING: swallowing exception Could not find "base-android-nexus-4" subdir
within expectations_root
"/usr/local/google/home/epoger/src/skia/gm-expectedfff/".  Are you sure
--expectations-root is pointing at a valid gm-expected directory?
WARNING: swallowing exception Could not find "base-android-nexus-7" subdir
within expectations_root
"/usr/local/google/home/epoger/src/skia/gm-expectedfff/".  Are you sure
--expectations-root is pointing at a valid gm-expected directory?
WARNING: swallowing exception Could not find "base-android-nexus-s" subdir
within expectations_root
"/usr/local/google/home/epoger/src/skia/gm-expectedfff/".  Are you sure
--expectations-root is pointing at a valid gm-expected directory?
WARNING: swallowing exception Could not find "base-android-xoom" subdir within
expectations_root "/usr/local/google/home/epoger/src/skia/gm-expectedfff/".  Are
you sure --expectations-root is pointing at a valid gm-expected directory?
WARNING: swallowing exception Could not find "base-macmini" subdir within
expectations_root "/usr/local/google/home/epoger/src/skia/gm-expectedfff/".  Are
you sure --expectations-root is pointing at a valid gm-expected directory?
WARNING: swallowing exception Could not find "base-macmini-lion-float" subdir
within expectations_root
"/usr/local/google/home/epoger/src/skia/gm-expectedfff/".  Are you sure
--expectations-root is pointing at a valid gm-expected directory?
WARNING: swallowing exception Could not find "base-shuttle-win7-intel-angle"
subdir within expectations_root
"/usr/local/google/home/epoger/src/skia/gm-expectedfff/".  Are you sure
--expectations-root is pointing at a valid gm-expected directory?
WARNING: swallowing exception Could not find
"base-shuttle-win7-intel-directwrite" subdir within expectations_root
"/usr/local/google/home/epoger/src/skia/gm-expectedfff/".  Are you sure
--expectations-root is pointing at a valid gm-expected directory?
WARNING: swallowing exception Could not find "base-shuttle-win7-intel-float"
subdir within expectations_root
"/usr/local/google/home/epoger/src/skia/gm-expectedfff/".  Are you sure
--expectations-root is pointing at a valid gm-expected directory?
WARNING: swallowing exception Could not find "base-shuttle_ubuntu12_ati5770"
subdir within expectations_root
"/usr/local/google/home/epoger/src/skia/gm-expectedfff/".  Are you sure
--expectations-root is pointing at a valid gm-expected directory?
Encountered 12 failures (see above).
1

[borenet, 2013-07-16 17:31:52]

LGTM but seems over-complicated.  I might just pass around a boolean
keep_going_on_failure and check that, but I'll leave it up to you.

https://codereview.chromium.org/18092004/diff/14001/tools/rebaseline.py
File tools/rebaseline.py (right):

https://codereview.chromium.org/18092004/diff/14001/tools/rebaseline.py#newco...
tools/rebaseline.py:293: 'keep going and rebaseline as many tests as possible,
and ' +
nit: 80 chars

[epoger, 2013-07-16 17:34:28]

On 2013/07/16 17:31:52, borenet wrote:
> LGTM but seems over-complicated.  I might just pass around a boolean
> keep_going_on_failure and check that, but I'll leave it up to you.

Believe me, I tried that first... it didn't work, for various reasons.

> https://codereview.chromium.org/18092004/diff/14001/tools/rebaseline.py
> File tools/rebaseline.py (right):
> 
>
https://codereview.chromium.org/18092004/diff/14001/tools/rebaseline.py#newco...
> tools/rebaseline.py:293: 'keep going and rebaseline as many tests as possible,
> and ' +
> nit: 80 chars

Fixed.

[epoger, 2013-07-16 17:36:03]

Committed patchset #4 manually as r10109 (presubmit successful).