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

Do the massive renaming of manual/visual tests #25535

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

gsnedders
Copy link
Member

This PR fixes #5381. It also renames the existing print reftests we have to be correctly picked up by the manifest as such (though some of these are within css/vendor-imports/mozilla, so if we want to continue doing a one-way sync there can @jgraham or someone else land these changes in mozilla-central?).

This test, while it tests fragmentation, isn't testing paged
layout and equally applies to scrolled media. It currently is classified
by the manifest as a manual test due to it being a paged layout test
which until recently we did not have any automation to handle.
These are currently classified as visual tests which is undesirable
In many ways this is part of web-platform-tests#5381 as most people consider these to
still be manual tests.
@gsnedders
Copy link
Member Author

Maybe the last commit on here is a bad idea in the short term, given it stops the lint from finding anything new that's added in the old style?

@frivoal
Copy link
Contributor

frivoal commented Sep 29, 2020

I didn't review every single one of these 4000 and some files, but at a glance, this looks right.

Note though that it will cause some (easily fixed) breakage in the specs that use bikeshed's <wpt> tag to document which tests apply to which part of the spec, so when you land this, it'd be nice to the the equivalent rename on the spec side as well. Should be doable mechanically, so it's not a big deal, but that'll be much nicer handled by the handlers of this pull request than by individual spec editors noticing after the fact (maybe long after the fact) than some of their tests have moved.

@frivoal
Copy link
Contributor

frivoal commented Oct 5, 2020

Poor timing maybe, but I've added refs to quite a bunch of previously manual CSS2 tests relating to text (as in css-text-3). Unsurprisingly, it looks like this conflicts with your patch :(

@gsnedders
Copy link
Member Author

Poor timing maybe, but I've added refs to quite a bunch of previously manual CSS2 tests relating to text (as in css-text-3). Unsurprisingly, it looks like this conflicts with your patch :(

On the other hand, the actual moving was easily scripted from the manifest, so any merge conflict is likely to be resolved by simply re-running the script. :)

@svgeesus svgeesus removed their request for review November 22, 2021 11:33
@dbaron
Copy link
Member

dbaron commented Jun 23, 2023

This has been sitting around for a while. Should it still happen? If so, should the script in question get re-run?

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

Successfully merging this pull request may close these issues.

Sort out -manual filename convention vs meta element "interact" flag
5 participants