run-webkit-tests should detect w3c test resource files

• Component: Tools / Tests
• Status: RESOLVED FIXED

[youenn fablet]

Currently we use TestExpectations to skip web-platform-tests resource files, since some of these files are not in 'resources' folders.
It would be better to put that information inside a new file outside TestExpectations as this clutters TestExpectations.
Additional benefit might be the possibility to generate automatically that new file when importing tests.

[youenn fablet]

Created attachment 287262 [details]
Patch

[youenn fablet]

Created attachment 287263 [details]
Patch

[youenn fablet]

Comment on attachment 287263 [details]
Patch

On my setup when doing --print-expectations before and after this patch, the list of runned test is the same.

[Alexey Proskuryakov]

Comment on attachment 287263 [details]
Patch

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

> Tools/ChangeLog:4
> +        run-webkit-tests should detect w3c test resource files
> +        https://bugs.webkit.org/show_bug.cgi?id=161307

Would it be feasible to make W3C tests adhere to an automation friendly layout, such as using a resources folder? Seems like everyone would have this problem.

[youenn fablet]

> Would it be feasible to make W3C tests adhere to an automation friendly
> layout, such as using a resources folder? Seems like everyone would have
> this problem.

Given the number of files that are skipped here, the refactoring time would be quite important.

The wpt test infrastructure allows generating a manifest file containing all tests (using a python script, see web-platform-tests/tools, probably in manifest.py file).

We could reuse that tool and that file.
The downside is that whenever a WebKit patch is adding a test or a test-suite to LayoutTests/imported/w3c, this file would need to be regenerated/updated so that the bots do not skip the new tests.

It might be easier to only update which files are test/not-test when doing a full wpt resync, typically doing the following:
- generate the manifest file using wpt script
- generate our own skip-list using the manifest
w3c test importer could do that easily I think.

[youenn fablet]

Ping review?

[Ryosuke Niwa]

Comment on attachment 287263 [details]
Patch

We should really fix this in the upstream repository though.  Having to track random source files like this isn't a scalable solution in the long term.

[youenn fablet]

(In reply to comment #7)
> Comment on attachment 287263 [details]
> Patch
> 
> We should really fix this in the upstream repository though.  Having to
> track random source files like this isn't a scalable solution in the long
> term.

The w3c importer could handle the generation of this file based on wpt test manifest.

[WebKit Commit Bot]

Comment on attachment 287263 [details]
Patch

Rejecting attachment 287263 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 287263, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
eLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/imported/w3c/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/TestExpectations
Hunk #2 FAILED at 259.
1 out of 2 hunks FAILED -- saving rejects to file LayoutTests/TestExpectations.rej
patching file LayoutTests/imported/w3c/resources/resource-files.json

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Ryosuke Niwa']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.webkit.org/results/1999349

[youenn fablet]

Created attachment 287857 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 287857 [details]
Patch for landing

Clearing flags on attachment: 287857

Committed r205399: <http://trac.webkit.org/changeset/205399>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryosuke Niwa]

(In reply to comment #8)
> (In reply to comment #7)
> > Comment on attachment 287263 [details]
> > Patch
> > 
> > We should really fix this in the upstream repository though.  Having to
> > track random source files like this isn't a scalable solution in the long
> > term.
> 
> The w3c importer could handle the generation of this file based on wpt test
> manifest.

But that's really silly. We shouldn't have dozens of exceptions like this. WPT should follow the convention to always put files under "resources" directory instead.

[youenn fablet]

(In reply to comment #13)
> (In reply to comment #8)
> WPT should follow the convention to always put files under "resources"
> directory instead.

This issue could be brought up, but I think it is a lost cause.
At this point, I don't know who can commit time to update the tests, find reviewers, update the lint tool, update the manifest generator script.