Bug 73680 - Convert isolate and plaintext tests to reftests
Summary: Convert isolate and plaintext tests to reftests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-02 11:40 PST by Ryosuke Niwa
Modified: 2011-12-20 14:25 PST (History)
4 users (show)

See Also:


Attachments
isolate difference (top: basic.html, below: basic-expected.html) (4.73 KB, image/png)
2011-12-02 12:13 PST, Ryosuke Niwa
no flags Details
converts the test (131.49 KB, patch)
2011-12-02 12:47 PST, Ryosuke Niwa
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2011-12-02 11:40:22 PST
Since we support reftests now, we should convert tests for -webkit-isolate and -webkit-plaintext to reftests.
Comment 1 Ryosuke Niwa 2011-12-02 12:13:21 PST
Created attachment 117667 [details]
isolate difference (top: basic.html, below: basic-expected.html)

It appears that I can't convert unicode-bidi-isolate-basic to a reftest. There's very slight difference in the way text is laid out. Notice in the attached screenshot, נ and ( are slightly to the left of the expected version.
Comment 2 Ryosuke Niwa 2011-12-02 12:47:37 PST
Created attachment 117671 [details]
converts the test
Comment 3 Ryosuke Niwa 2011-12-02 12:48:14 PST
I'll add aharon-failing to appropriate test_expectation.txt and Skipped files before landing this patch.
Comment 4 Ryosuke Niwa 2011-12-02 13:44:11 PST
Eric, Dan, & Levi, do you have any idea why we have a different rendering in isolate and inline-block?
Comment 5 Darin Adler 2011-12-03 20:20:36 PST
Comment on attachment 117671 [details]
converts the test

Is there a height limit on ref tests? Do they still work on content that is very tall or very wide?

Removing the expected result lines (the “reference” ones) from the original of the tests is OK for DumpRenderTree, but not so great for running the tests in the browser and looking to see if they are working. For these sorts of tests, I’d prefer to have the ref contain doubled versions of things and have the original show both the original and ref so the test is still easy to understand if you just load it up in a browser. If the test describes correctness, and the ref-test aspect is “just for the computer” that’s one thing, but please remember we want these tests to be easy to understand on their own in a web browser as well.
Comment 6 Ryosuke Niwa 2011-12-03 21:05:02 PST
(In reply to comment #5)
> (From update of attachment 117671 [details])
> Is there a height limit on ref tests? Do they still work on content that is very tall or very wide?

Reftests still have 800x600 restriction. We discussed about this at TPAC, we agreed that WebKit's 800x600 screen size is a nice once to converge to. Opera uses something like 1600x1200 and that apparently takes up a lot of space to store.

> Removing the expected result lines (the “reference” ones) from the original of the tests is OK for DumpRenderTree, but not so great for running the tests in the browser and looking to see if they are working. For these sorts of tests, I’d prefer to have the ref contain doubled versions of things and have the original show both the original and ref so the test is still easy to understand if you just load it up in a browser. If the test describes correctness, and the ref-test aspect is “just for the computer” that’s one thing, but please remember we want these tests to be easy to understand on their own in a web browser as well.

Yeah, I was thinking about this while converting these tests. One drawback of reftests is that tests and expected results aren't self-contained in one file. Maybe we just need a tool to view reftests e.g. an iframe that loads into two different frames next to each other for easy comparison.

On the other hand, I find going back & forth between test and expected result using history navigation to be most useful tool when comparing two results because human brains are pretty good at keeping visual images momentary to see the differences.
Comment 7 Ryosuke Niwa 2011-12-04 00:28:24 PST
Committed r101949: <http://trac.webkit.org/changeset/101949>
Comment 8 Darin Adler 2011-12-04 14:19:47 PST
On Mac Lion, css3/bdi-element.html is now failing.

The text ": 3 posts" is 1 pixel further to the left in the reference rendering compared to the test result.

The technique we are using here may be incorrect.
Comment 9 mitz 2011-12-04 15:00:59 PST
(In reply to comment #8)
> On Mac Lion, css3/bdi-element.html is now failing.
> 
> The text ": 3 posts" is 1 pixel further to the left in the reference rendering compared to the test result.
> 
> The technique we are using here may be incorrect.

Right, an inline can have a non-integral width, but the reference rendering uses an inline block, whose width is always rounded up.
Comment 10 Ryosuke Niwa 2011-12-04 16:06:26 PST
Oops. I didn't know that. Let me fix that. Will file a new bug and post a patch.
Comment 11 Levi Weintraub 2011-12-20 14:25:20 PST
(In reply to comment #6)
> Yeah, I was thinking about this while converting these tests. One drawback of reftests is that tests and expected results aren't self-contained in one file. Maybe we just need a tool to view reftests e.g. an iframe that loads into two different frames next to each other for easy comparison.

You can simply put both the test and expected results in the test file, and duplicate expected results in the expectation file.