Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removing -support naming convention #15749

Closed
jugglinmike opened this issue Mar 9, 2019 · 3 comments · Fixed by #17645
Closed

Removing -support naming convention #15749

jugglinmike opened this issue Mar 9, 2019 · 3 comments · Fixed by #17645

Comments

@jugglinmike
Copy link
Contributor

When identifying tests, the WPT infrastructure ignores files whose name (not including extension) ends in -support. Today, only 13 files meet that criteria:

$ git ls-files | grep '\-support\.'
WebIDL/ecmascript-binding/constructors-support.html
css/css-values/vh-calc-support.html
css/css-values/vh-support.html
css/css-values/vh-zero-support.html
dom/traversal/traversal-support.js
event-timing/resources/event-timing-support.js
eventsource/eventsource-onmessage-realm-support.htm
fullscreen/rendering/fullscreen-pseudo-class-support.html
html/semantics/forms/resetting-a-form/reset-form-event-realm-support.html
html/syntax/parsing/DOMContentLoaded-defer-support.js
html/the-xhtml-syntax/parsing-xhtml-documents/xhtml-mathml-dtd-entity-support.htm
html/webappapis/scripting/events/resources/compiled-event-handler-settings-objects-support.html

The infrastructure also ignores files placed within subdirectories named "resources", "tools", or "support". That approach is far more common:

$ git ls-files | grep -E '(/|^)(resources|tools|support)/.*\.(html?|xht)' | wc -l
851

I'd like remove the infrastructure that recognizes the -support file name pattern and relocate the files that rely on it to an appropriately-named subdirectory. My motivation is to reduce the learning curve for contribution and limit the number of ways to express the same thing. In addition, four of the -support files listed above appear to actually be tests. If so, they are evidence that the naming scheme is prone to accidental opt-in.

Does anyone know of a reason why this simplification cannot or should not be made? @gsnedders or @jgraham, perhaps?

@gsnedders
Copy link
Member

Yeah, this seems a bit surprising. Totally happy to drop this.

@foolip
Copy link
Member

foolip commented Mar 15, 2019

@gsnedders is this straightforward? Can I assign to you and plan to fix in the coming months?

@Ms2ger
Copy link
Contributor

Ms2ger commented May 15, 2019

I guess this is fine, though I don't think we particularly need to start moving these tests. I expect the actual support files will end up in the default case in SourceFile.manifest_items() and be treated as support files anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants