Bug 203783 – Stop parsing reftest.list files

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

[Simon Fraser (smfr)]

Code in _parse_reftest_list() in port/base.py (very odd that this is in port!) parses reftest.list files when those exist, and uses that to decide what reference to use for a test.

reftest.list seems to be a vestige of Mozilla-imported tests (https://developer.mozilla.org/en-US/docs/Mozilla/QA/web-platform-tests#Metadata). WPT doesn't use these lists any more and instead uses <link rel="match"> from the tests themselves.

In addition, when we import WPT I think we have to clone a bunch of shared references into lots of identical -expected.html, which we would not if we respected <link rel="match">

[Simon Fraser (smfr)]

retest.list is only present in:
LayoutTests % find . -name reftest.list                    
./imported/w3c/web-platform-tests/encoding/reftest.list
./media/track/opera/track/webvtt/rendering/reftest/reftest.list

We need to re-import encoding tests to get rid of the first one. Maybe we can migrate to WPT media/track tests to eliminate the second?

[Carlos Alberto Lopez Perez]

I'm updating the imported WPT encoding tests in bug 207644
I encountered unexpected failures with those tests when updating them and then I realized there where old -ref.html and new -expected.html references so I found about this reftest.list thing :)

I'm deleting there the imported/w3c/web-platform-tests/encoding/reftest.list file and moving the references affected to use the usual convention of -expected.html

It seems support for reftest.list was introduced in bug 66837

[Sam Sneddon [:gsnedders]]

(In reply to Simon Fraser (smfr) from comment #1)
> We need to re-import encoding tests to get rid of the first one. Maybe we
> can migrate to WPT media/track tests to eliminate the second?

We should be able to; zcorpan's WebVTT tests were added to WPT long ago.

[Sam Sneddon [:gsnedders]]

Looking into this, it seems like we _already_ don't run anything from reftest.list; I think ./Tools/Scripts/webkit-patch print-baselines is the only command that actually ends up running the code path that actually parses them!

So I don't think we need to block on finally importing the WebVTT tests from WPT, given we aren't losing any test coverage if we remove them?

[Sam Sneddon [:gsnedders]]

Ah, finally got to the bottom of this. https://trac.webkit.org/changeset/136459/webkit added the Opera tests, _but_ https://trac.webkit.org/changeset/101727/webkit had meant we never consider file in a directory called "reftest" as a tests (look at skipped_directories).

So we've literally never run anything from ./media/track/opera/track/webvtt/rendering/reftest/reftest.list, which I guess means we have less test coverage than we thought. Thus we don't need to worry about lost coverage from dropping support for this!

[Sam Sneddon [:gsnedders]]

Created attachment 416466 [details]
Patch

[Jonathan Bedard]

Comment on attachment 416466 [details]
Patch

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

> Tools/ChangeLog:10
> +        We do still have one reftest.list file, media/track/opera/track/webvtt/rendering/reftest/reftest.list, but as this is in a reftest directory we don't run any of the tests in it (and never have). See the above bug for more detail about that history.

Should probably be 2 lines.

[Sam Sneddon [:gsnedders]]

Created attachment 416467 [details]
Patch

[EWS]

Committed r270954: <https://trac.webkit.org/changeset/270954>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 416467 [details].

[Radar WebKit Bug Importer]

<rdar://problem/72443355>