Bug 98810

Summary: webkit-patch rebaseline-expectations doesn't work w/o failures specified
Product: WebKit Reporter: Dirk Pranke <dpranke>
Component: New BugsAssignee: Dirk Pranke <dpranke>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, jchaffraix, ojan, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
just get all of the tests marked for rebaselining none

Description Dirk Pranke 2012-10-09 12:00:38 PDT
webkit-patch rebaseline-expectations doesn't work w/o failures specified
Comment 1 Dirk Pranke 2012-10-09 12:01:21 PDT
Created attachment 167807 [details]
Patch
Comment 2 Ojan Vafai 2012-10-09 12:21:06 PDT
Comment on attachment 167807 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=167807&action=review

> Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:872
> +        return self._model.get_test_set(REBASELINE, IMAGE) | self._model.get_test_set(REBASELINE, FAIL) | self._model.get_test_set(REBASELINE, PASS)

Can you just use self._model.get_test_set("REBASELINE")?

In addition to the fixme to get rid of get_test_set, I worry that we'll eventually do the cleanup to not mark skip/rebaseline tests as pass, but just have the expectation be skip/rebaseline. Then this code will break again.
Comment 3 Dirk Pranke 2012-10-09 12:26:53 PDT
(In reply to comment #2)
> (From update of attachment 167807 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=167807&action=review
> 
> > Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:872
> > +        return self._model.get_test_set(REBASELINE, IMAGE) | self._model.get_test_set(REBASELINE, FAIL) | self._model.get_test_set(REBASELINE, PASS)
> 
> Can you just use self._model.get_test_set("REBASELINE")?
> 

Oh, yeah, I guess you can. I was thinking (a) that you had to have expectations, and (b) that we wouldn't want to rebaseline something if it was marked Crash or Timeout, but perhaps that's overly paranoid.

> In addition to the fixme to get rid of get_test_set, I worry that we'll eventually do the cleanup to not mark skip/rebaseline tests as pass, but just have the expectation be skip/rebaseline. Then this code will break again.

But the test should catch it if it breaks again, right?
Comment 4 Ojan Vafai 2012-10-09 12:28:00 PDT
Comment on attachment 167807 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=167807&action=review

>>> Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:872
>>> +        return self._model.get_test_set(REBASELINE, IMAGE) | self._model.get_test_set(REBASELINE, FAIL) | self._model.get_test_set(REBASELINE, PASS)
>> 
>> Can you just use self._model.get_test_set("REBASELINE")?
>> 
>> In addition to the fixme to get rid of get_test_set, I worry that we'll eventually do the cleanup to not mark skip/rebaseline tests as pass, but just have the expectation be skip/rebaseline. Then this code will break again.
> 
> Oh, yeah, I guess you can. I was thinking (a) that you had to have expectations, and (b) that we wouldn't want to rebaseline something if it was marked Crash or Timeout, but perhaps that's overly paranoid.

Yeah, I think that's overly paranoid. If you add rebaseline, you're asking for it I think.
Comment 5 Dirk Pranke 2012-10-09 14:09:18 PDT
Created attachment 167845 [details]
just get all of the tests marked for rebaselining
Comment 6 WebKit Review Bot 2012-10-09 15:19:34 PDT
Comment on attachment 167845 [details]
just get all of the tests marked for rebaselining

Clearing flags on attachment: 167845

Committed r130819: <http://trac.webkit.org/changeset/130819>
Comment 7 WebKit Review Bot 2012-10-09 15:19:38 PDT
All reviewed patches have been landed.  Closing bug.