[CP-beta][Widget Preview] Fix flaky integration test timeout during flutter clean (#184991)#185300
Conversation
…ter clean (flutter#184991) The test was flaking and timing out because it requested a Hot Restart via DTD immediately after running `flutter clean` on the root project. Wiping the root project's `.dart_tool/` directory deletes `package_config.json`, which breaks the background Analysis Server and incremental compilers. Because the compiler cannot re-resolve dependencies, the Hot Restart fails and the previewer never sends the second `Connected` event, resulting in a timeout. This fix resolves the flakiness by synchronously restoring dependencies using `flutter pub get` right after the `flutter clean` call. This restores `package_config.json` so the background compilers and Analysis Server can successfully re-read dependencies and execute the Hot Restart deterministically. We also updated process spawners to use `addTearDown` logic to ensure leaked processes are safely killed after test completion. Fixes flutter#184985
|
This pull request was opened from and to a release candidate branch. This should only be done as part of the official Flutter release process. If you are attempting to make a regular contribution to the Flutter project, please close this PR and follow the instructions at Tree Hygiene for detailed instructions on contributing to Flutter. Reviewers: Use caution before merging pull requests to release branches. Ensure the proper procedure has been followed. |
There was a problem hiding this comment.
Code Review
This pull request integrates shutdownHooks into the widget preview and compiler components to gracefully handle process termination and prevent 'unexpected exit' errors during shutdown. It also updates integration tests to restore dependencies after a flutter clean and improves test reliability with better process monitoring and cleanup. Review feedback suggests expanding the try-catch block in LspPreviewDetector to include waitForAnalysis, ensuring that the compilerOutput future is always completed to prevent potential hangs, and utilizing addTearDown for stream subscriptions in tests to avoid resource leaks.
justinmc
left a comment
There was a problem hiding this comment.
CPLGTM 👍 . Apologies that I missed this PR earlier in the week, it will likely not be released until next week.
My bad! The issue was filed after the change landed since I actually found and fixed this issue while fixing some test flakiness. The PR closing that issue was #184991. |
|
Ok that makes sense, thanks! |
|
@bkonyi Could you check the test failure? It doesn't look like a typical flake but not sure. |
That definitely doesn't look related to this change. I'm hoping it's just a flake, so I've restarted the check. |
|
Looks like there's still a failure, I think the same one (Mac tool_integration_tests_4_5). |
|
I think you just need to rebase this to a commit after 1af378e which fixes this test failure. |
36e0007
into
flutter:flutter-3.44-candidate.0
Issue Link:
What is the link to the issue this cherry-pick is addressing?
#185299
Impact Description:
What is the impact (ex. visual jank on Samsung phones, app crash, cannot ship an iOS app)?
Does it impact development (ex. flutter doctor crashes when Android Studio is installed),
or the shipping of production apps (the app crashes on launch).
This information is for domain experts and release engineers to understand the consequences of saying yes or no to the cherry pick.
flutter widget-preview startcan crash on shutdown if there's an outstanding request to retrieve previews from the analysis server.Changelog Description:
Explain this cherry pick:
See best practices for examples.
flutter widget-preview startcan encounter an unhandled exception during shutdown due to uncomplete requests.Workaround:
Is there a workaround for this issue?
No.
Risk:
What is the risk level of this cherry-pick?
Test Coverage:
Are you confident that your fix is well-tested by automated tests?
Validation Steps:
What are the steps to validate that this fix works?
Run
packages/flutter_tools/test/integration.shard/widget_preview_smoke_test.dartand verify there's no crashes.