| Summary: | Stop assuming WPT is a reftest based on existence of -ref.html file | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tim Nguyen (:ntim) <ntim> | ||||||
| Component: | Tools / Tests | Assignee: | Tim Nguyen (:ntim) <ntim> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | darin, ews-watchlist, glenn, gsnedders, jbedard, rego, webkit-bug-importer, youennf | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | Safari 14 | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=203784 | ||||||||
| Attachments: |
|
||||||||
|
Description
Tim Nguyen (:ntim)
2021-12-20 06:29:05 PST
Created attachment 447601 [details]
Patch
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)?
(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. 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. Created attachment 447605 [details]
Patch
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!).
Committed r287267 (245423@trunk): <https://commits.webkit.org/245423@trunk> (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 |