[webkit-reviews] review granted: [Bug 186261] import-w3c-tests should rely on <meta name="flags"> to detect CSS manual tests : [Attachment 341895] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Frédéric Wang (:fredw)
<fred.wang at free.fr>'s request for review:
Bug 186261: import-w3c-tests should rely on <meta name="flags"> to detect CSS
manual tests
https://bugs.webkit.org/show_bug.cgi?id=186261

Attachment 341895: Patch

https://bugs.webkit.org/attachment.cgi?id=341895&action=review




--- Comment #3 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 341895
  --> https://bugs.webkit.org/attachment.cgi?id=341895
Patch

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

> Tools/ChangeLog:14
> +	   <meta name="flags"> are manual are non-manual according to CSS WG
rules.

s/are/or/

> Tools/Scripts/webkitpy/w3c/test_parser.py:119
> +	   elif self.is_wpt_manualtest() or self.is_css_manualtest():

We should probably merge is_wpt_manualtest and is_css_manualtest

> Tools/Scripts/webkitpy/w3c/test_parser_unittest.py:183
> +	       self.assertEqual(test_info, None, 'test_info is not None')

Maybe we should add asis which is a valid css flag but not classified as
manual.

> Tools/Scripts/webkitpy/w3c/test_parser_unittest.py:194
> +	       self.assertTrue(test_info['manualtest'], 'test with CSS flag %s
is not manual' % flag)

I never know whether the assertion message tells what is expected or what is
unexpected.
I would rewrite it to 'test with CSS flag %s should be manual'