Bug 234510 – Stop assuming WPT is a reftest based on existence of -ref.html file

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

[Tim Nguyen (:ntim)]

Only the presence of <link rel="match">/<link rel="mismatch"> indicates a test is a reftest. The script should not assume that the existence of a similarly named file with a `-ref.html` suffix means that the original test file is reftest.

[Tim Nguyen (:ntim)]

https://github.com/web-platform-tests/wpt/blob/22f29564bb82b407aeaf6507c8efffdbd51b9974/tools/manifest/sourcefile.py#L1065

[Tim Nguyen (:ntim)]

Created attachment 447601 [details]
Patch

[Manuel Rego Casasnovas]

Comment on attachment 447601 [details]
Patch

Why this was added initially? Are we going to be missing any test now (or they were actually just being imported but not working fine)?

[Tim Nguyen (:ntim)]

(In reply to Manuel Rego Casasnovas from comment #3)
> Comment on attachment 447601 [details]
> Patch
> 
> Why this was added initially? Are we going to be missing any test now (or
> they were actually just being imported but not working fine)?

Blame links to: https://github.com/WebKit/WebKit/commit/fe8645cb599fd89729602a5c992f1fa93a88f5d7

Maybe it was useful before?

Anyway, this is troublesome for the top-layer-position tests:

https://github.com/web-platform-tests/wpt/tree/master/html/semantics/interactive-elements/the-dialog-element

where we have:

top-layer-position-ref.html
top-layer-position-relative.html
top-layer-position-static.html
top-layer-position.html

The ref is for the relative & static tests, not the plain one.

[Sam Sneddon [:gsnedders]]

Comment on attachment 447601 [details]
Patch

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

(non-reviewer r-)

> Tools/Scripts/webkitpy/w3c/test_parser.py:-124
> -        elif self.is_wpt_reftest():

Removing this changes the semantics of everything that follows; notably this means things can get picked up as manual tests now when previously they were excluded for having `-ref` in the name.

Compare this with https://github.com/web-platform-tests/wpt/blob/22f29564bb82b407aeaf6507c8efffdbd51b9974/tools/manifest/sourcefile.py#L1041 where certain tests are excluded from the possibility of being picked up as manual tests as a result of this.

[Tim Nguyen (:ntim)]

Created attachment 447605 [details]
Patch

[Sam Sneddon [:gsnedders]]

Comment on attachment 447605 [details]
Patch

This looks good, aside from the lack of tests in test_parser_unittest.py for this behaviour (which we really ought to have!).

[Tim Nguyen (:ntim)]

Committed r287267 (245423@trunk): <https://commits.webkit.org/245423@trunk>

[Radar WebKit Bug Importer]

<rdar://problem/86732196>

[Tim Nguyen (:ntim)]

(In reply to Sam Sneddon [:gsnedders] from comment #7)
> Comment on attachment 447605 [details]
> Patch
> 
> This looks good, aside from the lack of tests in test_parser_unittest.py for
> this behaviour (which we really ought to have!).

I added tests in the patch I landed: https://commits.webkit.org/r287267