Make it easier to write RenderTextHarfBuzz tests (wrt. text runs) |
||
Issue description
Say you have a test for HarfBuzz run lists:
const base::string16 mixed = WideToUTF16(
L"\x05D0\x05D1"
L"1234"
L"\x05D2\x05D3"
L"abc");
render_text->SetText(mixed);
render_text->SetDirectionalityMode(DIRECTIONALITY_FORCE_LTR);
test_api()->EnsureLayout();
internal::TextRunList* run_list = GetHarfBuzzRunList();
What if I told you we could take expectations like this:
ASSERT_EQ(4U, run_list->size());
EXPECT_TRUE(run_list->runs()[0]->is_rtl);
EXPECT_FALSE(run_list->runs()[1]->is_rtl);
EXPECT_TRUE(run_list->runs()[2]->is_rtl);
EXPECT_FALSE(run_list->runs()[3]->is_rtl);
EXPECT_EQ(2U, run_list->logical_to_visual(0));
EXPECT_EQ(1U, run_list->logical_to_visual(1));
EXPECT_EQ(0U, run_list->logical_to_visual(2));
EXPECT_EQ(3U, run_list->logical_to_visual(3));
and rewrite them like this:
EXPECT_EQ("[7<-6][2->5][1<-0][8->10]", RunListDebugString(*run_list));
By making a TextRunList into a human-readable string then comparing it in the test, we get:
- More compact tests: Instead of two lines of code for each run, one line for the whole runlist.
- More readable tests: You can actually see the author's intent.
- More readable failure messages: Shows the expected and actual runlist string, rather than individual numerical failures.
- Tests more: Some of these tests *only* test the order of runs (not their ranges). Other tests *only* test the ranges (not the order). The proposed style tests both in one.
The debug string shows the runs in visual order. Each run is enclosed in square brackets, and shows the begin and end inclusive logical character position, with an arrow indicating the direction of the run. Single-character runs just show the character position.
For example, the corresponding run-list for the string "abc+אבג" is expressed as "[0->2][3][6<-4]".
So, that's the plan.
,
Aug 23 2017
I like it a lot! Thanks for putting some good thought into this, reviewing now.
,
Aug 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5bd2a3472d09ca58ce9de2a74ad6cc867115c5d3 commit 5bd2a3472d09ca58ce9de2a74ad6cc867115c5d3 Author: Matt Giuca <mgiuca@chromium.org> Date: Thu Aug 24 07:07:12 2017 RenderTextHarfBuzzTest: Rewrite run-list tests by comparing strings. Previously, run-list tests would directly inspect the run-list object, resulting in long, error-prone and unreadable tests. This adds functionality to convert a run-list into a human-readable string that can be compared with a one-line expectation. This allows tests to be shorter, more readable, check more things and provide more readable failure messages. Bug: 758063 Change-Id: I8746cf8ade460da1ae5b286ea3150f35678aa668 Reviewed-on: https://chromium-review.googlesource.com/627529 Commit-Queue: Matt Giuca <mgiuca@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Cr-Commit-Position: refs/heads/master@{#496970} [modify] https://crrev.com/5bd2a3472d09ca58ce9de2a74ad6cc867115c5d3/ui/gfx/render_text_unittest.cc
,
Aug 24 2017
|
||
►
Sign in to add a comment |
||
Comment 1 by mgiuca@chromium.org
, Aug 23 2017