[Webkit-unassigned] [Bug 111513] Create a script to import W3C tests

[bugzilla-daemon at webkit.org]

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





--- Comment #12 from Dirk Pranke <dpranke at chromium.org>  2013-03-08 11:44:11 PST ---
(From update of attachment 192111)
View in context: https://bugs.webkit.org/attachment.cgi?id=192111&action=review

Here's some general high-level comments about writing Python code in a manner more resembling the rest of WebKit's python code :) :

> Tools/Scripts/generate-w3c-testsuite-manifest:16
> +from HTMLParser import HTMLParser

I don't think the HTMLParser library is particularly robust. We generally use BeautifulSoup for parsing HTML in webkitpy.

> Tools/Scripts/generate-w3c-testsuite-manifest:35
> +def main():

Generally speaking, we don't tend to use a lot of free (file-level) functions in webkit python code. The top-level executable scripts (in Tools/Scripts) tend to be very short wrappers around modules that live under Tools/Scripts/webkitpy. Even there, we usually hang functions off of classes and objects, unless the functions are pure (have no side effects).

>> Tools/Scripts/generate-w3c-testsuite-manifest:56
>> +        testPath = sys.argv[1]
> 
> As there's no advanced processing of the parameters going on, I'd rather drop the testPath global variable and make the scanTestDir() method accept sys.argv[1] as a parameter.

In webkitpy we invariably use the optparse module for option parsing and usage messages. Please look into that rather than rolling your own parsing.

>> Tools/Scripts/generate-w3c-testsuite-manifest:61
>> +    ignoreFiles = ["-ref", "-notref", ".css", ".js", ".png", ".jpg", ".jpeg", ".DS_Store", ".mf", ".log"]
> 
> If these are just used as parameters to other functions, I'd rather have them as script constants/variables.

script-level (global) constants are fine, but we don't like mutable globals; they're much harder to test and mock-out.

> Tools/Scripts/generate-w3c-testsuite-manifest:96
> +    f = open(testFilePath)

Also, as far as making things more webkitpy-like goes, we frown on direct access to os-level services like open(). Please look into the Host abstraction in webkitpy/common/host, and the FileSystem and Executive abstractions in webkitpy/common/system . We have all this stuff so that it is much easier to write unit tests for our code.

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