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 [email protected] Enhances intuitiveness of RawMagnifier's example (flutter/flutter#150308) 2024-07-19 [email protected] Roll pub packages (flutter/flutter#151992) 2024-07-19 [email protected] Add test for scrollbar.1.dart (flutter/flutter#151463) 2024-07-19 [email protected] Roll Flutter Engine from ea1e53a4e810 to 969fb7abc449 (3 revisions) (flutter/flutter#152018) 2024-07-19 [email protected] docimports for rendering library (flutter/flutter#151958) 2024-07-19 [email protected] Roll Flutter Engine from b65c93ea948e to ea1e53a4e810 (2 revisions) (flutter/flutter#152012) 2024-07-19 [email protected] painting: drop deprecated (exported) hashList and hashValues functions (flutter/flutter#151677) 2024-07-18 [email protected] Roll Flutter Engine from 766f7bed7185 to b65c93ea948e (2 revisions) (flutter/flutter#152004) 2024-07-18 [email protected] Update TESTOWNERS (flutter/flutter#151907) 2024-07-18 [email protected] chore: fix test name & add description of tests (flutter/flutter#151959) 2024-07-18 [email protected] Roll Flutter Engine from 564ded4c4742 to 766f7bed7185 (2 revisions) (flutter/flutter#151998) 2024-07-18 [email protected] Fix SelectionArea scrolling conflicts (flutter/flutter#151138) 2024-07-18 [email protected] Fix: BaseTapAndDragGestureRecognizer should reset drag state after losing gesture arena (flutter/flutter#151989) 2024-07-18 [email protected] Roll pub packages (flutter/flutter#151975) 2024-07-18 [email protected] Roll Flutter Engine from 8bcf638eb893 to 564ded4c4742 (2 revisions) (flutter/flutter#151986) 2024-07-18 [email protected] Fix WidgetStateTextStyle's doc (flutter/flutter#151935) 2024-07-18 [email protected] Roll Flutter Engine from d58ba74250ce to 8bcf638eb893 (2 revisions) (flutter/flutter#151977) 2024-07-18 [email protected] Adds 3.22.3 changelog (flutter/flutter#151974) 2024-07-18 [email protected] Roll Packages from d03b1b4 to c7f0526 (8 revisions) (flutter/flutter#151971) 2024-07-18 [email protected] `WidgetState` mapping (flutter/flutter#146043) 2024-07-18 [email protected] Fix AppBar doc to keep diagram next to its description (flutter/flutter#151937) 2024-07-18 [email protected] Small fixes to Image docs: NNBD, and add a cross-reference (flutter/flutter#151938) 2024-07-18 [email protected] Roll Flutter Engine from b043fe447bb3 to d58ba74250ce (1 revision) (flutter/flutter#151964) 2024-07-18 [email protected] Roll pub packages (flutter/flutter#151946) 2024-07-18 [email protected] Roll pub packages (flutter/flutter#151904) 2024-07-18 [email protected] Roll Flutter Engine from e3abca2d8105 to b043fe447bb3 (1 revision) (flutter/flutter#151942) 2024-07-18 [email protected] Roll Flutter Engine from 8073523b4623 to e3abca2d8105 (1 revision) (flutter/flutter#151936) 2024-07-18 [email protected] Roll Flutter Engine from dfe22e3acc19 to 8073523b4623 (1 revision) (flutter/flutter#151934) 2024-07-18 [email protected] Roll Flutter Engine from 184c3f0de6b3 to dfe22e3acc19 (1 revision) (flutter/flutter#151930) 2024-07-18 [email protected] Roll Flutter Engine from 00f0f6b74da7 to 184c3f0de6b3 (1 revision) (flutter/flutter#151928) 2024-07-18 [email protected] Roll Flutter Engine from d194a2f0e5da to 00f0f6b74da7 (1 revision) (flutter/flutter#151927) 2024-07-18 [email protected] Roll Flutter Engine from a5a93bb80bd1 to d194a2f0e5da (3 revisions) (flutter/flutter#151925) 2024-07-17 [email protected] Roll Flutter Engine from e9dc62074c2b to a5a93bb80bd1 (1 revision) (flutter/flutter#151918) 2024-07-17 [email protected] [web] use the new backlog Github project in triage links (flutter/flutter#151920) 2024-07-17 [email protected] Update Flutter-Web-Triage.md (flutter/flutter#151607) 2024-07-17 [email protected] Reland fix InputDecorator hint default text style on M3 (flutter/flutter#150835) 2024-07-17 [email protected] Roll Flutter Engine from 39ee1a549581 to e9dc62074c2b (3 revisions) (flutter/flutter#151915) 2024-07-17 [email protected] Constrain `CupertinoContextMenu` animation to safe area (flutter/flutter#151860) 2024-07-17 [email protected] Create `CarouselView` widget - Part 2 (flutter/flutter#149775) 2024-07-17 [email protected] Roll Flutter Engine from 45b722b661f0 to 39ee1a549581 (3 revisions) (flutter/flutter#151905) 2024-07-17 [email protected] docs: Fix typo in data driven fixes test folder section (flutter/flutter#151836) 2024-07-17 [email protected] Stop running flaky mac tests in presubmit (flutter/flutter#151870) 2024-07-17 [email protected] Roll Flutter Engine from 7e2579634027 to 45b722b661f0 (1 revision) (flutter/flutter#151895) 2024-07-17 [email protected] fix(Flutter Web App): fixes html lang typo (flutter/flutter#151866) 2024-07-17 [email protected] Delete `docs/engine` directory (flutter/flutter#151616) 2024-07-17 [email protected] 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

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