Remove font flag from tests with @font-face #5341
Conversation
Firefox (nightly channel)Testing web-platform-tests at revision af83e41 |
Chrome (unstable channel)Testing web-platform-tests at revision af83e41 |
|
Notifying @khaledhosny, @rsheeter, and @svgeesus. (Learn how reviewing works.) |
1 similar comment
|
Notifying @khaledhosny, @rsheeter, and @svgeesus. (Learn how reviewing works.) |
gsnedders
force-pushed
the
gratuitous_font_meta_flag
branch
from
35aaf73
to
d936f5f
Compare
|
These tests are now available on w3c-test.org |
|
The WOFF and WOFF2 tests require fallback fonts to be installed. |
|
@khaledhosny WOFF Test CFF Fallback, I assume? Thanks for pointing that out! |
gsnedders
force-pushed
the
gratuitous_font_meta_flag
branch
from
d936f5f
to
324589c
Compare
|
I believe the randomness in Chrome is just w3c/wptrunner#241 rather than any issue with the tests. |
|
@gsnedders that is one of them, the other three are SFNT-CFF-Reference.otf, SFNT-TTF-Fallback.ttf, and SFNT-TTF-Reference.ttf |
|
Given @wpt-pr-bot seems to have failed, @kojiishi and @r12a this alters a lot of your tests. |
|
Why are we going to so much trouble to remove the font flags, rather than just ignoring them. I use the test files i send to WPT elsewhere (where the font flag is used), and now i have to change all of those files if i'm to spot important differences that arise. I really don't have time for non-essential work, so please explain why we are removing the font flags. |
|
@r12a Where are you sending them that uses font flags? csswg-test is merged into wpt now, so if that's what you're thinking of that's not relevant anymore, and if that was the case the font flags were wrong anyway (the tests use The reason to get rid of them is that they are found (by tooling for both the old csswg-test repo and wpt as a whole) as manual tests (as they, per metadata, have a manual step: installing a font), when in fact they don't need to be manual at all and can run in automated systems. |
|
When i run tests in the i18n test framework such as this one the framework pops up a message that a font is required. I could probably be easily convinced that that message is not necessary, since i link to a WOFF font (although i have a nagging feeling that there are some tests that need a non-Latin font and don't link to a woff font - i should probably track those down). But my preferred solution to that would be to change the framework so that it ignores redundant font tags, rather than changing all the test files. Is that not possible? |
|
@r12a How do you detect a redundant font tag? In the general case, that's impossible (JavaScript, halting problem, etc.). But yes, I'm going to claim that if the test has |
What are the failures? I'm still not used to this system yet how/where to look at. You're just removing a flag, correct? Why would that cause failures? |
|
@kojiishi it just failed to ping you, probably because of the number of files modified |
|
Oh I understand what you meant now. I don't have needs to keep the master files separated, unlike @r12a, so fine for me. Thank you for the ping. |
|
@r12a ping re the above |
|
Now the WOFF2 tests are sorted (yes, they do need a font flag as they rely on specific local fonts to be installed which makes test failure visible) this change looks good to me. |
@gsnedders are you waiting on me to change the tests, or just give you permission to do so? I thought you were going to ignore font flags if a web font is used, is that not going to happen? btw, you may want to change the wording in the TTWF instructions, since i had been following that. It says:
which to my mind suggested that a font flag was needed for embedded fonts too (what i supposed 'font linked to' meant). I assume now that it just meant adding a link so that people can find the font. It would be useful to explain that adding the font flag makes the framework treat the tests as a manual font. |
|
@r12a Give me permission; the changes are already done in this pull request. If you're okay with it, let's just land this. |
Per the documentation: > Requires a specific font to be installed. (Details must be provided and/or the font linked to in the test description) As a result, tests with @font-face don't need the font flag, as they don't require a font to be installed. I think this manages to remove it from all such test cases, but I can't be sure that there isn't something UTF-16 encoded lying around somewhere.
gsnedders
force-pushed
the
gratuitous_font_meta_flag
branch
from
324589c
to
f1985bb
Compare
|
@gsnedders I would be happy merging this except the travis-ci fail troubles me. If you feel that can be ignored then consider this a favourable review. |
wpt-pr-bot
requested review from
frivoal,
nox,
fantasai,
kojiishi,
plinss,
hakatashi and
r12a
Make it so. Sorry for the delay - it's a bit busy here. |
Per the documentation:
As a result, tests with @font-face don't need the font flag, as they
don't require a font to be installed.
I think this manages to remove it from all such test cases, but I can't
be sure that there isn't something UTF-16 encoded lying around
somewhere.
Partly resolves w3c/wpt-tools#196.