DEV/flutter_flutter
1
0
Fork 0
mirror of https://github.com/flutter/flutter.git synced 2026-02-20 02:29:02 +08:00
Code Issues Packages Projects Releases Wiki Activity

[DisplayList] Fix assertions on DisplayList verbose comparison tests (flutter/engine#54065)

Browse Source 
Fixes 2 problems recently uncovered in the DisplayList verbose comparison test mechanism:

- The verbose compare methods never asserted a test failure, relying on the caller to do so from their return value - but they also did not prompt the caller to check the return value. So a `[[nodiscard]]` is added to remind test writers that they need to assert on the return value
- As a result of the above, some bad tests were recently added to the tree that were failing but did not assert a test failure. Now that the `[[nodiscard]]` is added, they failed to compile and had to have asserts added.

A secondary problem is that those non-failing tests were inadvertently cherry-picked from a reverted PR that is being reintroduced in incremental sections so as to avoid large scale golden image failures. The tests depend on parts of that PR that haven't been pulled forward yet (but will soon be) so those tests shouldn't have been added in the first place (and were failing, but not causing a gtest failure because of their missing asserts). They remain here, but their results are reversed to indicate the current state of affairs (they assert that the missing functionality isn't in place yet). Their assertions will be reverted when/as the missing functionality is pulled forward in a more incremental (responsible) way.
This commit is contained in:
Jim Graham 2024-07-23 17:18:05 -07:00 committed by GitHub
parent 361e7a07fa
commit 57d78e3da4
2 changed files with 46 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
engine/src/flutter/display_list/display_list_unittests.cc
View File

@ -676,12 +676,12 @@ TEST_F(DisplayListTest, SingleOpDisplayListsRecapturedAreEqual) {
sk_sp<DisplayList> copy = copy_builder.Build();
auto desc =
group.op_name + "(variant " + std::to_string(i + 1) + " == copy)";
DisplayListsEQ_Verbose(dl, copy);
ASSERT_TRUE(DisplayListsEQ_Verbose(dl, copy));
ASSERT_EQ(copy->op_count(false), dl->op_count(false)) << desc;
ASSERT_EQ(copy->bytes(false), dl->bytes(false)) << desc;
ASSERT_EQ(copy->op_count(true), dl->op_count(true)) << desc;
ASSERT_EQ(copy->bytes(true), dl->bytes(true)) << desc;
EXPECT_EQ(copy->total_depth(), dl->total_depth()) << desc;
ASSERT_EQ(copy->total_depth(), dl->total_depth()) << desc;
ASSERT_EQ(copy->bounds(), dl->bounds()) << desc;
ASSERT_TRUE(copy->Equals(*dl)) << desc;
ASSERT_TRUE(dl->Equals(*copy)) << desc;
@ -4955,7 +4955,7 @@ TEST_F(DisplayListTest, DrawRectRRectPromoteToDrawRect) {
expected.DrawRect(rect, DlPaint());
auto expect_dl = expected.Build();
DisplayListsEQ_Verbose(dl, expect_dl);
ASSERT_TRUE(DisplayListsEQ_Verbose(dl, expect_dl));
}
TEST_F(DisplayListTest, DrawOvalRRectPromoteToDrawOval) {
@ -4969,7 +4969,7 @@ TEST_F(DisplayListTest, DrawOvalRRectPromoteToDrawOval) {
expected.DrawOval(rect, DlPaint());
auto expect_dl = expected.Build();
DisplayListsEQ_Verbose(dl, expect_dl);
ASSERT_TRUE(DisplayListsEQ_Verbose(dl, expect_dl));
}
TEST_F(DisplayListTest, DrawRectPathPromoteToDrawRect) {
@ -4983,7 +4983,9 @@ TEST_F(DisplayListTest, DrawRectPathPromoteToDrawRect) {
expected.DrawRect(rect, DlPaint());
auto expect_dl = expected.Build();
DisplayListsEQ_Verbose(dl, expect_dl);
// Support for this will be re-added soon, until then verify that we
// do not promote.
ASSERT_TRUE(DisplayListsNE_Verbose(dl, expect_dl));
}
TEST_F(DisplayListTest, DrawOvalPathPromoteToDrawOval) {
@ -4997,7 +4999,9 @@ TEST_F(DisplayListTest, DrawOvalPathPromoteToDrawOval) {
expected.DrawOval(rect, DlPaint());
auto expect_dl = expected.Build();
DisplayListsEQ_Verbose(dl, expect_dl);
// Support for this will be re-added soon, until then verify that we
// do not promote.
ASSERT_TRUE(DisplayListsNE_Verbose(dl, expect_dl));
}
TEST_F(DisplayListTest, DrawRRectPathPromoteToDrawRRect) {
@ -5012,7 +5016,9 @@ TEST_F(DisplayListTest, DrawRRectPathPromoteToDrawRRect) {
expected.DrawRRect(rrect, DlPaint());
auto expect_dl = expected.Build();
DisplayListsEQ_Verbose(dl, expect_dl);
// Support for this will be re-added soon, until then verify that we
// do not promote.
ASSERT_TRUE(DisplayListsNE_Verbose(dl, expect_dl));
}
TEST_F(DisplayListTest, DrawRectRRectPathPromoteToDrawRect) {
@ -5027,7 +5033,9 @@ TEST_F(DisplayListTest, DrawRectRRectPathPromoteToDrawRect) {
expected.DrawRect(rect, DlPaint());
auto expect_dl = expected.Build();
DisplayListsEQ_Verbose(dl, expect_dl);
// Support for this will be re-added soon, until then verify that we
// do not promote.
ASSERT_TRUE(DisplayListsNE_Verbose(dl, expect_dl));
}
TEST_F(DisplayListTest, DrawOvalRRectPathPromoteToDrawOval) {
@ -5042,7 +5050,9 @@ TEST_F(DisplayListTest, DrawOvalRRectPathPromoteToDrawOval) {
expected.DrawOval(rect, DlPaint());
auto expect_dl = expected.Build();
DisplayListsEQ_Verbose(dl, expect_dl);
// Support for this will be re-added soon, until then verify that we
// do not promote.
ASSERT_TRUE(DisplayListsNE_Verbose(dl, expect_dl));
}
TEST_F(DisplayListTest, ClipRectRRectPromoteToClipRect) {
@ -5060,7 +5070,7 @@ TEST_F(DisplayListTest, ClipRectRRectPromoteToClipRect) {
expected.DrawRect(draw_rect, DlPaint());
auto expect_dl = expected.Build();
DisplayListsEQ_Verbose(dl, expect_dl);
ASSERT_TRUE(DisplayListsEQ_Verbose(dl, expect_dl));
}
TEST_F(DisplayListTest, ClipOvalRRectPromoteToClipOval) {
@ -5078,7 +5088,9 @@ TEST_F(DisplayListTest, ClipOvalRRectPromoteToClipOval) {
expected.DrawRect(draw_rect, DlPaint());
auto expect_dl = expected.Build();
DisplayListsEQ_Verbose(dl, expect_dl);
// Support for this will be re-added soon, until then verify that we
// do not promote.
ASSERT_TRUE(DisplayListsNE_Verbose(dl, expect_dl));
}
TEST_F(DisplayListTest, ClipRectPathPromoteToClipRect) {
@ -5099,7 +5111,7 @@ TEST_F(DisplayListTest, ClipRectPathPromoteToClipRect) {
expected.DrawRect(draw_rect, DlPaint());
auto expect_dl = expected.Build();
DisplayListsEQ_Verbose(dl, expect_dl);
ASSERT_TRUE(DisplayListsEQ_Verbose(dl, expect_dl));
}
TEST_F(DisplayListTest, ClipOvalPathPromoteToClipOval) {
@ -5120,7 +5132,9 @@ TEST_F(DisplayListTest, ClipOvalPathPromoteToClipOval) {
expected.DrawRect(draw_rect, DlPaint());
auto expect_dl = expected.Build();
DisplayListsEQ_Verbose(dl, expect_dl);
// Support for this will be re-added soon, until then verify that we
// do not promote.
ASSERT_TRUE(DisplayListsNE_Verbose(dl, expect_dl));
}
TEST_F(DisplayListTest, ClipRRectPathPromoteToClipRRect) {
@ -5142,7 +5156,7 @@ TEST_F(DisplayListTest, ClipRRectPathPromoteToClipRRect) {
expected.DrawRect(draw_rect, DlPaint());
auto expect_dl = expected.Build();
DisplayListsEQ_Verbose(dl, expect_dl);
ASSERT_TRUE(DisplayListsEQ_Verbose(dl, expect_dl));
}
TEST_F(DisplayListTest, ClipRectInversePathNoPromoteToClipRect) {
@ -5237,7 +5251,7 @@ TEST_F(DisplayListTest, ClipRectRRectPathPromoteToClipRect) {
expected.DrawRect(draw_rect, DlPaint());
auto expect_dl = expected.Build();
DisplayListsEQ_Verbose(dl, expect_dl);
ASSERT_TRUE(DisplayListsEQ_Verbose(dl, expect_dl));
}
TEST_F(DisplayListTest, ClipOvalRRectPathPromoteToClipOval) {
@ -5259,7 +5273,9 @@ TEST_F(DisplayListTest, ClipOvalRRectPathPromoteToClipOval) {
expected.DrawRect(draw_rect, DlPaint());
auto expect_dl = expected.Build();
DisplayListsEQ_Verbose(dl, expect_dl);
// Support for this will be re-added soon, until then verify that we
// do not promote.
ASSERT_TRUE(DisplayListsNE_Verbose(dl, expect_dl));
}
TEST_F(DisplayListTest, BoundedRenderOpsDoNotReportUnbounded) {

22
engine/src/flutter/testing/display_list_testing.h
View File

@ -13,20 +13,26 @@
namespace flutter {
namespace testing {
bool DisplayListsEQ_Verbose(const DisplayList* a, const DisplayList* b);
bool inline DisplayListsEQ_Verbose(const DisplayList& a, const DisplayList& b) {
[[nodiscard]] bool DisplayListsEQ_Verbose(const DisplayList* a,
const DisplayList* b);
[[nodiscard]] bool inline DisplayListsEQ_Verbose(const DisplayList& a,
const DisplayList& b) {
return DisplayListsEQ_Verbose(&a, &b);
}
bool inline DisplayListsEQ_Verbose(const sk_sp<const DisplayList>& a,
const sk_sp<const DisplayList>& b) {
[[nodiscard]] bool inline DisplayListsEQ_Verbose(
const sk_sp<const DisplayList>& a,
const sk_sp<const DisplayList>& b) {
return DisplayListsEQ_Verbose(a.get(), b.get());
}
bool DisplayListsNE_Verbose(const DisplayList* a, const DisplayList* b);
bool inline DisplayListsNE_Verbose(const DisplayList& a, const DisplayList& b) {
[[nodiscard]] bool DisplayListsNE_Verbose(const DisplayList* a,
const DisplayList* b);
[[nodiscard]] bool inline DisplayListsNE_Verbose(const DisplayList& a,
const DisplayList& b) {
return DisplayListsNE_Verbose(&a, &b);
}
bool inline DisplayListsNE_Verbose(const sk_sp<const DisplayList>& a,
const sk_sp<const DisplayList>& b) {
[[nodiscard]] bool inline DisplayListsNE_Verbose(
const sk_sp<const DisplayList>& a,
const sk_sp<const DisplayList>& b) {
return DisplayListsNE_Verbose(a.get(), b.get());
}