[ci] Adds test for web hot restart with const App. by ditman · Pull Request #141824 · flutter/flutter

ditman · 2024-01-19T01:09:52Z

This PR adds a test that reproduces the problem described in the linked issue: hot restart on the web seems to not update if the app being run is const.

The new test is expected to fail, until the const issue with hot restart in the web is resolved.

Expected failure mode is a 15s timeout in the following test:

02:31 +3 ~1 -1: Hot reload (index.html: Default) (with `const MyApp()`)): newly added code executes during hot restart [E]
  TimeoutException after 0:00:15.000000: Future not completed
  dart:async  _startMicrotaskLoop
  ...

(And then a bunch of output that I'm not 100% sure is intended :))

Issues

[web] Flutter web hot restart not reflecting changes #141588

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.
All existing and new tests are passing.

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

ditman · 2024-01-19T01:32:08Z

Seems to be failing as expected: https://ci.chromium.org/ui/p/flutter/builders/try/Mac%20web_tool_tests/36148/overview

nshahan

Thanks David, LGTM! I have not idea what the process would be to land this change if it is failing currently. I'll try using it locally though to verify my changes in the Dart SDK.

christopherfujino · 2024-01-19T20:46:31Z

Thanks David, LGTM! I have not idea what the process would be to land this change if it is failing currently. I'll try using it locally though to verify my changes in the Dart SDK.

Probably the best way would be to manually verify the dart sdk changes running this test locally, and then once the fix lands in the Dart SDK main and rolls to flutter/flutter, we can rebase this PR and the presubmits should go green, which would verify the fix.

nshahan · 2024-01-19T23:32:20Z

Probably the best way would be to manually verify the dart sdk changes running this test locally, and then once the fix lands in the Dart SDK main and rolls to flutter/flutter, we can rebase this PR and the presubmits should go green, which would verify the fix.

Sounds good, can you help me identify the right command to run this test locally if I already have my engine built with my changes to the embedded dart sdk?

christopherfujino · 2024-01-19T23:51:01Z

Probably the best way would be to manually verify the dart sdk changes running this test locally, and then once the fix lands in the Dart SDK main and rolls to flutter/flutter, we can rebase this PR and the presubmits should go green, which would verify the fix.

Sounds good, can you help me identify the right command to run this test locally if I already have my engine built with my changes to the embedded dart sdk?

Ooh, that's a good question actually....

christopherfujino · 2024-01-19T23:55:51Z

packages/flutter_tools/test/web.shard/hot_reload_web_test.dart

@nshahan Ooh, it looks like this test harness will conditionally check for the following environment variables, and if provided, forward them to the underlying flutter tool it invokes: https://github.com/ditman/flutter-flutter/blob/web-test-hot-restart-const-app/packages/flutter_tools/test/integration.shard/test_utils.dart#L73

I'm guessing you want to run this with something like:

$ cd /path/to/framework/checkout $ cd packages/flutter_tools $ FLUTTER_LOCAL_ENGINE=host_debug_unopt FLUTTER_LOCAL_ENGINE_HOST=host_debug_unopt ../../bin/dart test test/web.shard/hot_reload_web_test.dart

ditman · 2024-01-20T00:20:39Z

@nshahan this is the command I've been using to run this test in my local checkout of the flutter framework:

dit@dit:/work/flutter/flutter/packages/flutter_tools$ flutter test test/web.shard/hot_reload_web_test.dart

(But what @christopherfujino might help with overriding the SDK)

[...] the process would be to land this change if it is failing currently. I'll try using it locally though to verify my changes in the Dart SDK.

[...] once the fix lands in the Dart SDK main and rolls to flutter/flutter, we can rebase this PR and the presubmits should go green

This is the idea, yes; this will need to be rebased before it can land (or be landed at the same time as a the engine roll that contains the fix).

ditman · 2024-01-24T19:03:09Z

This should pass, now that #142098 has rolled into the framework.

ditman · 2024-01-24T19:22:07Z

However, they don't :/

04:28 +8 ~2 -1: /Volumes/Work/s/w/ir/x/w/flutter/packages/flutter_tools/test/web.shard/hot_reload_web_test.dart: Hot reload (index.html: Default) (with `const MyApp()`)): newly added code executes during hot restart [E]
  TimeoutException after 0:00:15.000000: Future not completed
  
186s            Spawning flutter [run, --machine, --no-serve-observatory, -d, chrome, --web-run-headless, --verbose, --web-renderer=html] in /Volumes/Work/s/w/ir/x/t/flutter_hot_reload_test.1XfRrI

https://ci.chromium.org/ui/p/flutter/builders/try/Mac%20web_tool_tests/36340/overview

ditman · 2024-01-24T19:31:59Z

Hot restart seems to work in my Linux machine in master, though, so I'm starting to think that this test is sus... (Or maybe the "update with master" button didn't do what I wanted, but still: sus...)

ditman · 2024-01-24T19:53:10Z

This seems to work locally, so the test must be bad :/

…not get lost.

ditman · 2024-01-25T00:30:00Z

@nshahan suggested a fix for the test (related to an old, linked issue) and it seems to work as expected now! (I'll verify the test does fail by rewinding master to before Nic's fix landed)

Mac web_tool_tests has succeeded, which is very promising!

christopherfujino · 2024-01-25T00:36:03Z

packages/flutter_tools/test/integration.shard/test_data/hot_reload_project.dart

+
+    // We need to insist here for `const` Apps, so print statements don't
+    // get lost between the browser and the test driver.
+    // See: https://github.com/flutter/flutter/issues/86202


IDK what's going on here, but if it fixes the test, I can live with that :P

Check the linked issue. This loop used to run runApp every second, but we discovered that we just need to run the "hot restart worked" bits :)

ditman · 2024-01-25T01:01:48Z

I've just checked that if I apply this PR on top of beta, the test fails as expected. See this branch.

christopherfujino

LGTM

ditman · 2024-01-25T01:48:15Z

Do your thang, autosubmit!

(And don't you dare become a flake now, test!)

…lutter#141824)

Roll Flutter from 19b06f4 to a8efa77 (38 revisions) flutter/flutter@19b06f4...a8efa77 2024-01-25 [email protected] Roll Flutter Engine from e2014f007f61 to 7c4ed15cb271 (1 revision) (flutter/flutter#142221) 2024-01-25 [email protected] Roll Flutter Engine from 5fa2e2920274 to e2014f007f61 (1 revision) (flutter/flutter#142213) 2024-01-25 [email protected] provide command to `FakeCommand::onRun` (flutter/flutter#142206) 2024-01-25 [email protected] Roll Flutter Engine from c346fd3d9ca1 to 5fa2e2920274 (1 revision) (flutter/flutter#142212) 2024-01-25 [email protected] Roll Flutter Engine from d4d8f668159b to c346fd3d9ca1 (1 revision) (flutter/flutter#142209) 2024-01-25 [email protected] Roll Flutter Engine from 499ed00bbda2 to d4d8f668159b (2 revisions) (flutter/flutter#142205) 2024-01-25 [email protected] Roll Flutter Engine from 4880592ca5ba to 499ed00bbda2 (6 revisions) (flutter/flutter#142202) 2024-01-25 [email protected] [ci] Adds test for web hot restart with const App. (flutter/flutter#141824) 2024-01-25 [email protected] Migrate android_view to linux_android_emu platform. (flutter/flutter#142184) 2024-01-25 [email protected] Refactor `external_ui` without making any name changes (I think) (flutter/flutter#142192) 2024-01-25 [email protected] Fix text selection edge scrolling when inside a horizontal scrollable (flutter/flutter#140250) 2024-01-25 [email protected] Roll Flutter Engine from d7bf5ec1dcdd to 4880592ca5ba (2 revisions) (flutter/flutter#142186) 2024-01-24 [email protected] Roll Flutter Engine from 6a7f963dc751 to d7bf5ec1dcdd (2 revisions) (flutter/flutter#142185) 2024-01-24 [email protected] Reland "Remove hack from PageView." (flutter/flutter#142172) 2024-01-24 [email protected] Upgrade leak_tracker. (flutter/flutter#142162) 2024-01-24 [email protected] Migrate android views to devicelab. (flutter/flutter#142081) 2024-01-24 [email protected] Roll Flutter Engine from ed498f111d53 to 6a7f963dc751 (4 revisions) (flutter/flutter#142176) 2024-01-24 [email protected] Support wireless debugging for iOS 12 or earlier (flutter/flutter#141439) 2024-01-24 [email protected] Roll Flutter Engine from 4bc368ee5f74 to ed498f111d53 (5 revisions) (flutter/flutter#142167) 2024-01-24 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Refactor `external_ui` â�� `external_textures`" (flutter/flutter#142173) 2024-01-24 [email protected] Refactor `external_ui` â�� `external_textures` (flutter/flutter#142062) 2024-01-24 [email protected] Roll Flutter Engine from 0b9fce355df9 to 4bc368ee5f74 (3 revisions) (flutter/flutter#142157) 2024-01-24 [email protected] Update navigationBar label's maxScaleFactor to meet GAR requirement (flutter/flutter#141998) 2024-01-24 [email protected] Roll Flutter Engine from b65556fa543e to 0b9fce355df9 (1 revision) (flutter/flutter#142147) 2024-01-24 [email protected] Marks Mac_arm64 build_tests_2_4 to be unflaky (flutter/flutter#142115) 2024-01-24 [email protected] Marks Mac_x64 build_tests_2_4 to be unflaky (flutter/flutter#142111) 2024-01-24 [email protected] Marks Mac_x64 build_tests_1_4 to be unflaky (flutter/flutter#142110) 2024-01-24 [email protected] Marks Mac_x64 build_tests_3_4 to be unflaky (flutter/flutter#142112) 2024-01-24 [email protected] Revise tooltip theme docs, including more cross-references (flutter/flutter#137316) 2024-01-24 [email protected] Run some tests explicitly in both arm and x64. (flutter/flutter#141910) 2024-01-24 [email protected] Roll Flutter Engine from ba3a70ce7722 to b65556fa543e (3 revisions) (flutter/flutter#142140) 2024-01-24 [email protected] Fixes #138773, port autocomplete to OverlayPortal (flutter/flutter#140285) 2024-01-24 [email protected] Revert "[web] - Fix broken `TextField` in semantics mode when it's a sibling of `Navigator`" (flutter/flutter#142129) 2024-01-24 [email protected] Marks Mac_arm64 build_tests_1_4 to be unflaky (flutter/flutter#142114) 2024-01-24 [email protected] Roll Packages from 841fe90 to 8fbdf65 (2 revisions) (flutter/flutter#142139) 2024-01-24 [email protected] Marks Mac_arm64 build_tests_3_4 to be unflaky (flutter/flutter#142116) 2024-01-24 [email protected] Marks Mac_x64 build_tests_4_4 to be unflaky (flutter/flutter#142113) 2024-01-24 [email protected] Marks Mac_arm64 build_tests_4_4 to be unflaky (flutter/flutter#142117) 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],[email protected],[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 ...

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

ditman force-pushed the web-test-hot-restart-const-app branch from 8e0af29 to 6298eb3 Compare January 19, 2024 01:11

ditman requested review from christopherfujino and nshahan January 19, 2024 01:12

ditman changed the title ~~[ci] Adds test for web hot restart with non-const app.~~ [ci] Adds test for web hot restart with const App. Jan 19, 2024

nshahan reviewed Jan 19, 2024

View reviewed changes

christopherfujino reviewed Jan 19, 2024

View reviewed changes

[ci] Adds test for flutter#141588

b06cc21

ditman force-pushed the web-test-hot-restart-const-app branch from 3a9356e to b06cc21 Compare January 24, 2024 19:34

Insist that hot reload really really REALLY worked, so the memo does …

2163c79

…not get lost.

christopherfujino reviewed Jan 25, 2024

View reviewed changes

christopherfujino approved these changes Jan 25, 2024

View reviewed changes

ditman added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 25, 2024

auto-submit bot merged commit 703e12f into flutter:master Jan 25, 2024

ditman deleted the web-test-hot-restart-const-app branch January 25, 2024 01:50

christopherfujino mentioned this pull request Jan 25, 2024

[web] Flutter web hot restart not reflecting changes #141588

Closed

engine-flutter-autoroll mentioned this pull request Jan 25, 2024

Roll Flutter from 19b06f4ee9a0 to a8efa771d6a3 (38 revisions) flutter/packages#5969

Merged

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

703e12f [ci] Adds test for web hot restart with const App. (flutter/f…

f241477

…lutter#141824)

darshankawar mentioned this pull request Nov 7, 2024

Hot Reload & Hot Restart don't work in web #158254

Closed

Conversation

ditman commented Jan 19, 2024

Issues

Pre-launch Checklist

Uh oh!

ditman commented Jan 19, 2024

Uh oh!

nshahan left a comment

Choose a reason for hiding this comment

Uh oh!

christopherfujino commented Jan 19, 2024

Uh oh!

nshahan commented Jan 19, 2024

Uh oh!

christopherfujino commented Jan 19, 2024

Uh oh!

christopherfujino Jan 19, 2024

Choose a reason for hiding this comment

Uh oh!

ditman commented Jan 20, 2024

Uh oh!

ditman commented Jan 24, 2024

Uh oh!

ditman commented Jan 24, 2024

Uh oh!

ditman commented Jan 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ditman commented Jan 24, 2024

Uh oh!

ditman commented Jan 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

christopherfujino Jan 25, 2024

Choose a reason for hiding this comment

Uh oh!

ditman Jan 25, 2024

Choose a reason for hiding this comment

Uh oh!

ditman commented Jan 25, 2024

Uh oh!

christopherfujino left a comment

Choose a reason for hiding this comment

Uh oh!

ditman commented Jan 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ditman commented Jan 24, 2024 •

edited

Loading

ditman commented Jan 25, 2024 •

edited

Loading