fix test `chrome.close can recover if getTab throws a StateError` #154889

andrewkolos · 2024-09-10T00:08:12Z

Does so by:

adding await chromiumLauncher.connect(chrome, false); before the close call to make sure we enter the block that actually tries to close chromium
adding an onGetTab callback to FakeChromeConnectionWithTab, which the test now uses to throw a StateError upon getTab getting called.

How I verified this change

Change Chromium.close from using the safer getChromeTabGuarded function to using the previous method of calling ChromeConnection.getTab directly. Do so by applying this diff:

diff --git a/packages/flutter_tools/lib/src/web/chrome.dart b/packages/flutter_tools/lib/src/web/chrome.dart
index c9a5fdab81..81bc246ff9 100644
--- a/packages/flutter_tools/lib/src/web/chrome.dart
+++ b/packages/flutter_tools/lib/src/web/chrome.dart
@@ -520,7 +520,7 @@ class Chromium {
     Duration sigtermDelay = Duration.zero;
     if (_hasValidChromeConnection) {
       try {
-        final ChromeTab? tab = await getChromeTabGuarded(chromeConnection,
+        final ChromeTab? tab = await chromeConnection.getTab(
             (_) => true, retryFor: const Duration(seconds: 1));
         if (tab != null) {
           final WipConnection wipConnection = await tab.connect();

Then, run the test, which should correctly fail:

dart test test/web.shard/chrome_test.dart --name="chrome.close can recover if getTab throws a StateError"`

Revert the change from step 1 and run again. The test should now pass.

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.

christopherfujino

LGTM

andrewkolos · 2024-09-10T17:23:56Z

Just an FYI: Looks like I made a typo in this description, the numbered list was meant to be a list of steps to verify this change, but I forgot to label it as such. Sorry for any confusion there

…ror` (flutter/flutter#154889)

Roll Flutter from 2e221e7 to 303f222 (77 revisions) flutter/flutter@2e221e7...303f222 2024-09-12 [email protected] Manual roll to 48ddaf578fb0c8326d5b4b680b0f49ea72e33216 (flutter/flutter#155070) 2024-09-12 [email protected] Externalize and update onboarding instructions (flutter/flutter#154730) 2024-09-12 [email protected] when setting up the log reader for a device during `flutter run`, discard any `RPCError` thrown due to the device being disconnected (flutter/flutter#155049) 2024-09-12 98614782+auto-submit[bot]@users.noreply.github.com Reverts "iOS: update provisioning profile for 2024-2025 cert (#155052)" (flutter/flutter#155059) 2024-09-12 [email protected] iOS: update provisioning profile for 2024-2025 cert (flutter/flutter#155052) 2024-09-11 [email protected] Factor out `Container` objects (flutter/flutter#153619) 2024-09-11 [email protected] Move (`dev/tools`), complete v0 of `native_driver` (Android) (flutter/flutter#154843) 2024-09-11 [email protected] Roll Flutter Engine from ade8ef293bc6 to ee5adf6d2ee1 (2 revisions) (flutter/flutter#155046) 2024-09-11 [email protected] Fix `flutter run` on Mac x64 hosts if Swift Package Manager is enabled (flutter/flutter#154645) 2024-09-11 [email protected] Roll Packages from bb53e5d to 4c18648 (1 revision) (flutter/flutter#155033) 2024-09-11 [email protected] Roll Flutter Engine from 4eb729b7a5c4 to ade8ef293bc6 (3 revisions) (flutter/flutter#155031) 2024-09-11 [email protected] fix: Dropdown menu trying to access highlight element which doesn't exist when search and filters both are enabled (flutter/flutter#151969) 2024-09-11 [email protected] Marks Linux build_tests_3_5 to be unflaky (flutter/flutter#154993) 2024-09-11 [email protected] Add 'direction' allow to 'SegmentedButton' oriented vertically (flutter/flutter#150903) 2024-09-11 [email protected] Marks Linux build_tests_5_5 to be unflaky (flutter/flutter#154995) 2024-09-11 [email protected] Update the signature of DDS launcher callback. (flutter/flutter#154949) 2024-09-11 [email protected] Migrate Color.toString() test, improves `equalsIgnoringHashCodes` (flutter/flutter#154934) 2024-09-11 [email protected] Update material and cupertino localizations (flutter/flutter#154959) 2024-09-11 [email protected] Marks Linux build_tests_1_5 to be unflaky (flutter/flutter#154991) 2024-09-11 [email protected] Marks Linux build_tests_2_5 to be unflaky (flutter/flutter#154992) 2024-09-11 [email protected] Fix `flutter create` warning regarding Java compatibility (flutter/flutter#152836) 2024-09-11 [email protected] Roll Flutter Engine from 54757dab9462 to 4eb729b7a5c4 (1 revision) (flutter/flutter#155022) 2024-09-11 [email protected] Fix java version used by `build_aar_module_test` (flutter/flutter#154967) 2024-09-11 [email protected] Roll Flutter Engine from 0a14c519ea4f to 54757dab9462 (1 revision) (flutter/flutter#155015) 2024-09-11 [email protected] Roll Flutter Engine from 35a3171b72c5 to 0a14c519ea4f (1 revision) (flutter/flutter#154984) 2024-09-11 [email protected] Roll Flutter Engine from b9c0b96c3316 to 35a3171b72c5 (1 revision) (flutter/flutter#154980) 2024-09-11 [email protected] Roll Flutter Engine from 52eeea075767 to b9c0b96c3316 (1 revision) (flutter/flutter#154976) 2024-09-11 [email protected] Roll Flutter Engine from a26075f9b1e6 to 52eeea075767 (1 revision) (flutter/flutter#154973) 2024-09-11 [email protected] Roll Flutter Engine from 60c15bc0f40e to a26075f9b1e6 (6 revisions) (flutter/flutter#154969) 2024-09-11 [email protected] Migrate `apple-mobile-web-*` to `mobile-web-*`. (flutter/flutter#154964) 2024-09-11 [email protected] Roll Flutter Engine from 8a038a6f7099 to 60c15bc0f40e (15 revisions) (flutter/flutter#154960) 2024-09-10 [email protected] Adds dart fixes for Color opacity functions (flutter/flutter#154953) 2024-09-10 [email protected] Missing benchmarks for `foundation/all_elements_bench.dart` (flutter/flutter#154954) 2024-09-10 [email protected] Update color assertions (flutter/flutter#154752) 2024-09-10 [email protected] handle EAGAIN (macOS) in ErrorHandlingProcessManager (flutter/flutter#154306) 2024-09-10 [email protected] fix unpack freezing app with animation duration zero (flutter/flutter#153890) 2024-09-10 [email protected] Remove last `--disable-dart-dev` in `flutter/flutter`. (flutter/flutter#154948) 2024-09-10 [email protected] Remove scheduler: luci from new `build_aar_module_test` (flutter/flutter#154945) 2024-09-10 [email protected] Roll pub packages (flutter/flutter#154939) 2024-09-10 [email protected] `CupertinoSlidingSegmentedControl` update (flutter/flutter#152976) 2024-09-10 [email protected] Roll pub packages (flutter/flutter#154933) 2024-09-10 [email protected] fix test `chrome.close can recover if getTab throws a StateError` (flutter/flutter#154889) 2024-09-10 [email protected] SearchBar context menu (flutter/flutter#154833) 2024-09-10 [email protected] Fix `flutter build aar` for modules that use a plugin (flutter/flutter#154757) 2024-09-10 [email protected] Roll Packages from b4e0fc1 to bb53e5d (4 revisions) (flutter/flutter#154926) 2024-09-10 [email protected] Clean up `SnackBar` inherit theme data test (flutter/flutter#154921) ...

…ror` (flutter/flutter#154889)

github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Sep 10, 2024

fix

8f6c6c9

andrewkolos force-pushed the fix-state-error-test branch 2 times, most recently from e8c1c60 to 8f6c6c9 Compare September 10, 2024 00:58

andrewkolos marked this pull request as ready for review September 10, 2024 01:01

andrewkolos requested a review from christopherfujino September 10, 2024 01:01

christopherfujino approved these changes Sep 10, 2024

View reviewed changes

andrewkolos added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 10, 2024

auto-submit bot merged commit d88e692 into flutter:master Sep 10, 2024

engine-flutter-autoroll mentioned this pull request Sep 10, 2024

Roll Flutter from 2e221e7308ba to d88e69289562 (36 revisions) flutter/packages#7619

Closed

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

d88e692 fix test `chrome.close can recover if getTab throws a StateEr…

12581c4

…ror` (flutter/flutter#154889)

engine-flutter-autoroll mentioned this pull request Sep 10, 2024

Roll Flutter from 2e221e7308ba to 55af75d57bc7 (37 revisions) flutter/packages#7620

Closed

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

d88e692 fix test `chrome.close can recover if getTab throws a StateEr…

b34d1e5

…ror` (flutter/flutter#154889)

andrewkolos added a commit to andrewkolos/flutter that referenced this pull request Sep 10, 2024

apply changes from flutter#154889

2a5d2cc

andrewkolos mentioned this pull request Sep 10, 2024

[CP-stable] When shutting down chrome, guard the call to ChromeConnection.getTab #154380

Merged

9 tasks

engine-flutter-autoroll mentioned this pull request Sep 11, 2024

Roll Flutter from 2e221e7308ba to fadbf4447172 (55 revisions) flutter/packages#7623

Closed

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

d88e692 fix test `chrome.close can recover if getTab throws a StateEr…

20eda0a

…ror` (flutter/flutter#154889)

engine-flutter-autoroll mentioned this pull request Sep 11, 2024

Roll Flutter from 2e221e7308ba to d565781992f1 (56 revisions) flutter/packages#7624

Closed

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

d88e692 fix test `chrome.close can recover if getTab throws a StateEr…

3e4f574

…ror` (flutter/flutter#154889)

engine-flutter-autoroll mentioned this pull request Sep 11, 2024

Roll Flutter from 2e221e7308ba to 0a9148870ad9 (57 revisions) flutter/packages#7625

Closed

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

d88e692 fix test `chrome.close can recover if getTab throws a StateEr…

05d3fc1

…ror` (flutter/flutter#154889)

engine-flutter-autoroll mentioned this pull request Sep 11, 2024

Roll Flutter from 2e221e7308ba to d8de38f10d56 (59 revisions) flutter/packages#7627

Closed

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

d88e692 fix test `chrome.close can recover if getTab throws a StateEr…

aa2fd82

…ror` (flutter/flutter#154889)

engine-flutter-autoroll mentioned this pull request Sep 11, 2024

Roll Flutter from 2e221e7308ba to bcf1c5a24358 (60 revisions) flutter/packages#7628

Closed

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

d88e692 fix test `chrome.close can recover if getTab throws a StateEr…

2a7bfa3

…ror` (flutter/flutter#154889)

engine-flutter-autoroll mentioned this pull request Sep 11, 2024

Roll Flutter from 2e221e7308ba to 6e46ee8d6d6f (63 revisions) flutter/packages#7629

Closed

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

d88e692 fix test `chrome.close can recover if getTab throws a StateEr…

21a21d0

…ror` (flutter/flutter#154889)

engine-flutter-autoroll mentioned this pull request Sep 11, 2024

Roll Flutter from 2e221e7308ba to 04e1b174ea6d (66 revisions) flutter/packages#7630

Closed

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

d88e692 fix test `chrome.close can recover if getTab throws a StateEr…

c409fc3

…ror` (flutter/flutter#154889)

engine-flutter-autoroll mentioned this pull request Sep 12, 2024

Roll Flutter from 2e221e7308ba to 50a190caec0c (74 revisions) flutter/packages#7634

Closed

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

d88e692 fix test `chrome.close can recover if getTab throws a StateEr…

fd6fb39

…ror` (flutter/flutter#154889)

engine-flutter-autoroll mentioned this pull request Sep 12, 2024

Roll Flutter from 2e221e7308ba to d8f8613a5b94 (75 revisions) flutter/packages#7635

Closed

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

d88e692 fix test `chrome.close can recover if getTab throws a StateEr…

4baa362

…ror` (flutter/flutter#154889)

engine-flutter-autoroll mentioned this pull request Sep 12, 2024

Roll Flutter from 2e221e7308ba to 90aa59f6c32f (76 revisions) flutter/packages#7637

Closed

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

d88e692 fix test `chrome.close can recover if getTab throws a StateEr…

ed8b399

…ror` (flutter/flutter#154889)

engine-flutter-autoroll mentioned this pull request Sep 12, 2024

Roll Flutter from 2e221e7308ba to 303f222e17e3 (77 revisions) flutter/packages#7638

Merged

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

d88e692 fix test `chrome.close can recover if getTab throws a StateEr…

8913ced

…ror` (flutter/flutter#154889)

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

d88e692 fix test `chrome.close can recover if getTab throws a StateEr…

d8bf01b

…ror` (flutter/flutter#154889)

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

d88e692 fix test `chrome.close can recover if getTab throws a StateEr…

93034ec

…ror` (flutter/flutter#154889)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix test `chrome.close can recover if getTab throws a StateError` #154889

fix test `chrome.close can recover if getTab throws a StateError` #154889

Uh oh!

andrewkolos commented Sep 10, 2024 •

edited

Loading

Uh oh!

christopherfujino left a comment

Uh oh!

andrewkolos commented Sep 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix test chrome.close can recover if getTab throws a StateError #154889

fix test chrome.close can recover if getTab throws a StateError #154889

Uh oh!

Conversation

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

Uh oh!

How I verified this change

Pre-launch Checklist

Uh oh!

christopherfujino left a comment

Choose a reason for hiding this comment

Uh oh!

andrewkolos commented Sep 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix test `chrome.close can recover if getTab throws a StateError` #154889

fix test `chrome.close can recover if getTab throws a StateError` #154889

andrewkolos commented Sep 10, 2024 •

edited

Loading