Skip to content

Commit a65f274

Browse files
Ben WagnerSkCQ
authored andcommitted
[paragraph] Clean up tests
Have EndWithLineSeparator and EmojiFontResolution tests use the ResourceFontCollection instead of the TestFontMgr for consistency. Convert a few REPORTER_ASSERT to report the actual values used in the assert. Simplify ResourceFontCollection construction by creating the typefaces while iterating over the file paths, instead of storing the file paths and iterating over them again. Change-Id: Ia63e3352f66e467d482688c50a0978b3e8c5b3b5 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/834702 Reviewed-by: Julia Lavrova <[email protected]> Commit-Queue: Ben Wagner <[email protected]> Auto-Submit: Ben Wagner <[email protected]>
1 parent e82ccda commit a65f274

File tree

1 file changed

+20
-28
lines changed

1 file changed

+20
-28
lines changed

modules/skparagraph/tests/SkParagraphTest.cpp

Lines changed: 20 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -123,26 +123,16 @@ class ResourceFontCollection : public FontCollection {
123123
}
124124
SkString fontResources = GetResourcePath(FLAGS_paragraph_fonts[0]);
125125
const char* fontDir = fontResources.c_str();
126-
std::vector<SkString> fonts;
127126
SkOSFile::Iter iter(fontDir);
128127

129128
SkString path;
129+
sk_sp<SkFontMgr> mgr = ToolUtils::TestFontMgr();
130130
while (iter.next(&path)) {
131131
if ((false)) {
132132
SkDebugf("font %s\n", path.c_str());
133133
}
134-
// Look for a sentinel font, without which several tests will fail/crash.
135-
if (path.endsWith("Roboto-Italic.ttf")) {
136-
fFontsFound = true;
137-
}
138-
fonts.emplace_back(path);
139-
}
140-
SkASSERTF(fFontsFound, "--paragraph_fonts was set but didn't have the fonts we need");
141-
142-
sk_sp<SkFontMgr> mgr = ToolUtils::TestFontMgr();
143-
for (auto& font : fonts) {
144134
SkString file_path;
145-
file_path.printf("%s/%s", fontDir, font.c_str());
135+
file_path.printf("%s/%s", fontDir, path.c_str());
146136
auto stream = SkStream::MakeFromFile(file_path.c_str());
147137
SkASSERTF(stream, "%s not readable", file_path.c_str());
148138
sk_sp<SkTypeface> face = mgr->makeFromStream(std::move(stream), {});
@@ -156,13 +146,16 @@ class ResourceFontCollection : public FontCollection {
156146
familyName.c_str(), face->openExistingStream(nullptr)->getLength());
157147
}
158148
fFontProvider->registerTypeface(std::move(face));
149+
if (path.endsWith("Roboto-Italic.ttf")) {
150+
fFontsFound = true;
151+
}
159152
} else {
160153
SkDEBUGF("%s was not turned into a Typeface. Did you set --nativeFonts?\n",
161154
file_path.c_str());
162155
}
163156
}
164-
SkASSERTF(fFontProvider->countFamilies(),
165-
"No font families found. Did you set --nativeFonts?");
157+
SkASSERTF(fFontProvider->countFamilies(), "No families found. Did you set --nativeFonts?");
158+
SkASSERTF(fFontsFound, "--paragraph_fonts was set but didn't have the fonts we need");
166159

167160
if (testOnly) {
168161
this->setTestFontManager(std::move(fFontProvider));
@@ -251,7 +244,7 @@ class TestCanvas {
251244
#define SKIP_IF_FONTS_NOT_FOUND(r, fontCollection) \
252245
if (!fontCollection->fontsFound()) { \
253246
if (FLAGS_paragraph_fonts.size() != 0) { \
254-
ERRORF(r, "SkParagraphTests Fonts not found!"); \
247+
ERRORF(r, "SkParagraphTests Fonts not found!\n"); \
255248
} \
256249
return; \
257250
}
@@ -1596,7 +1589,7 @@ UNIX_ONLY_TEST(SkParagraph_StrutHalfLeadingSimple, reporter) {
15961589
REPORTER_ASSERT(reporter, SkScalarNearlyEqual(firstLine.fHeight, 200.0f));
15971590

15981591
// Half leading does not move the text horizontally.
1599-
REPORTER_ASSERT(reporter, SkScalarNearlyEqual(boxes[1].rect.left(), 0, EPSILON100));
1592+
REPORTER_ASSERT(reporter, SkScalarNearlyEqual(boxes[0].rect.left(), 0, EPSILON100));
16001593
}
16011594

16021595
UNIX_ONLY_TEST(SkParagraph_StrutHalfLeadingMultiline, reporter) {
@@ -4107,11 +4100,11 @@ UNIX_ONLY_TEST(SkParagraph_EmojiParagraph, reporter) {
41074100
REPORTER_ASSERT(reporter, impl->lines().size() == 8);
41084101
for (auto& line : impl->lines()) {
41094102
if (&line != impl->lines().end() - 1) {
4110-
REPORTER_ASSERT(reporter, line.width() == 998.25f);
4103+
REPORTER_ASSERT(reporter, line.width() == 998.25f, "width: %f", line.width());
41114104
} else {
4112-
REPORTER_ASSERT(reporter, line.width() < 998.25f);
4105+
REPORTER_ASSERT(reporter, line.width() < 998.25f, "width: %f", line.width());
41134106
}
4114-
REPORTER_ASSERT(reporter, line.height() == 59);
4107+
REPORTER_ASSERT(reporter, line.height() == 59, "height: %f", line.height());
41154108
}
41164109
}
41174110

@@ -7317,12 +7310,12 @@ UNIX_ONLY_TEST(SkParagraph_lineMetricsAfterUpdate, reporter) {
73177310

73187311
std::vector<LineMetrics> lm;
73197312
paragraph->getLineMetrics(lm);
7320-
REPORTER_ASSERT(reporter, lm.size() == 1);
7313+
REPORTER_ASSERT(reporter, lm.size() == 1, "size: %zu", lm.size());
73217314

73227315
paragraph->updateFontSize(0, text.size(), 42);
73237316
paragraph->layout(200.);
73247317
paragraph->getLineMetrics(lm);
7325-
REPORTER_ASSERT(reporter, lm.size() == 2);
7318+
REPORTER_ASSERT(reporter, lm.size() == 2, "size: %zu", lm.size());
73267319
}
73277320

73287321
// Google logo is shown in one style (the first one)
@@ -8082,9 +8075,8 @@ UNIX_ONLY_TEST(SkParagraph_SingleDummyPlaceholder, reporter) {
80828075
}
80838076

80848077
UNIX_ONLY_TEST(SkParagraph_EndWithLineSeparator, reporter) {
8085-
sk_sp<FontCollection> fontCollection = sk_make_sp<FontCollection>();
8086-
fontCollection->setDefaultFontManager(ToolUtils::TestFontMgr());
8087-
fontCollection->enableFontFallback();
8078+
sk_sp<ResourceFontCollection> fontCollection = sk_make_sp<ResourceFontCollection>();
8079+
SKIP_IF_FONTS_NOT_FOUND(reporter, fontCollection)
80888080

80898081
const char* text = "A text ending with line separator.\u2028";
80908082
const size_t len = strlen(text);
@@ -8110,9 +8102,8 @@ UNIX_ONLY_TEST(SkParagraph_EndWithLineSeparator, reporter) {
81108102
}
81118103

81128104
[[maybe_unused]] static void SkParagraph_EmojiFontResolution(sk_sp<SkUnicode> icu, skiatest::Reporter* reporter) {
8113-
auto fontCollection = sk_make_sp<FontCollection>();
8114-
fontCollection->setDefaultFontManager(ToolUtils::TestFontMgr(), std::vector<SkString>());
8115-
fontCollection->enableFontFallback();
8105+
sk_sp<ResourceFontCollection> fontCollection = sk_make_sp<ResourceFontCollection>();
8106+
SKIP_IF_FONTS_NOT_FOUND(reporter, fontCollection)
81168107

81178108
const char* text = "♻️🏴󠁧󠁢󠁳󠁣󠁴󠁿";
81188109
const char* text1 = "♻️";
@@ -8129,6 +8120,7 @@ UNIX_ONLY_TEST(SkParagraph_EndWithLineSeparator, reporter) {
81298120
paragraph->layout(SK_ScalarMax);
81308121

81318122
auto impl = static_cast<ParagraphImpl*>(paragraph.get());
8123+
REPORTER_ASSERT(reporter, impl->runs().size() == 1, "size: %zu", impl->runs().size());
81328124

81338125
ParagraphBuilderImpl builder1(paragraph_style, fontCollection);
81348126
builder1.pushStyle(text_style);
@@ -8137,7 +8129,7 @@ UNIX_ONLY_TEST(SkParagraph_EndWithLineSeparator, reporter) {
81378129
paragraph1->layout(SK_ScalarMax);
81388130

81398131
auto impl1 = static_cast<ParagraphImpl*>(paragraph1.get());
8140-
REPORTER_ASSERT(reporter, impl1->runs().size() == 1);
8132+
REPORTER_ASSERT(reporter, impl1->runs().size() == 1, "size: %zu", impl1->runs().size());
81418133
if (impl1->runs().size() == 1) {
81428134
SkString ff;
81438135
impl->run(0).font().getTypeface()->getFamilyName(&ff);

0 commit comments

Comments
 (0)