From 6701ce80b70ee86ba6a70ee9507d52334cbd4aa1 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Wed, 23 Apr 2025 13:44:49 -0700 Subject: [PATCH] scrolling test: Skip tests against old default z-order of slivers These two tests on CustomPaintOrderScrollView compare its behavior to the default z-order / paint order behavior of CustomScrollView. They'd therefore be broken by my upstream PR that gives CustomScrollView the functionality of CustomPaintOrderScrollView and simplifies its default behavior: https://github.com/flutter/flutter/pull/164818 So skip the tests. Once that PR lands, we can update past it and then remove CustomPaintOrderScrollView and its tests entirely. (The customer_tests failures currently seen on the upstream PR are different, because the Zulip pin in flutter/tests is currently at d302584e5. The failures there are in the sticky_header tests, which want to exercise a variety of sliver configurations and need to control the z-order when they do so. As of d302584e5, they did so by relying on the specifics of the existing CustomScrollView behavior. Since then we merged CustomPaintOrderScrollView, and in 49eff8bb3 adapted those tests to use it, making them immune to changes in the default z-order; but that also meant adding CustomPaintOrderScrollView's own tests, including these two.) --- test/widgets/scrolling_test.dart | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/test/widgets/scrolling_test.dart b/test/widgets/scrolling_test.dart index bfb010ccf0..2fb606d1dd 100644 --- a/test/widgets/scrolling_test.dart +++ b/test/widgets/scrolling_test.dart @@ -43,12 +43,6 @@ void main() { check(paintLog).deepEquals([0, 1, 2, 3, 4]); }); - // This test will fail if a corresponding upstream PR lands: - // https://github.com/flutter/flutter/pull/164818 - // because that eliminates the quirky centerTopFirstBottom behavior. - // In that case, skip this test for a quick fix; or go ahead and - // rip out CustomPaintOrderScrollView in favor of CustomScrollView. - // (Greg has a draft commit ready which does the latter.) testWidgets('centerTopFirstBottom', (tester) async { addTearDown(paintLog.clear); await tester.pumpWidget(Directionality(textDirection: TextDirection.ltr, @@ -68,7 +62,10 @@ void main() { center: ValueKey(2), anchor: 0.5, slivers: List.generate(5, makeSliver)))); check(paintLog).deepEquals(result); - }); + }, skip: true, // TODO(upstream): once PR 164818 lands, cut our CustomPaintOrderScrollView + // in favor of CustomScrollView; this test was checking that + // the former matched the latter's old default behavior. + ); }); group('CustomPaintOrderScrollView hit-test order', () { @@ -104,9 +101,6 @@ void main() { check(sliverIds(result.path)).deepEquals([4, 3, 2, 1, 0]); }); - // This test will fail if the upstream PR 164818 lands. - // In that case the test is no longer needed and we'll take it out; - // see comment on other centerTopFirstBottom test above. testWidgets('centerTopFirstBottom', (tester) async { await tester.pumpWidget(Directionality(textDirection: TextDirection.ltr, child: CustomPaintOrderScrollView( @@ -125,7 +119,10 @@ void main() { slivers: List.generate(5, makeSliver)))); check(sliverIds(tester.hitTestOnBinding(const Offset(400, 300)).path)) .deepEquals(sliverIds(result.path)); - }); + }, skip: true, // TODO(upstream): once PR 164818 lands, cut our CustomPaintOrderScrollView + // in favor of CustomScrollView; this test was checking that + // the former matched the latter's old default behavior. + ); }); }