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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed May 1 01:06:04 PDT 2013


Dirk Pranke <dpranke at chromium.org> changed:

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

--- Comment #18 from Dirk Pranke <dpranke at chromium.org>  2013-05-01 01:04:25 PST ---
(From update of attachment 194324)
View in context: https://bugs.webkit.org/attachment.cgi?id=194324&action=review

Okay, as per our discussion over IM/IRC earlier today, I think it makes sense to land this and iterate as soon as we can, even if the code isn't ideal. The scripts shouldn't break anything else or affect anyone's checkout in particular.

Please address as many of the stylistic nits as you can (especially the copyright and changelog info). Don't worry about cleaning up the tests too much. Re-post another patch just so we can be sure it'll still apply against the tree and we should be able to land something soon.

Thanks for bearing with me during the long delay!

> Tools/Scripts/webkitpy/w3c/test_converter.py:125
> +        @return True if changes were made

We don't usually use javadoc-style @param/@return notation for parameters. We would probably do something more like:

def convert_testharness_paths(self, doc, new_path):
   """Update links to testharness.js in the BeautifulSoup |doc| to point to the copy in |new_path|. Returns whether the document was modified."""

I.e., you can get a lot terser. A "proper" pythonic docstring has a one-line summary, then a blank line, then the rest of the string, but we don't tend to stick to this that carefully and favor shorter docstrings (or none at all) where they're not really needed.

> Tools/Scripts/webkitpy/w3c/test_converter_unittest.py:3
> +# Copyright (C) 2010 Apple Inc. All rights reserved.

update copyright here as well.

> Tools/Scripts/webkitpy/w3c/test_converter_unittest.py:77
> +"""

There's a lot of boilerplate text in these tests, and it's hard to tell which parts of the string are relevant and change from test to test. We should try to wrap the test html in a generator function or something.

> Tools/Scripts/webkitpy/w3c/test_importer.py:3
> +# Copyright (C) 2010 Apple Inc. All rights reserved.

update the copyright.

> Tools/Scripts/webkitpy/w3c/test_importer_unittest.py:3
> +# Copyright (C) 2010 Apple Inc. All rights reserved.


> Tools/Scripts/webkitpy/w3c/test_parser.py:3
> +# Copyright (C) 2010 Apple Inc. All rights reserved.


> Tools/Scripts/webkitpy/w3c/test_parser_unittest.py:3
> +# Copyright (C) 2010 Apple Inc. All rights reserved.


> Tools/Scripts/webkitpy/w3c/test_parser_unittest.py:98
> +"""

same comments about repeat blocks of text.

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

More information about the webkit-unassigned mailing list