run-web-platform-tests: remove support for in-repository manifest, expectation management
Created attachment 379225 [details] Patch
Comment on attachment 379225 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=379225&action=review > Tools/ChangeLog:19 > + Instead of keeping wpt metadata inside the WebKit repository or have it > + generated on-the-fly, provide additional option flags for the > + run-web-platform-tests script that allows detailed specification of the > + metadata, manifest and include manifest locations, if necessary. > + > + If the metadata location is not provided, the wptrunner tool will simply > + not rely on any metadata to adjust expected results. With no manifest > + path specified, the manifest will be generated inside the wpt checkout. > + If no include manifest is specified, all the tests will be initially > + selected for running (until they're possibly filtered through additional > + command line arguments). > + Having extra options to specify a random path for the metadata or manifest is nice. But I wonder why removing the current expectation files and the possibility to have them inside the webkit repository is a good thing?
(In reply to Carlos Alberto Lopez Perez from comment #2) > Comment on attachment 379225 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=379225&action=review > > > Tools/ChangeLog:19 > > + Instead of keeping wpt metadata inside the WebKit repository or have it > > + generated on-the-fly, provide additional option flags for the > > + run-web-platform-tests script that allows detailed specification of the > > + metadata, manifest and include manifest locations, if necessary. > > + > > + If the metadata location is not provided, the wptrunner tool will simply > > + not rely on any metadata to adjust expected results. With no manifest > > + path specified, the manifest will be generated inside the wpt checkout. > > + If no include manifest is specified, all the tests will be initially > > + selected for running (until they're possibly filtered through additional > > + command line arguments). > > + > > Having extra options to specify a random path for the metadata or manifest > is nice. > But I wonder why removing the current expectation files and the possibility > to have them inside the webkit repository is a good thing? The system that's being removed was based on providing the include manifest, specifying which tests should be included in the testing by default, and was using a JSON file to gather test expectations for each test and sub-test in one place. This JSON file was then parsed and the corresponding metadata files (per-test .ini files) were generated in a directory. Over time I came to think that it would be preferable if these two sets of files (include manifest and the metadata file) were managed in some separate repository. For our own ports, this could be a git repository on GitHub. I think this would be better as it would enable an easier way to modify test expectations, without flooding the WebKit's svn repository with commits adjusting test expectations for a test suite that's pretty much separate from the WebKit project. A smaller problem was the JSON file that was used to generate the expectations metadata. This was mainly used to condense all the information into one place, and to avoid populating the repository with hundreds of files. This approach had some smaller issues around how to properly generate metadata from that JSON. I would prefer, if a separate repository is used, to just rely on spawning whatever amount of metadata files is required.
(In reply to Zan Dobersek from comment #3) > (In reply to Carlos Alberto Lopez Perez from comment #2) > > Comment on attachment 379225 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=379225&action=review > > > > > Tools/ChangeLog:19 > > > + Instead of keeping wpt metadata inside the WebKit repository or have it > > > + generated on-the-fly, provide additional option flags for the > > > + run-web-platform-tests script that allows detailed specification of the > > > + metadata, manifest and include manifest locations, if necessary. > > > + > > > + If the metadata location is not provided, the wptrunner tool will simply > > > + not rely on any metadata to adjust expected results. With no manifest > > > + path specified, the manifest will be generated inside the wpt checkout. > > > + If no include manifest is specified, all the tests will be initially > > > + selected for running (until they're possibly filtered through additional > > > + command line arguments). > > > + > > > > Having extra options to specify a random path for the metadata or manifest > > is nice. > > But I wonder why removing the current expectation files and the possibility > > to have them inside the webkit repository is a good thing? > > The system that's being removed was based on providing the include manifest, > specifying which tests should be included in the testing by default, and was > using a JSON file to gather test expectations for each test and sub-test in > one place. This JSON file was then parsed and the corresponding metadata > files (per-test .ini files) were generated in a directory. > > Over time I came to think that it would be preferable if these two sets of > files (include manifest and the metadata file) were managed in some separate > repository. For our own ports, this could be a git repository on GitHub. I > think this would be better as it would enable an easier way to modify test > expectations, without flooding the WebKit's svn repository with commits > adjusting test expectations for a test suite that's pretty much separate > from the WebKit project. > I'm a bit sceptic that by keeping the metadata on another repository, managing the expectations is going to be easier. But maybe I still don't have the full picture about how this script is going to work in the end. So let's give it a try. Also I hope that discovering the metadata doesn't become more difficult. So, in a next step please consider the idea of teaching this script (or adding a README) about how to fetch that metadata more or less automatically before running.
Comment on attachment 379225 [details] Patch Clearing flags on attachment: 379225 Committed r250286: <https://trac.webkit.org/changeset/250286>
All reviewed patches have been landed. Closing bug.
<rdar://problem/55651438>
<rdar://problem/55651439>