Bug 186261 – import-w3c-tests should rely on <meta name="flags"> to detect CSS manual tests

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Frédéric Wang (:fredw)]

CSS tests have additional flags, which can be used to indicate that the tests are manual:

https://web-platform-tests.org/writing-tests/css-metadata.html#requirement-flags

From WPT's tools/manifest/sourcefile.py:

def content_is_css_manual(self):
        """Boolean indicating whether the file content represents a
        CSS WG-style manual test"""
        if self.root is None:
            return None
        # return True if the intersection between the two sets is non-empty
return bool(self.css_flags & {"animated", "font", "history", "interact", "paged", "speech", "userstyle"})

Currently, import-w3c-tests just treat them as "support" files, instead of ignoring them.

[Frédéric Wang (:fredw)]

Created attachment 341895 [details]
Patch

[Ms2ger (he/him; ⌚ UTC+1/+2)]

LGTM

[youenn fablet]

Comment on attachment 341895 [details]
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'

[Frédéric Wang (:fredw)]

Comment on attachment 341895 [details]
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/

Done

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

Done

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

Done.

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

Yeah, I was also confused by existing tests. Done here and as well above "test_info should be None"

[Frédéric Wang (:fredw)]

Committed r232506: <https://trac.webkit.org/changeset/232506>

[Radar WebKit Bug Importer]

<rdar://problem/40800382>