Closed Bug 1556041 Opened 5 months ago Closed 5 months ago

add test cases for patch that added support for text-underline-offset to the style system

Categories

(Core :: CSS Parsing and Computation, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed

People

(Reporter: cmarlow, Assigned: cmarlow, Mentored)

References

()

Details

Attachments

(1 file)

Implementing changes recommended here: https://phabricator.services.mozilla.com/D33233
Mainly adding test cases and ref tests for text-underline-offset

Per https://phabricator.services.mozilla.com/D33233#986429 , I suspect emilio may actually have been asking for a "testharness.js"-based test here, not a reftest, to test the parsing behavior (the part that we'll support [aside from from-font] when your patch there lands).

(This will be sort of redundant in our own test suite, since we've got the property_database.js-based mochitests that should exercise this pretty well. But I suppose it's still good to add as a validation that our expectations about parsing matches what other browsers do. And it lets us express that e.g. from-font is supposed to be supported but is not yet, which we can do by adding a failure annotation for just that one part of the test, if we hypothetically had a web-platform-test that tests all the values.)

You could probably start with this testcase as an example to use:
https://searchfox.org/mozilla-central/source/testing/web-platform/tests/css/css-text-decor/text-decoration-skip-ink.html

Note that we fail that test right now, and we have the specific failures annotated here:
https://searchfox.org/mozilla-central/source/testing/web-platform/meta/css/css-text-decor/text-decoration-skip-ink.html.ini
(Note the paths are almost the same, except for "meta" instead of "tests" and the ".ini" suffix. This is how our test harness discovers known/expected test failures.)

So you could, for example, make a copy of that testcase and just swap out the property & values for text-underline-offset and its values in your copy.

So, supposing your new test is named e.g. "text-underline-offset-parsing-001.html", you'll need to create an .ini file in the "parallel-universe" subdirectory tree but within the web-platform/meta directory instead of web-platform/tests (like the pair of files linked above in comment 1), and that .ini file should...

(a) start out with the name of the test in square-brackets on the first line (like other WPT known-failure-annotation .ini files do)

(b) set the pref that controls your property (to enable it for this specific test), like we do here for example:
https://searchfox.org/mozilla-central/source/testing/web-platform/meta/compat/webkit-text-fill-color-property-002.html.ini

(c) If your test includes a check for the from-font value (and it probably should), then the .ini file will need to include a chunk to express that this particular check is currently expected to fail for us, similar to the failure-annotations in text-decoration-skip-ink.html.ini linked in comment 0. (I believe the failure annotations are programmatically matched up against the specific bits of logging for the failing subtest, e.g. the "assert_true" message in this case.)

Summary: fix indentations, add test cases for patch that added support for text-underline-offset to the style system → add test cases for patch that added support for text-underline-offset to the style system
Blocks: 1555863
Keywords: checkin-needed
Keywords: checkin-needed

One test case for the from-font feature is expected to fail (noted in it's ini file), when this is implemented later it should pass

Pushed by aethanyc@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/83167321bb45
added web platform tests for text-underline-offset r=emilio
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/17194 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
Upstream PR merged
You need to log in before you can comment on or make changes to this bug.