[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





--- Comment #19 from Philippe Normand <pnormand at igalia.com>  2010-09-07 00:55:17 PST ---
(In reply to comment #18)
> (From update of attachment 66656 [details])
> 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.
> 

Crazy indeed! Will be fixed in next patch.

> > 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.
> 

Sure we can use that parser but it's really slow, at least here. Anyway, will make use of it in next patch...

> > 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.
> 

I don't see how to do that. The port modules are imported conditionally (see the get function in that same file, which is too BTW an interesting maintenance burden :)).

> > 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?
> 

The chromium and webkit port implementations require a class or instance object, see for instance:

webkit.py, line 368, in _build_path
    self.flag_from_configuration(self._options.configuration),

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