Fix bug in test_golden_comparator, add an e2e test. by matanlurey · Pull Request #174459 · flutter/flutter

matanlurey · 2025-08-26T19:18:01Z

The actual fix is 1-line, but of course there is no test suite to verify that, so that took the bulk of the time.

gemini-code-assist

Code Review

This pull request addresses a bug in TestGoldenComparator that caused crashes on golden test failures. The fix involves broadening the exception handling in test_golden_comparator.dart from Exception to Object? to catch any thrown object. Additionally, a new integration test, test_golden_comparator_test.dart, has been added with tests for both successful and failed comparison scenarios to verify the fix. My review includes a suggestion to refactor the new test to reduce code duplication for better maintainability.

packages/flutter_tools/test/integration.shard/test_golden_comparator_test.dart

bkonyi

LGTM overall!

bkonyi · 2025-08-26T19:53:51Z

packages/flutter_tools/test/integration.shard/test_golden_comparator_test.dart

+    printOnFailure(logger.errorText);
+  });
+
+  // Cannot be in 'setUp' because it implicitly uses [global] state that comes from 'testUsingContext'.


There's calls to global in this function, so it's hardly implicit 😜

Ack. Removed :)

bkonyi · 2025-08-26T19:54:52Z

packages/flutter_tools/test/integration.shard/test_golden_comparator_test.dart

+    expect(result, isA<TestGoldenComparisonDone>());
+  });
+
+  testUsingContext('failed comparison is failed but not crashed', () async {


Consider leaving a breadcrumb to point to the original issue this regression test was written for.

bkonyi · 2025-08-26T19:55:44Z

packages/flutter_tools/lib/src/test/test_golden_comparator.dart

          print(jsonEncode({'success': success}));
-        } on Exception catch (ex) {
-          print(jsonEncode({'success': false, 'message': '\$ex'}));
+        } on Object? catch (e) {


Out of curiosity, what's being thrown that wasn't caught by Exception?

FlutterError

auto-submit · 2025-08-26T21:01:00Z

autosubmit label was removed for flutter/flutter/174459, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

auto-submit · 2025-08-26T21:36:36Z

autosubmit label was removed for flutter/flutter/174459, because - The status or check suite Windows plugin_test has failed. Please fix the issues identified (or deflake) before re-applying this label.

Closes flutter#174267. The actual fix is 1-line, but of course there is no test suite to verify that, so that took the bulk of the time.

…flutter#174459)

Closes flutter#174267. The actual fix is 1-line, but of course there is no test suite to verify that, so that took the bulk of the time.

Manual CP for #174459.

flutter/flutter@c65f01d...da5523a 2025-08-27 [email protected] [native assets] Roll dependencies (flutter/flutter#174522) 2025-08-27 [email protected] Roll Skia from 8cf2faada2b5 to 7e6da45059c5 (5 revisions) (flutter/flutter#174523) 2025-08-27 [email protected] [Android] Restrict AOT shared library engine flag to trusted paths (flutter/flutter#173359) 2025-08-27 [email protected] Roll Fuchsia Linux SDK from L5zGzsIWIS8N36AFQ... to bHYRvLv2Dg56RWZF2... (flutter/flutter#174518) 2025-08-27 [email protected] Roll Packages from 1ef712e to 86fbeec (7 revisions) (flutter/flutter#174521) 2025-08-27 [email protected] Roll Skia from 448a0d0e57e3 to 8cf2faada2b5 (1 revision) (flutter/flutter#174510) 2025-08-27 [email protected] Roll Skia from 4703976a4dae to 448a0d0e57e3 (3 revisions) (flutter/flutter#174494) 2025-08-27 [email protected] Fix SliverMainAxisGroup and SliverCrossAxisGroup gestures' local positions. (flutter/flutter#174265) 2025-08-27 [email protected] Fix lock up when window resized with merged UI and platform threads. (flutter/flutter#172893) 2025-08-27 [email protected] Roll Skia from dc2872de506f to 4703976a4dae (1 revision) (flutter/flutter#174489) 2025-08-27 [email protected] Roll Dart SDK from db45c0ce46f9 to 89023922f96d (2 revisions) (flutter/flutter#174481) 2025-08-27 [email protected] Migrate examples and defaults to `WidgetState` (flutter/flutter#174421) 2025-08-27 [email protected] Roll Skia from 8e48b9e6d67b to dc2872de506f (7 revisions) (flutter/flutter#174479) 2025-08-27 [email protected] SnackBar with action no longer auto-dismiss (flutter/flutter#173084) 2025-08-26 [email protected] Roll Dart SDK from 9054cd8af73c to db45c0ce46f9 (1 revision) (flutter/flutter#174438) 2025-08-26 [email protected] Roll Skia from f941bfab7c09 to 8e48b9e6d67b (4 revisions) (flutter/flutter#174468) 2025-08-26 [email protected] Fix bug in test_golden_comparator, add an e2e test. (flutter/flutter#174459) 2025-08-26 [email protected] fix typo in test_profile/README.md (flutter/flutter#174384) 2025-08-26 [email protected] Remove CP labels on not-merged PRs, and explain why (flutter/flutter#174448) 2025-08-26 [email protected] Increase testing coverage and maintainability of android manifest parsing logic (flutter/flutter#174070) 2025-08-26 [email protected] [web] Add test that pictures are not rasterized when clipped out (flutter/flutter#174452) 2025-08-26 [email protected] Roll Skia from 538260c45b4a to f941bfab7c09 (3 revisions) (flutter/flutter#174450) 2025-08-26 [email protected] fixes the vulkan image layout transitions for mipmap generation (flutter/flutter#173884) 2025-08-26 [email protected] Move flakey iOS tests to bringup (flutter/flutter#174446) 2025-08-26 [email protected] [Impeller] Make sure inline passes always do a clear action. (flutter/flutter#174083) 2025-08-26 [email protected] Roll Skia from 21214d63fc40 to 538260c45b4a (2 revisions) (flutter/flutter#174441) 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 Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…flutter#174459)

Closes flutter#174267. The actual fix is 1-line, but of course there is no test suite to verify that, so that took the bulk of the time.

…flutter#174459)

Closes flutter#174267. The actual fix is 1-line, but of course there is no test suite to verify that, so that took the bulk of the time.

Fix bug in test_golden_comparator, add an e2e test.

6eea141

matanlurey requested a review from bkonyi August 26, 2025 19:18

github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Aug 26, 2025

gemini-code-assist bot reviewed Aug 26, 2025

View reviewed changes

packages/flutter_tools/test/integration.shard/test_golden_comparator_test.dart Outdated Show resolved Hide resolved

Refactor test

0912494

bkonyi approved these changes Aug 26, 2025

View reviewed changes

++

8ab3941

matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 26, 2025

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 26, 2025

matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 26, 2025

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 26, 2025

matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 26, 2025

auto-submit bot added this pull request to the merge queue Aug 26, 2025

Merged via the queue into flutter:master with commit 6f26959 Aug 26, 2025
149 checks passed

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

matanlurey added cp: beta cherry pick this pull request to beta release candidate branch cp: stable cherry pick this pull request to stable release candidate branch labels Aug 26, 2025

engine-flutter-autoroll mentioned this pull request Aug 27, 2025

Roll Flutter from c65f01dc373c to 562ff056cb11 (21 revisions) flutter/packages#9896

Closed

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

6f26959 Fix bug in test_golden_comparator, add an e2e test. (flutter/…

1f7b8b4

…flutter#174459)

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

6f26959 Fix bug in test_golden_comparator, add an e2e test. (flutter/…

f6649b8

…flutter#174459)

engine-flutter-autoroll mentioned this pull request Aug 27, 2025

Roll Flutter from c65f01dc373c to 31085d38c0c8 (22 revisions) flutter/packages#9899

Closed

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

6f26959 Fix bug in test_golden_comparator, add an e2e test. (flutter/…

59bc263

…flutter#174459)

engine-flutter-autoroll mentioned this pull request Aug 27, 2025

Roll Flutter from c65f01dc373c to e1315d392b10 (23 revisions) flutter/packages#9900

Closed

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

6f26959 Fix bug in test_golden_comparator, add an e2e test. (flutter/…

da5bf3e

…flutter#174459)

engine-flutter-autoroll mentioned this pull request Aug 27, 2025

Roll Flutter from c65f01dc373c to daadbba85611 (24 revisions) flutter/packages#9901

Closed

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

6f26959 Fix bug in test_golden_comparator, add an e2e test. (flutter/…

b3b4d65

…flutter#174459)

engine-flutter-autoroll mentioned this pull request Aug 27, 2025

Roll Flutter from c65f01dc373c to 1a5ec4ddacac (25 revisions) flutter/packages#9902

Closed

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

6f26959 Fix bug in test_golden_comparator, add an e2e test. (flutter/…

ff6afdb

…flutter#174459)

engine-flutter-autoroll mentioned this pull request Aug 27, 2025

Roll Flutter from c65f01dc373c to da5523afc3c1 (26 revisions) flutter/packages#9903

Merged

matanlurey added cp: stable cherry pick this pull request to stable release candidate branch and removed cp: beta cherry pick this pull request to beta release candidate branch cp: stable cherry pick this pull request to stable release candidate branch labels Aug 27, 2025

matanlurey mentioned this pull request Aug 27, 2025

[3.35] CP #174459 (Fix bug in test_golden_comparator) #174602

Merged

matanlurey mentioned this pull request Aug 27, 2025

[3.36] CP #174459 (Fix bug in test_golden_comparator) #174603

Merged

auto-submit bot pushed a commit that referenced this pull request Aug 27, 2025

[3.35] CP #174459 (Fix bug in test_golden_comparator) (#174602)

01586a6

Manual CP for #174459.

auto-submit bot pushed a commit that referenced this pull request Aug 27, 2025

[3.36] CP #174459 (Fix bug in test_golden_comparator) (#174603)

6c79484

Manual CP for #174459.

engine-flutter-autoroll mentioned this pull request Aug 28, 2025

Roll Flutter from c65f01dc373c to 0628f86a780d (41 revisions) flutter/packages#9918

Closed

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

6f26959 Fix bug in test_golden_comparator, add an e2e test. (flutter/…

069d715

…flutter#174459)

reidbaker mentioned this pull request Oct 1, 2025

[CP] Flutter analyze --suggestions update for 3.35 #176359

Merged

9 tasks

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

6f26959 Fix bug in test_golden_comparator, add an e2e test. (flutter/…

9de3016

…flutter#174459)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix bug in test_golden_comparator, add an e2e test.#174459

Fix bug in test_golden_comparator, add an e2e test.#174459
auto-submit[bot] merged 3 commits intoflutter:masterfrom
matanlurey:fix-test_golden_comparator-add_test

matanlurey commented Aug 26, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

bkonyi left a comment

Uh oh!

bkonyi Aug 26, 2025

Uh oh!

matanlurey Aug 26, 2025

Uh oh!

bkonyi Aug 26, 2025

Uh oh!

matanlurey Aug 26, 2025

Uh oh!

bkonyi Aug 26, 2025

Uh oh!

matanlurey Aug 26, 2025

Uh oh!

auto-submit bot commented Aug 26, 2025

Uh oh!

auto-submit bot commented Aug 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

matanlurey commented Aug 26, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

bkonyi left a comment

Choose a reason for hiding this comment

Uh oh!

bkonyi Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

matanlurey Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

bkonyi Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

matanlurey Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

bkonyi Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

matanlurey Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Aug 26, 2025

Uh oh!

auto-submit bot commented Aug 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants