[Webkit-unassigned] [Bug 42832] webkit-patch command to find the ports covering a specific layout test

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=42832


Adam Barth <abarth at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #66656|review?                     |review-
               Flag|                            |




--- Comment #18 from Adam Barth <abarth at webkit.org>  2010-09-06 10:30:36 PST ---
(From update of attachment 66656)
View in context: https://bugs.webkit.org/attachment.cgi?id=66656&action=prettypatch

Mostly nits.  The fake test_expectations parser is my main complaint.

> WebKitTools/Scripts/webkitpy/layout_tests/port/base.py:317
> +            basename, extension = os.path.splitext(test_or_category)
> +            if not extension and test_name.startswith(basename):
> +                return True
Crazy.  We decide if something is a directory by whether it has an extension?  It feels like there should be a better way to do this, but I don't know what it is.

> WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py:239
> +    def skipped_layout_tests(self):
> +        return [line.split(':')[1].split("=")[0].strip()
> +                for line in self.test_expectations().splitlines()
> +                if line.find('SKIP') != -1 and not line.startswith('//')]
Surely we should use the test_expectation file parser instead of hacking one together here.

> WebKitTools/Scripts/webkitpy/layout_tests/port/factory.py:37
> +ALL_PORT_NAMES = ['test', 'dryrun', 'mac', 'win', 'gtk', 'qt', 'chromium-mac',
> +                  'chromium-linux', 'chromium-win', 'google-chrome-win',
> +                  'google-chrome-mac', 'google-chrome-linux32', 'google-chrome-linux64']
This seems like a maintenance burden.  Can we get this information by crawling the port classes that have been imported into memory?  You can see how we do that to autoregister commands.

> WebKitTools/Scripts/webkitpy/tool/commands/queries.py:298
> +        class Options:
> +            pass
This seems like a ugly pattern.  Can't we just pass an empty dictionary to represent no options?

> WebKitTools/Scripts/webkitpy/tool/main.py:80
> +        self.port_factory = port.factory
Nice.  I'm glad these two subsystems are finally getting integrated.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.