Add test for scrollbar.1.dart #151463

ValentinVignal · 2024-07-09T09:54:35Z

Contributes to #130459

It adds a test for

examples/api/lib/material/scrollbar/scrollbar.1.dart

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

ValentinVignal · 2024-07-09T09:57:48Z

I'm not really happy with this test, it doesn't verify the behavior the code sample is made for (displaying the scrollbar thumb all the time).
The thumb is drawn on the screen by the RawScrollbar which uses a custom patiner. Is there a way to verify that ? Should I do a golden? I was hoping to get some help from https://github.com/flutter/flutter/blob/master/examples/api/test/material/scrollbar/scrollbar.0_test.dart (the code sample is made to showcase the thumb fading in/out when the user starts/stops to scroll). But the test doesn't do much

Do you have any suggestions?

bleroux · 2024-07-10T07:51:22Z

I'm not really happy with this test, it doesn't verify the behavior the code sample is made for (displaying the scrollbar thumb all the time). The thumb is drawn on the screen by the RawScrollbar which uses a custom patiner. Is there a way to verify that ? Should I do a golden?

Golden are somewhat opaque, when possible it is way better to use mock canvas as it makes very explicit what is expected to be drawn.
For instance a basic test which checks that the Scrollbar was drawn could be:

  testWidgets('Scrollbar appears on drag', (WidgetTester tester) async {
    await tester.pumpWidget(
      const example.ScrollbarExampleApp(),
    );

    final Finder scrollbarFinder = find.byType(Scrollbar).last;

    expect(scrollbarFinder, isNot(paints..rect()));
    await tester.fling(scrollbarFinder.last, const Offset(0, -300), 10.0);
    expect(scrollbarFinder.last, paints..rect());
  });

This is inspired by https://github.com/flutter/flutter/blob/master/packages/flutter/test/material/scrollbar_paint_test.dart

I'm not a Scrollbar testing expert 😄 , so here are two things I noticed while writing this test:

With tester.drag (followed by a pump), the Scrollbar is not painted (there might be a way to get it works). With tester.fling which pumps frames during the gesture it is ok.
For both tester.drag and tester.fling, offset.dy should be negative to scroll down (this is not properly documented as far as I can tell and seems counterintuitive).

Here is a second test, that checks which items are visible before and after the scroll down:

  testWidgets('Scrollbar scrolls the items', (WidgetTester tester) async {
    await tester.pumpWidget(
      const example.ScrollbarExampleApp(),
    );

    expect(find.text('item 0'), findsOne);
    expect(find.text('item 9'), findsNothing);

    await tester.fling(find.byType(Scrollbar).last, const Offset(0, -300), 10.0);

    expect(find.text('item 0'), findsNothing);
    expect(find.text('item 9'), findsOne);
  });

ValentinVignal · 2024-07-12T14:35:22Z

Oh wow! Thank you a lot for that huge help @bleroux ! I didn't know about paints and mock canvas and all 👍

I implemented the test in test: Verify scrollbar paints all even without user interaction.

I also took the opportunity to make examples/api/test/material/scrollbar/scrollbar.0_test.dart stronger in test: Verify scrollbar paints only after the user interaction

ValentinVignal · 2024-07-12T14:37:24Z

examples/api/test/material/scrollbar/scrollbar.1_test.dart

+    await tester.pumpWidget(
+      const example.ScrollbarExampleApp(),
+    );
+    await tester.pumpAndSettle(); // Waits for all the paints to be done.


I noticed I need a pumpAndSettle() here, several pumps were not doing the trick.

I guess this is because the scrollbar is not painted in the 1st frame as it needs to get the extent of the list. Is there something better to do to make the test pass here?

It seems that the following would be ok (Maybe it is clearer than pumpAndSettle with a comment such as the one I added).

await tester.pump(); await tester.pump(const Duration(milliseconds: 10)); // Wait for the thumb to start appearing.

Sure, I made the change in test: Remove empty line and use pumps

bleroux · 2024-07-12T15:34:52Z

examples/api/test/material/scrollbar/scrollbar.0_test.dart

  }, variant: TargetPlatformVariant.all());
+
+  testWidgets('The scrollbar should be painted when the user scrolls', (WidgetTester tester) async {
+


Consider removing this extra line

Sorry for that again, I removed it in test: Remove empty line and use pumps

bleroux

LGTM! Thanks!

TahaTesser · 2024-07-15T07:22:23Z

examples/api/test/material/scrollbar/scrollbar.0_test.dart

+    final Finder scrollbarFinder = find.byType(Scrollbar).last;
+
+    expect(find.text('item 0'), findsOne);
+    expect(find.text('item 9'), findsNothing);
+    expect(scrollbarFinder, isNot(paints..rect()));
+
+    await tester.fling(find.byType(Scrollbar).last, const Offset(0, -300), 10.0);
+
+    expect(find.text('item 0'), findsNothing);
+    expect(find.text('item 9'), findsOne);
+    expect(scrollbarFinder.last, paints..rect());


Here you're using scrollbarFinder but not in the second test.

We should be consistent, i would suggest to not use scrollbarFinder as find.byType(Scrollbar).last is already simple enough.

Sure, I deleted it in test: Remove unnecessary scrollbarFinder

TahaTesser

LGTM!

Roll Flutter from 58068d8 to 7d5c1c0 (104 revisions) flutter/flutter@58068d8...7d5c1c0 2024-07-19 104349824+huycozy@users.noreply.github.com Enhances intuitiveness of RawMagnifier's example (flutter/flutter#150308) 2024-07-19 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#151992) 2024-07-19 32538273+ValentinVignal@users.noreply.github.com Add test for scrollbar.1.dart (flutter/flutter#151463) 2024-07-19 engine-flutter-autoroll@skia.org Roll Flutter Engine from ea1e53a4e810 to 969fb7abc449 (3 revisions) (flutter/flutter#152018) 2024-07-19 goderbauer@google.com docimports for rendering library (flutter/flutter#151958) 2024-07-19 engine-flutter-autoroll@skia.org Roll Flutter Engine from b65c93ea948e to ea1e53a4e810 (2 revisions) (flutter/flutter#152012) 2024-07-19 kevmoo@users.noreply.github.com painting: drop deprecated (exported) hashList and hashValues functions (flutter/flutter#151677) 2024-07-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 766f7bed7185 to b65c93ea948e (2 revisions) (flutter/flutter#152004) 2024-07-18 magder@google.com Update TESTOWNERS (flutter/flutter#151907) 2024-07-18 n7484443@naver.com chore: fix test name & add description of tests (flutter/flutter#151959) 2024-07-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 564ded4c4742 to 766f7bed7185 (2 revisions) (flutter/flutter#151998) 2024-07-18 rmolivares@renzo-olivares.dev Fix SelectionArea scrolling conflicts (flutter/flutter#151138) 2024-07-18 rmolivares@renzo-olivares.dev Fix: BaseTapAndDragGestureRecognizer should reset drag state after losing gesture arena (flutter/flutter#151989) 2024-07-18 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#151975) 2024-07-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 8bcf638eb893 to 564ded4c4742 (2 revisions) (flutter/flutter#151986) 2024-07-18 88094492+croro613@users.noreply.github.com Fix WidgetStateTextStyle's doc (flutter/flutter#151935) 2024-07-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from d58ba74250ce to 8bcf638eb893 (2 revisions) (flutter/flutter#151977) 2024-07-18 kevinjchisholm@google.com Adds 3.22.3 changelog (flutter/flutter#151974) 2024-07-18 engine-flutter-autoroll@skia.org Roll Packages from d03b1b4 to c7f0526 (8 revisions) (flutter/flutter#151971) 2024-07-18 nate.w5687@gmail.com `WidgetState` mapping (flutter/flutter#146043) 2024-07-18 greg@zulip.com Fix AppBar doc to keep diagram next to its description (flutter/flutter#151937) 2024-07-18 greg@zulip.com Small fixes to Image docs: NNBD, and add a cross-reference (flutter/flutter#151938) 2024-07-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from b043fe447bb3 to d58ba74250ce (1 revision) (flutter/flutter#151964) 2024-07-18 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#151946) 2024-07-18 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#151904) 2024-07-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from e3abca2d8105 to b043fe447bb3 (1 revision) (flutter/flutter#151942) 2024-07-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 8073523b4623 to e3abca2d8105 (1 revision) (flutter/flutter#151936) 2024-07-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from dfe22e3acc19 to 8073523b4623 (1 revision) (flutter/flutter#151934) 2024-07-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 184c3f0de6b3 to dfe22e3acc19 (1 revision) (flutter/flutter#151930) 2024-07-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 00f0f6b74da7 to 184c3f0de6b3 (1 revision) (flutter/flutter#151928) 2024-07-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from d194a2f0e5da to 00f0f6b74da7 (1 revision) (flutter/flutter#151927) 2024-07-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from a5a93bb80bd1 to d194a2f0e5da (3 revisions) (flutter/flutter#151925) 2024-07-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from e9dc62074c2b to a5a93bb80bd1 (1 revision) (flutter/flutter#151918) 2024-07-17 yjbanov@google.com [web] use the new backlog Github project in triage links (flutter/flutter#151920) 2024-07-17 yjbanov@google.com Update Flutter-Web-Triage.md (flutter/flutter#151607) 2024-07-17 leroux_bruno@yahoo.fr Reland fix InputDecorator hint default text style on M3 (flutter/flutter#150835) 2024-07-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from 39ee1a549581 to e9dc62074c2b (3 revisions) (flutter/flutter#151915) 2024-07-17 victorsanniay@gmail.com Constrain `CupertinoContextMenu` animation to safe area (flutter/flutter#151860) 2024-07-17 36861262+QuncCccccc@users.noreply.github.com Create `CarouselView` widget - Part 2 (flutter/flutter#149775) 2024-07-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from 45b722b661f0 to 39ee1a549581 (3 revisions) (flutter/flutter#151905) 2024-07-17 34465683+rkishan516@users.noreply.github.com docs: Fix typo in data driven fixes test folder section (flutter/flutter#151836) 2024-07-17 zanderso@users.noreply.github.com Stop running flaky mac tests in presubmit (flutter/flutter#151870) 2024-07-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7e2579634027 to 45b722b661f0 (1 revision) (flutter/flutter#151895) 2024-07-17 44146839+essjay05@users.noreply.github.com fix(Flutter Web App): fixes html lang typo (flutter/flutter#151866) 2024-07-17 matanlurey@users.noreply.github.com Delete `docs/engine` directory (flutter/flutter#151616) 2024-07-17 31859944+LongCatIsLooong@users.noreply.github.com Make `CupertinoSlidingSegmentedControl` type parameter non-null (flutter/flutter#151803) ...

Contributes to flutter#130459 It adds a test for - `examples/api/lib/material/scrollbar/scrollbar.1.dart`

test: Add test for scrollbar.1.dart

c5c572a

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: scrolling Viewports, list views, slivers, etc. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos labels Jul 9, 2024

goderbauer requested review from TahaTesser and bleroux July 9, 2024 22:05

ValentinVignal added 2 commits July 12, 2024 22:33

test: Verify scrollbar paints all even without user interaction

7e847f1

test: Verify scrollbar paints only after the user interaction

6628a28

test: Remove useless pumps

f448eb6

ValentinVignal commented Jul 12, 2024

View reviewed changes

bleroux reviewed Jul 12, 2024

View reviewed changes

test: Remove empty line and use pumps

627803e

ValentinVignal requested a review from bleroux July 12, 2024 16:26

bleroux approved these changes Jul 12, 2024

View reviewed changes

TahaTesser requested changes Jul 15, 2024

View reviewed changes

test: Remove unnecessary scrollbarFinder

811773e

ValentinVignal requested a review from TahaTesser July 15, 2024 10:57

TahaTesser approved these changes Jul 15, 2024

View reviewed changes

ValentinVignal added 2 commits July 16, 2024 09:20

Merge branch 'master' into flutter/api/example/Add-test-for-scrollbar-1

1f3c8bc

Merge branch 'master' into flutter/api/example/Add-test-for-scrollbar-1

6c5698b

TahaTesser added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 19, 2024

auto-submit bot merged commit 879fa83 into flutter:master Jul 19, 2024

engine-flutter-autoroll mentioned this pull request Jul 19, 2024

Roll Flutter from 58068d8ea4ec to 7d5c1c04fb56 (104 revisions) flutter/packages#7176

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 19, 2024

879fa83 Add test for scrollbar.1.dart (flutter/flutter#151463)

a70d09b

engine-flutter-autoroll mentioned this pull request Jul 19, 2024

Roll Flutter from 58068d8ea4ec to 7d5c1c04fb56 (104 revisions) flutter/packages#7177

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 19, 2024

879fa83 Add test for scrollbar.1.dart (flutter/flutter#151463)

efed905

engine-flutter-autoroll mentioned this pull request Jul 19, 2024

Roll Flutter from 58068d8ea4ec to 49a315284d11 (105 revisions) flutter/packages#7178

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 19, 2024

879fa83 Add test for scrollbar.1.dart (flutter/flutter#151463)

f2c4974

engine-flutter-autoroll mentioned this pull request Jul 19, 2024

Roll Flutter from 58068d8ea4ec to 20242d0c4142 (107 revisions) flutter/packages#7179

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 19, 2024

879fa83 Add test for scrollbar.1.dart (flutter/flutter#151463)

034c493

TytaniumDev pushed a commit to TytaniumDev/flutter that referenced this pull request Aug 7, 2024

Add test for scrollbar.1.dart (flutter#151463)

f0b4406

Contributes to flutter#130459 It adds a test for - `examples/api/lib/material/scrollbar/scrollbar.1.dart`

Buchimi pushed a commit to Buchimi/flutter that referenced this pull request Sep 2, 2024

Add test for scrollbar.1.dart (flutter#151463)

06dd5f1

Contributes to flutter#130459 It adds a test for - `examples/api/lib/material/scrollbar/scrollbar.1.dart`

		}, variant: TargetPlatformVariant.all());

		testWidgets('The scrollbar should be painted when the user scrolls', (WidgetTester tester) async {

Add test for scrollbar.1.dart #151463

Add test for scrollbar.1.dart #151463

Uh oh!

Conversation

ValentinVignal commented Jul 9, 2024

Pre-launch Checklist

Uh oh!

ValentinVignal commented Jul 9, 2024

Uh oh!

bleroux commented Jul 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ValentinVignal commented Jul 12, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bleroux left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TahaTesser left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bleroux commented Jul 10, 2024 •

edited

Loading