Skip to content

Conversation

@andrewkolos
Copy link
Contributor

@andrewkolos andrewkolos commented Sep 10, 2024

Fixes #154857.

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

  1. 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();
  1. 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"`
  1. Revert the change from step 1 and run again. The test should now pass.

Pre-launch Checklist

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

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Sep 10, 2024
@andrewkolos andrewkolos force-pushed the fix-state-error-test branch 2 times, most recently from e8c1c60 to 8f6c6c9 Compare September 10, 2024 00:58
@andrewkolos andrewkolos marked this pull request as ready for review September 10, 2024 01:01
Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@andrewkolos andrewkolos added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 10, 2024
@andrewkolos
Copy link
Contributor Author

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

@auto-submit auto-submit bot merged commit d88e692 into flutter:master Sep 10, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 10, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 10, 2024
andrewkolos added a commit to andrewkolos/flutter that referenced this pull request Sep 10, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 12, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Sep 12, 2024
Roll Flutter from 2e221e7 to 303f222 (77 revisions)

flutter/flutter@2e221e7...303f222

2024-09-12 34871572+gmackall@users.noreply.github.com Manual roll to 48ddaf578fb0c8326d5b4b680b0f49ea72e33216 (flutter/flutter#155070)
2024-09-12 reidbaker@google.com Externalize and update onboarding instructions (flutter/flutter#154730)
2024-09-12 andrewrkolos@gmail.com 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 chris@bracken.jp iOS: update provisioning profile for 2024-2025 cert (flutter/flutter#155052)
2024-09-11 nate.w5687@gmail.com Factor out `Container` objects (flutter/flutter#153619)
2024-09-11 matanlurey@users.noreply.github.com Move (`dev/tools`), complete v0 of `native_driver` (Android) (flutter/flutter#154843)
2024-09-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from ade8ef293bc6 to ee5adf6d2ee1 (2 revisions) (flutter/flutter#155046)
2024-09-11 737941+loic-sharma@users.noreply.github.com Fix `flutter run` on Mac x64 hosts if Swift Package Manager is enabled (flutter/flutter#154645)
2024-09-11 engine-flutter-autoroll@skia.org Roll Packages from bb53e5d to 4c18648 (1 revision) (flutter/flutter#155033)
2024-09-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from 4eb729b7a5c4 to ade8ef293bc6 (3 revisions) (flutter/flutter#155031)
2024-09-11 34465683+rkishan516@users.noreply.github.com 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 fluttergithubbot@gmail.com Marks Linux build_tests_3_5 to be unflaky (flutter/flutter#154993)
2024-09-11 abnhamoda@gmail.com Add 'direction' allow to 'SegmentedButton' oriented vertically (flutter/flutter#150903)
2024-09-11 fluttergithubbot@gmail.com Marks Linux build_tests_5_5 to be unflaky (flutter/flutter#154995)
2024-09-11 chingjun@google.com Update the signature of DDS launcher callback. (flutter/flutter#154949)
2024-09-11 30870216+gaaclarke@users.noreply.github.com Migrate Color.toString() test, improves `equalsIgnoringHashCodes` (flutter/flutter#154934)
2024-09-11 36861262+QuncCccccc@users.noreply.github.com Update material and cupertino localizations (flutter/flutter#154959)
2024-09-11 fluttergithubbot@gmail.com Marks Linux build_tests_1_5 to be unflaky (flutter/flutter#154991)
2024-09-11 fluttergithubbot@gmail.com Marks Linux build_tests_2_5 to be unflaky (flutter/flutter#154992)
2024-09-11 111122076+shashwatpathak98@users.noreply.github.com Fix `flutter create` warning regarding Java compatibility (flutter/flutter#152836)
2024-09-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from 54757dab9462 to 4eb729b7a5c4 (1 revision) (flutter/flutter#155022)
2024-09-11 34871572+gmackall@users.noreply.github.com Fix java version used by `build_aar_module_test` (flutter/flutter#154967)
2024-09-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from 0a14c519ea4f to 54757dab9462 (1 revision) (flutter/flutter#155015)
2024-09-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from 35a3171b72c5 to 0a14c519ea4f (1 revision) (flutter/flutter#154984)
2024-09-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from b9c0b96c3316 to 35a3171b72c5 (1 revision) (flutter/flutter#154980)
2024-09-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from 52eeea075767 to b9c0b96c3316 (1 revision) (flutter/flutter#154976)
2024-09-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from a26075f9b1e6 to 52eeea075767 (1 revision) (flutter/flutter#154973)
2024-09-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from 60c15bc0f40e to a26075f9b1e6 (6 revisions) (flutter/flutter#154969)
2024-09-11 matanlurey@users.noreply.github.com Migrate `apple-mobile-web-*` to `mobile-web-*`. (flutter/flutter#154964)
2024-09-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from 8a038a6f7099 to 60c15bc0f40e (15 revisions) (flutter/flutter#154960)
2024-09-10 30870216+gaaclarke@users.noreply.github.com Adds dart fixes for Color opacity functions (flutter/flutter#154953)
2024-09-10 codefu@google.com Missing benchmarks for `foundation/all_elements_bench.dart` (flutter/flutter#154954)
2024-09-10 30870216+gaaclarke@users.noreply.github.com Update color assertions (flutter/flutter#154752)
2024-09-10 andrewrkolos@gmail.com handle EAGAIN (macOS) in ErrorHandlingProcessManager (flutter/flutter#154306)
2024-09-10 50643541+Mairramer@users.noreply.github.com fix unpack freezing app with animation duration zero  (flutter/flutter#153890)
2024-09-10 matanlurey@users.noreply.github.com Remove last `--disable-dart-dev` in `flutter/flutter`. (flutter/flutter#154948)
2024-09-10 34871572+gmackall@users.noreply.github.com Remove scheduler: luci from new `build_aar_module_test` (flutter/flutter#154945)
2024-09-10 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#154939)
2024-09-10 36861262+QuncCccccc@users.noreply.github.com `CupertinoSlidingSegmentedControl` update (flutter/flutter#152976)
2024-09-10 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#154933)
2024-09-10 andrewrkolos@gmail.com fix test `chrome.close can recover if getTab throws a StateError` (flutter/flutter#154889)
2024-09-10 jmccandless@google.com SearchBar context menu (flutter/flutter#154833)
2024-09-10 34871572+gmackall@users.noreply.github.com Fix `flutter build aar` for modules that use a plugin (flutter/flutter#154757)
2024-09-10 engine-flutter-autoroll@skia.org Roll Packages from b4e0fc1 to bb53e5d (4 revisions) (flutter/flutter#154926)
2024-09-10 tessertaha@gmail.com Clean up `SnackBar` inherit theme data test (flutter/flutter#154921)
...
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[tool] the chrome.close can recover if getTab throws a StateError test doesn't verify intended behavior

2 participants