On 01/06/2018 19:50, Robert Ma wrote: > Chromium stopped relying on magic filenames for WPT reftests a long time ago. We now have WPT manifest support built into run_webkit_tests: if a test is in WPT, we get its reference from the WPT manifest instead of mangling the filename. The logic is hooked into the base port (not sure how similar our code bases are nowadays, but you'll get the idea). > > Hence, there's no renaming during import or export. > > I'd suggest you to investigate supporting WPT manifest properly, for the following reasons: > > WPT reftests can be complicated. Each test can have multiple match/mismatch references. The relationship actually forms a digraph. It's much easier/reliable to use the manifest than parsing the <link> tags. > Testharness tests can have some metadata like long timeout and variants (one test file expands to multiple tests), which are also encoded in the manifest. > > The WPT manifest Python module is 2/3 compatible with limited dependencies and it should be possible to import it to webkitpy.
Some more references about expectation manifest: https://wptrunner.readthedocs.io/en/latest/expectation.html#expectation-manifests https://github.com/web-platform-tests/wpt/tree/master/tools/wptrunner#expectation-data This is supported by upstream WPT but also by runners from Mozilla and Chromium repositories. As mentioned in comment 0, this avoids the renaming of -ref.html => -expected.html for reftests, which is causing issues like bug 186132. In addition, this can also avoid the need for -expected.txt files for testharness.js tests (bug 82245).
(In reply to Frédéric Wang (:fredw) from comment #1) > Some more references about expectation manifest: > https://wptrunner.readthedocs.io/en/latest/expectation.html#expectation- > manifests > https://github.com/web-platform-tests/wpt/tree/master/tools/ > wptrunner#expectation-data > > This is supported by upstream WPT but also by runners from Mozilla and > Chromium repositories. > > As mentioned in comment 0, this avoids the renaming of -ref.html => > -expected.html for reftests, which is causing issues like bug 186132. This is a pain point and I agree aligning with WPT makes sense for ref tests. We can probably generate/update the manifest at import time and use it to know whether a WPT test is a ref test or not. We could later extend it to determine the list of WPT tests to run, but that can be done/discussed as a follow-up. > In addition, this can also avoid the need for -expected.txt files for > testharness.js tests (bug 82245). I would tend to decorrelate this particular issue from the use of manifests.
(In reply to youenn fablet from comment #2) > > In addition, this can also avoid the need for -expected.txt files for > > testharness.js tests (bug 82245). > > I would tend to decorrelate this particular issue from the use of manifests. Yes, I agree this would be nice but being able to properly import/export reftests seems more important for now.
I don't think we should do this. There was a long discussion about it in webkit-dev. See https://lists.webkit.org/pipermail/webkit-dev/2011-November/018470.html
(In reply to Frédéric Wang (:fredw) from comment #1) > Some more references about expectation manifest: > https://wptrunner.readthedocs.io/en/latest/expectation.html#expectation- > manifests > https://github.com/web-platform-tests/wpt/tree/master/tools/ > wptrunner#expectation-data I think I've been mixing up things, there are optional expectations manifests but there is also the generated MANIFEST.json ( https://wptrunner.readthedocs.io/en/latest/usage.html#id2 ). I think in most cases, just using MANIFEST.json will be enough to solve the issues we have with reftests. (In reply to Ryosuke Niwa from comment #4) > I don't think we should do this. There was a long discussion about it in > webkit-dev. See > https://lists.webkit.org/pipermail/webkit-dev/2011-November/018470.html I wonder if this discussion is really up-to-date given Robert Ma's comment that Chromium finally decided to switch to WPT manifests instead of using the filename convention or link tags. How do you plan to solve the issues with reftest import/export?
> I think I've been mixing up things, there are optional expectations > manifests but there is also the generated MANIFEST.json ( > https://wptrunner.readthedocs.io/en/latest/usage.html#id2 ). I think in most > cases, just using MANIFEST.json will be enough to solve the issues we have > with reftests. Right, the idea would be to read manifest.json from webkitpy whenever trying to run some WPT tests. That would give us the reference files that should be validated for a given test. That does seem ok to me. I do not know how much this is used but I think WPT allows to check a given test against several references. This would probably require more work on webkitpy side.
(In reply to youenn fablet from comment #6) > > I think I've been mixing up things, there are optional expectations > > manifests but there is also the generated MANIFEST.json ( > > https://wptrunner.readthedocs.io/en/latest/usage.html#id2 ). I think in most > > cases, just using MANIFEST.json will be enough to solve the issues we have > > with reftests. > > I do not know how much this is used but I think WPT allows to check a given > test against several references. This would probably require more work on > webkitpy side. Darin objected to this kind of solution in the past and I have no reason to believe his opinion has changed. There are a number of others who have objected to respecting manifest.json in the past. Regardless, if you strongly feel about this matter, you should start a new webkit-dev thread.