Flutter test cleanup #170891

kevmoo · 2025-06-19T21:38:06Z

Mostly related to error tracking.

kevmoo · 2025-06-19T21:38:46Z

packages/flutter/test/cupertino/text_field_test.dart

-        try {
-          await tester.binding.defaultBinaryMessenger.handlePlatformMessage(
+
+        await tester.binding.defaultBinaryMessenger.handlePlatformMessage(


SO MANY of these.

Just run the code. If it fails, the test will fail.

What do we get by catching and checking for a null error?

I agree with you by looking at this code. I'm trying to think of a benefit of try/catch but not seeing it.

@Renzo-Olivares do you remember why you wrote the test this way in #168678? Is it relevant that you pumpAndSettle before checking error?

One argument is that this communicates the intent of the test author that this specific code should not throw an error. If these lines are broken in the future so that they do throw an error, someone might think that the test was poorly written and update the test to work around the error, rather than realizing that the test is explicitly written to ensure that this code does not throw an error.

Maybe this problem is more elegantly solved with a simple comment though!

See my notes in the doc...

I don't think I was trying to communicate anything by using a try/catch block here. I most likely just pulled this from another test. Makes sense to remove it.

kevmoo · 2025-06-19T21:39:14Z

packages/flutter/test/widgets/dismissible_test.dart

    final dynamic exception = tester.takeException();
-    expect(exception, isNotNull);
-    expect(exception, isFlutterError);
    final FlutterError error = exception as FlutterError;


This line will throw with a helpful error. No reason to do these extra checks, right?

kevmoo · 2025-06-19T21:39:33Z

packages/flutter/test/widgets/list_view_test.dart

-    Object? error;
-    try {
-      await tester.pumpWidget(
+    await expectLater(


MUCH cleaner

Agreed, it's also more precise by using throwsAssertionError instead of isNotNull. I will use this pattern in the future if it comes up 👍 .

kevmoo · 2025-06-20T16:49:01Z

If there are reasons that these tests shouldn't be simplified, I'd love to know!

justinmc

LGTM but I also want to hear from @Renzo-Olivares in case he has any opinions.

I gave some thoughts in the comments below. One other point I wanted to make is that the cases that use takeException can affect control flow.

throw FlutterError('fail');

expect(thing, isTrue); // Never executed.

await tester.pumpWidgets(<widget tree that produces an exception>);

expect(thing, isTrue); // Still executed!

await tester.pumpWidgets(<widget tree that produces an exception>);

expect(tester.takeException(), isNull); // Fails here.

expect(thing, isTrue); // Not executed.

However, I don't think this has any notable implications on the tests that were changed in this PR.

justinmc · 2025-06-20T17:42:50Z

packages/flutter/test/cupertino/text_field_test.dart

-        try {
-          await tester.binding.defaultBinaryMessenger.handlePlatformMessage(
+
+        await tester.binding.defaultBinaryMessenger.handlePlatformMessage(


I agree with you by looking at this code. I'm trying to think of a benefit of try/catch but not seeing it.

@Renzo-Olivares do you remember why you wrote the test this way in #168678? Is it relevant that you pumpAndSettle before checking error?

justinmc · 2025-06-20T17:49:09Z

packages/flutter/test/cupertino/text_field_test.dart

-        try {
-          await tester.binding.defaultBinaryMessenger.handlePlatformMessage(
+
+        await tester.binding.defaultBinaryMessenger.handlePlatformMessage(


One argument is that this communicates the intent of the test author that this specific code should not throw an error. If these lines are broken in the future so that they do throw an error, someone might think that the test was poorly written and update the test to work around the error, rather than realizing that the test is explicitly written to ensure that this code does not throw an error.

Maybe this problem is more elegantly solved with a simple comment though!

justinmc · 2025-06-20T18:19:26Z

packages/flutter/test/widgets/list_view_test.dart

-    Object? error;
-    try {
-      await tester.pumpWidget(
+    await expectLater(


Agreed, it's also more precise by using throwsAssertionError instead of isNotNull. I will use this pattern in the future if it comes up 👍 .

justinmc

Renewing my LGTM.

The lesson here seems to be:

// Bad:
AssertionError? error;
 try {
   myTestCode();
 } on AssertionError catch (e) {
   error = e;
 }
 expect(error, isNull);

// Good:
// Regression test for <GitHub URL>.
// Should not trigger an assertion.
myTestCode();
// Or for async code:
await expectLater(myAsyncTestCode(), completes);

kevmoo · 2025-06-23T21:56:35Z

Do you want me to update things to use completes or land as-is?

justinmc · 2025-06-25T18:23:59Z

Sorry, as-is is fine 👍

Roll Flutter from d733bea to 2773c0c (42 revisions) flutter/flutter@d733bea...2773c0c 2025-06-25 jason-simmons@users.noreply.github.com Log stack traces from exceptions thrown by devicelab test tasks (flutter/flutter#171165) 2025-06-25 mdebbar@google.com Revert "Move `web_long_running_tests_{1,5}_5` to `bringup`." (flutter/flutter#171100) 2025-06-25 bruno.leroux@gmail.com Add missing M3 tests for InputDecoration.isDense (flutter/flutter#171058) 2025-06-25 34871572+gmackall@users.noreply.github.com Add Android specific sub-step to validate the Android sdk path has no spaces (flutter/flutter#170829) 2025-06-24 737941+loic-sharma@users.noreply.github.com Update foundation library to export internal (flutter/flutter#170563) 2025-06-24 matanlurey@users.noreply.github.com Remove stale references to `Release-process.md` and `conductor` (flutter/flutter#171046) 2025-06-24 30870216+gaaclarke@users.noreply.github.com License cpp jun23 (flutter/flutter#171047) 2025-06-24 magder@google.com Add android-reviewers to CODEOWNERS (flutter/flutter#170157) 2025-06-24 srawlins@google.com Update tool/README.md regarding locally-built engine (flutter/flutter#171102) 2025-06-24 mdebbar@google.com [web] Align the PR triage process with the ecosystem's triage flow (flutter/flutter#171086) 2025-06-24 danny@tuppeny.com [flutter_tool] Migrate DAP off `ProcessUtils.writelnToStdinUnsafe` (flutter/flutter#171081) 2025-06-24 mdebbar@google.com [web] More granular configuration of the test environment (flutter/flutter#168767) 2025-06-24 127918074+salemiranloye@users.noreply.github.com Clean up Devfs_Web into separate files (flutter/flutter#170769) 2025-06-24 59215665+davidhicks980@users.noreply.github.com Add RawMenuAnchor animation callbacks (flutter/flutter#167806) 2025-06-24 jason-simmons@users.noreply.github.com Support wide gamut colors when applying a DlColor to an SkPaint (flutter/flutter#170613) 2025-06-24 nshahan@google.com Remove temporary workaround for web testing (flutter/flutter#170949) 2025-06-24 engine-flutter-autoroll@skia.org Roll Packages from 02770da to d9d3191 (6 revisions) (flutter/flutter#171075) 2025-06-24 15619084+vashworth@users.noreply.github.com Add LLDB warnings (flutter/flutter#170827) 2025-06-24 bruno.leroux@gmail.com Update FormField.initialValue documentation (flutter/flutter#171061) 2025-06-24 engine-flutter-autoroll@skia.org Roll Skia from 132cb2052565 to a462e701b493 (2 revisions) (flutter/flutter#171063) 2025-06-24 engine-flutter-autoroll@skia.org Roll Skia from f88706e3a863 to 132cb2052565 (4 revisions) (flutter/flutter#171057) 2025-06-24 zeqinjie@qq.com When maintainHintSize is false, hint is centered and aligned, it is different from the original one (flutter/flutter#168654) 2025-06-24 bruno.leroux@gmail.com Deprecate DropdownButtonFormField "value" parameter in favor of "initialValue" (flutter/flutter#170805) 2025-06-24 engine-flutter-autoroll@skia.org Roll Skia from af6feb799ea6 to f88706e3a863 (2 revisions) (flutter/flutter#171056) 2025-06-24 engine-flutter-autoroll@skia.org Roll Dart SDK from aebd78999b1a to d9edd9e7a634 (1 revision) (flutter/flutter#171053) 2025-06-24 engine-flutter-autoroll@skia.org Roll Skia from ae517eba0170 to af6feb799ea6 (1 revision) (flutter/flutter#171052) 2025-06-24 engine-flutter-autoroll@skia.org Roll Skia from a7735d517e6a to ae517eba0170 (9 revisions) (flutter/flutter#171049) 2025-06-24 rmacnak@google.com Enable interpretation fallback when unable to JIT on iOS. (flutter/flutter#170835) 2025-06-24 kevmoo@users.noreply.github.com Flutter test cleanup (flutter/flutter#170891) 2025-06-24 matanlurey@users.noreply.github.com Move `packages_autoroller` out of the carcass of `conductor`, delete `conductor` (flutter/flutter#171029) 2025-06-23 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Don't strip symbols from `libapp.so` on android by default (#162464)" (flutter/flutter#171044) 2025-06-23 engine-flutter-autoroll@skia.org Roll Dart SDK from a09de0d3556c to aebd78999b1a (2 revisions) (flutter/flutter#171039) 2025-06-23 34871572+gmackall@users.noreply.github.com Don't strip symbols from `libapp.so` on android by default (flutter/flutter#162464) 2025-06-23 engine-flutter-autoroll@skia.org Roll Skia from 0311837abe86 to a7735d517e6a (12 revisions) (flutter/flutter#171037) 2025-06-23 bungeman@chromium.org Pass font scanner to font mgr that need it (flutter/flutter#170701) 2025-06-23 jacksongardner@google.com Make service worker tests more lenient. (flutter/flutter#170939) 2025-06-23 737941+loic-sharma@users.noreply.github.com Remove update CHANGELOG step from stable cherry pick process (flutter/flutter#171017) 2025-06-23 15619084+vashworth@users.noreply.github.com Include dev_dependencies in all builds for iOS and macOS (flutter/flutter#171015) 2025-06-23 matanlurey@users.noreply.github.com Move `web_long_running_tests_{1,5}_5` to `bringup`. (flutter/flutter#171026) 2025-06-23 muhatashim@google.com rename from announce to supportsAnnounce on engine (flutter/flutter#170618) 2025-06-23 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#171016) 2025-06-23 azat24680@gmail.com Enhance Text Contrast for WCAG AAA Compliance (flutter/flutter#170758) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages ...

Mostly related to error tracking.

Roll Flutter from d733bea to 2773c0c (42 revisions) flutter/flutter@d733bea...2773c0c 2025-06-25 jason-simmons@users.noreply.github.com Log stack traces from exceptions thrown by devicelab test tasks (flutter/flutter#171165) 2025-06-25 mdebbar@google.com Revert "Move `web_long_running_tests_{1,5}_5` to `bringup`." (flutter/flutter#171100) 2025-06-25 bruno.leroux@gmail.com Add missing M3 tests for InputDecoration.isDense (flutter/flutter#171058) 2025-06-25 34871572+gmackall@users.noreply.github.com Add Android specific sub-step to validate the Android sdk path has no spaces (flutter/flutter#170829) 2025-06-24 737941+loic-sharma@users.noreply.github.com Update foundation library to export internal (flutter/flutter#170563) 2025-06-24 matanlurey@users.noreply.github.com Remove stale references to `Release-process.md` and `conductor` (flutter/flutter#171046) 2025-06-24 30870216+gaaclarke@users.noreply.github.com License cpp jun23 (flutter/flutter#171047) 2025-06-24 magder@google.com Add android-reviewers to CODEOWNERS (flutter/flutter#170157) 2025-06-24 srawlins@google.com Update tool/README.md regarding locally-built engine (flutter/flutter#171102) 2025-06-24 mdebbar@google.com [web] Align the PR triage process with the ecosystem's triage flow (flutter/flutter#171086) 2025-06-24 danny@tuppeny.com [flutter_tool] Migrate DAP off `ProcessUtils.writelnToStdinUnsafe` (flutter/flutter#171081) 2025-06-24 mdebbar@google.com [web] More granular configuration of the test environment (flutter/flutter#168767) 2025-06-24 127918074+salemiranloye@users.noreply.github.com Clean up Devfs_Web into separate files (flutter/flutter#170769) 2025-06-24 59215665+davidhicks980@users.noreply.github.com Add RawMenuAnchor animation callbacks (flutter/flutter#167806) 2025-06-24 jason-simmons@users.noreply.github.com Support wide gamut colors when applying a DlColor to an SkPaint (flutter/flutter#170613) 2025-06-24 nshahan@google.com Remove temporary workaround for web testing (flutter/flutter#170949) 2025-06-24 engine-flutter-autoroll@skia.org Roll Packages from 02770da to d9d3191 (6 revisions) (flutter/flutter#171075) 2025-06-24 15619084+vashworth@users.noreply.github.com Add LLDB warnings (flutter/flutter#170827) 2025-06-24 bruno.leroux@gmail.com Update FormField.initialValue documentation (flutter/flutter#171061) 2025-06-24 engine-flutter-autoroll@skia.org Roll Skia from 132cb2052565 to a462e701b493 (2 revisions) (flutter/flutter#171063) 2025-06-24 engine-flutter-autoroll@skia.org Roll Skia from f88706e3a863 to 132cb2052565 (4 revisions) (flutter/flutter#171057) 2025-06-24 zeqinjie@qq.com When maintainHintSize is false, hint is centered and aligned, it is different from the original one (flutter/flutter#168654) 2025-06-24 bruno.leroux@gmail.com Deprecate DropdownButtonFormField "value" parameter in favor of "initialValue" (flutter/flutter#170805) 2025-06-24 engine-flutter-autoroll@skia.org Roll Skia from af6feb799ea6 to f88706e3a863 (2 revisions) (flutter/flutter#171056) 2025-06-24 engine-flutter-autoroll@skia.org Roll Dart SDK from aebd78999b1a to d9edd9e7a634 (1 revision) (flutter/flutter#171053) 2025-06-24 engine-flutter-autoroll@skia.org Roll Skia from ae517eba0170 to af6feb799ea6 (1 revision) (flutter/flutter#171052) 2025-06-24 engine-flutter-autoroll@skia.org Roll Skia from a7735d517e6a to ae517eba0170 (9 revisions) (flutter/flutter#171049) 2025-06-24 rmacnak@google.com Enable interpretation fallback when unable to JIT on iOS. (flutter/flutter#170835) 2025-06-24 kevmoo@users.noreply.github.com Flutter test cleanup (flutter/flutter#170891) 2025-06-24 matanlurey@users.noreply.github.com Move `packages_autoroller` out of the carcass of `conductor`, delete `conductor` (flutter/flutter#171029) 2025-06-23 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Don't strip symbols from `libapp.so` on android by default (#162464)" (flutter/flutter#171044) 2025-06-23 engine-flutter-autoroll@skia.org Roll Dart SDK from a09de0d3556c to aebd78999b1a (2 revisions) (flutter/flutter#171039) 2025-06-23 34871572+gmackall@users.noreply.github.com Don't strip symbols from `libapp.so` on android by default (flutter/flutter#162464) 2025-06-23 engine-flutter-autoroll@skia.org Roll Skia from 0311837abe86 to a7735d517e6a (12 revisions) (flutter/flutter#171037) 2025-06-23 bungeman@chromium.org Pass font scanner to font mgr that need it (flutter/flutter#170701) 2025-06-23 jacksongardner@google.com Make service worker tests more lenient. (flutter/flutter#170939) 2025-06-23 737941+loic-sharma@users.noreply.github.com Remove update CHANGELOG step from stable cherry pick process (flutter/flutter#171017) 2025-06-23 15619084+vashworth@users.noreply.github.com Include dev_dependencies in all builds for iOS and macOS (flutter/flutter#171015) 2025-06-23 matanlurey@users.noreply.github.com Move `web_long_running_tests_{1,5}_5` to `bringup`. (flutter/flutter#171026) 2025-06-23 muhatashim@google.com rename from announce to supportsAnnounce on engine (flutter/flutter#170618) 2025-06-23 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#171016) 2025-06-23 azat24680@gmail.com Enhance Text Contrast for WCAG AAA Compliance (flutter/flutter#170758) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages ...

Flutter test cleanup

81c82b9

Mostly related to error tracking.

kevmoo commented Jun 19, 2025

View reviewed changes

kevmoo added 2 commits June 19, 2025 16:38

one more

bc16316

format

fd3fac7

kevmoo marked this pull request as ready for review June 20, 2025 16:45

kevmoo requested a review from justinmc June 20, 2025 16:45

justinmc approved these changes Jun 20, 2025

View reviewed changes

justinmc approved these changes Jun 23, 2025

View reviewed changes

kevmoo added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 24, 2025

auto-submit bot added this pull request to the merge queue Jun 24, 2025

Merged via the queue into master with commit 8aef6e2 Jun 24, 2025
76 checks passed

auto-submit bot deleted the error_cleanup branch June 24, 2025 00:46

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 24, 2025

engine-flutter-autoroll mentioned this pull request Jun 24, 2025

Roll Flutter from d733bea58c1a to 97853d5f1fc6 (24 revisions) flutter/packages#9483

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 24, 2025

8aef6e2 Flutter test cleanup (flutter/flutter#170891)

60c4b5b

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 24, 2025

8aef6e2 Flutter test cleanup (flutter/flutter#170891)

8c21aeb

engine-flutter-autoroll mentioned this pull request Jun 24, 2025

Roll Flutter from d733bea58c1a to d49d805ea313 (25 revisions) flutter/packages#9485

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 24, 2025

8aef6e2 Flutter test cleanup (flutter/flutter#170891)

e274f8b

engine-flutter-autoroll mentioned this pull request Jun 24, 2025

Roll Flutter from d733bea58c1a to 87748a01282d (26 revisions) flutter/packages#9487

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 24, 2025

8aef6e2 Flutter test cleanup (flutter/flutter#170891)

c7f6104

engine-flutter-autoroll mentioned this pull request Jun 24, 2025

Roll Flutter from d733bea58c1a to 8c104eb053fa (27 revisions) flutter/packages#9488

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 24, 2025

8aef6e2 Flutter test cleanup (flutter/flutter#170891)

cdbe915

engine-flutter-autoroll mentioned this pull request Jun 24, 2025

Roll Flutter from d733bea58c1a to 12e4a898dcf0 (28 revisions) flutter/packages#9490

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 24, 2025

8aef6e2 Flutter test cleanup (flutter/flutter#170891)

f3b53ba

engine-flutter-autoroll mentioned this pull request Jun 24, 2025

Roll Flutter from d733bea58c1a to 5a0a9d0f9db1 (30 revisions) flutter/packages#9491

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 25, 2025

8aef6e2 Flutter test cleanup (flutter/flutter#170891)

6dd60e3

engine-flutter-autoroll mentioned this pull request Jun 25, 2025

Roll Flutter from d733bea58c1a to e9d8e93e5322 (40 revisions) flutter/packages#9493

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 25, 2025

8aef6e2 Flutter test cleanup (flutter/flutter#170891)

fa35315

This was referenced Jun 25, 2025

Roll Flutter from d733bea58c1a to 6e68383f7941 (41 revisions) flutter/packages#9494

Closed

Roll Flutter from d733bea58c1a to 2773c0c8e15e (42 revisions) flutter/packages#9495

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 25, 2025

8aef6e2 Flutter test cleanup (flutter/flutter#170891)

47988a4

mboetger pushed a commit to mboetger/flutter that referenced this pull request Jul 21, 2025

Flutter test cleanup (flutter#170891)

1c1388d

Mostly related to error tracking.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025

8aef6e2 Flutter test cleanup (flutter/flutter#170891)

4fd4d67

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025

8aef6e2 Flutter test cleanup (flutter/flutter#170891)

b38f974

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025

8aef6e2 Flutter test cleanup (flutter/flutter#170891)

75babb1

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025

8aef6e2 Flutter test cleanup (flutter/flutter#170891)

c8fd160

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2025

8aef6e2 Flutter test cleanup (flutter/flutter#170891)

ac83543

Flutter test cleanup #170891

Flutter test cleanup #170891

Uh oh!

Conversation

kevmoo commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

kevmoo commented Jun 20, 2025

Uh oh!

justinmc 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!

Choose a reason for hiding this comment

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

kevmoo commented Jun 23, 2025

Uh oh!

Uh oh!

justinmc commented Jun 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kevmoo commented Jun 19, 2025 •

edited

Loading