Bug 53217 - new-run-webkit-tests: turn off pixel tests correctly by default for webkit-based ports
Summary: new-run-webkit-tests: turn off pixel tests correctly by default for webkit-ba...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Dirk Pranke
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-26 17:41 PST by Dirk Pranke
Modified: 2011-01-27 14:21 PST (History)
7 users (show)

See Also:


Attachments
Patch (4.32 KB, patch)
2011-01-26 17:56 PST, Dirk Pranke
no flags Details | Formatted Diff | Diff
fix so that you can still manually enable pixel tests (4.42 KB, patch)
2011-01-26 18:17 PST, Dirk Pranke
mihaip: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Pranke 2011-01-26 17:41:33 PST
new-run-webkit-tests: turn off pixel tests correctly by default for webkit-based ports
Comment 1 Dirk Pranke 2011-01-26 17:56:59 PST
Created attachment 80284 [details]
Patch
Comment 2 Dirk Pranke 2011-01-26 18:17:27 PST
Created attachment 80285 [details]
fix so that you can still manually enable pixel tests
Comment 3 Mihai Parparita 2011-01-26 18:21:40 PST
Comment on attachment 80285 [details]
fix so that you can still manually enable pixel tests

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

> Tools/Scripts/webkitpy/layout_tests/port/webkit.py:60
> +        if not hasattr(self._options, "pixel_tests") or self._options.pixel_tests == None:

I think the condition "not get_option('pixel_tests')" should result in the same behavior.
Comment 4 Dirk Pranke 2011-01-26 22:35:05 PST
(In reply to comment #3)
> (From update of attachment 80285 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=80285&action=review
> 
> > Tools/Scripts/webkitpy/layout_tests/port/webkit.py:60
> > +        if not hasattr(self._options, "pixel_tests") or self._options.pixel_tests == None:
> 
> I think the condition "not get_option('pixel_tests')" should result in the same behavior.

"not get_option('pixel_tests')" would be True if was set to False. Which, in this case, would be harmless, but if you wanted the default to be True, would do the wrong thing, so probably better to leave it as it is.
Comment 5 Eric Seidel (no email) 2011-01-26 22:48:38 PST
Comment on attachment 80285 [details]
fix so that you can still manually enable pixel tests

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

> Tools/Scripts/webkitpy/layout_tests/port/base.py:126
> +        if not hasattr(self._options, 'configuration'):
> +            self._options.configuration = None
>          if self._options.configuration is None:

seems sily.  Why not combine these two with an or?
Comment 6 Dirk Pranke 2011-01-26 23:20:59 PST
(In reply to comment #5)
> (From update of attachment 80285 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=80285&action=review
> 
> > Tools/Scripts/webkitpy/layout_tests/port/base.py:126
> > +        if not hasattr(self._options, 'configuration'):
> > +            self._options.configuration = None
> >          if self._options.configuration is None:
> 
> seems sily.  Why not combine these two with an or?

Can do!
Comment 7 Dirk Pranke 2011-01-27 12:54:11 PST
Committed r76829: <http://trac.webkit.org/changeset/76829>
Comment 8 WebKit Review Bot 2011-01-27 14:21:31 PST
http://trac.webkit.org/changeset/76829 might have broken GTK Linux 32-bit Release
The following tests are not passing:
svg/animations/animate-text-nested-transforms.html