Sitelet https://github.com/flutter/flutter/pull/186604
Skip to content

Allow target platform override teardown in widget tests - #186604

Open
mvincentong wants to merge 3 commits into
flutter:masterfrom
mvincentong:fix-target-platform-add-teardown
Open

Allow target platform override teardown in widget tests#186604
mvincentong wants to merge 3 commits into
flutter:masterfrom
mvincentong:fix-target-platform-add-teardown

Conversation

@mvincentong

@mvincentong mvincentong commented May 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Issue: Fixes #110488.

testWidgets used to verify Flutter test invariants before package:test executed per-test addTearDown callbacks. That made cleanup such as addTearDown(() => debugDefaultTargetPlatformOverride = null) fail even though it is the safer cleanup pattern when the test body can fail before manual cleanup runs. The same ordering problem applies to other Flutter debug invariants, not just the target platform override.

Fix: Defer the full Flutter invariant verification for testWidgets until after user addTearDown callbacks have run, but before postTest resets binding state. Direct binding.runTest callers keep the existing immediate invariant verification unless they explicitly opt into the deferred hook.

Tests:

  • ./bin/flutter test packages/flutter_test/test/bindings_invariants_test.dart
  • ./bin/flutter test packages/flutter_test/test/widget_tester_test.dart
  • ./bin/flutter test packages/flutter_test/test/event_simulation_test.dart --plain-name "debugKeyEventSimulatorTransitModeOverride overrides default transit mode"
  • ./bin/flutter analyze packages/flutter/lib/src/foundation/debug.dart packages/flutter_test/lib/src/binding.dart packages/flutter_test/lib/src/widget_tester.dart packages/flutter_test/test/bindings_invariants_test.dart
  • ./bin/dart format --output=none --set-exit-if-changed packages/flutter/lib/src/foundation/debug.dart packages/flutter_test/lib/src/binding.dart packages/flutter_test/lib/src/widget_tester.dart packages/flutter_test/test/bindings_invariants_test.dart
  • git diff --check

Risk: This is limited to flutter_test binding cleanup. The invariant checks still run before postTest; tests that leave debug state dirty after their addTearDown callbacks still fail.

@github-actions github-actions Bot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. labels May 16, 2026
gemini-code-assist[bot]
gemini-code-assist Bot reviewed May 16, 2026
View reviewed changes

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request modifies the test binding logic to allow debugDefaultTargetPlatformOverride to be reset within addTearDown callbacks. It updates debugAssertAllFoundationVarsUnset to support an expected value for the platform override and introduces verifyInvariantsAfterTestTearDown to defer invariant checks until after user tear-downs are complete. A new test case is added to verify this behavior. I have no feedback to provide.

@mvincentong
mvincentong changed the base branch from main to master May 21, 2026 03:01
@Piinks
Piinks requested a review from dkwingsmt May 26, 2026 22:42
@dkwingsmt

dkwingsmt commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Thanks for taking a look. But I don't think this is the correct way. The fundamental problem is that tearDown occurs after the check, against people's expectation. This flag is not the only victim.
I'm not sure how feasible it would be, but I think the correct way to solve this problem is to check the invariants after tearDown.

@mvincentong

mvincentong commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, that makes sense. I updated the PR in 114c487 to follow that direction: testWidgets now defers the full Flutter invariant check until after user addTearDown callbacks and before postTest, rather than special-casing debugDefaultTargetPlatformOverride. Direct binding.runTest callers keep immediate invariant verification unless they explicitly opt into the deferred hook.

I also broadened the regression to cover another foundation debug variable (debugDoublePrecision) so the test is no longer target-platform-only.

Verification:

  • ./bin/flutter test packages/flutter_test/test/bindings_invariants_test.dart passed
  • ./bin/flutter test packages/flutter_test/test/widget_tester_test.dart passed
  • ./bin/flutter test packages/flutter_test/test/event_simulation_test.dart --plain-name "debugKeyEventSimulatorTransitModeOverride overrides default transit mode" passed
  • ./bin/flutter analyze packages/flutter/lib/src/foundation/debug.dart packages/flutter_test/lib/src/binding.dart packages/flutter_test/lib/src/widget_tester.dart packages/flutter_test/test/bindings_invariants_test.dart passed
  • ./bin/dart format --output=none --set-exit-if-changed packages/flutter/lib/src/foundation/debug.dart packages/flutter_test/lib/src/binding.dart packages/flutter_test/lib/src/widget_tester.dart packages/flutter_test/test/bindings_invariants_test.dart passed
  • git diff --check passed

@dkwingsmt dkwingsmt added the CICD Run CI/CD label Jun 24, 2026
dkwingsmt
dkwingsmt reviewed Jun 24, 2026
View reviewed changes
Comment thread packages/flutter_test/lib/src/widget_tester.dart
@github-actions github-actions Bot removed the CICD Run CI/CD label Jun 25, 2026
@mvincentong

mvincentong commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Clarified this in e2cc32e: testWidgets can defer because it owns the package:test tearDown callbacks, while lower-level binding.runTest callers keep the immediate invariant check before postTest.

Local verification:

  • ./bin/dart format --output=none --set-exit-if-changed packages/flutter_test/lib/src/binding.dart packages/flutter_test/lib/src/widget_tester.dart
  • ./bin/dart analyze packages/flutter_test/lib/src/binding.dart packages/flutter_test/lib/src/widget_tester.dart
  • git diff --check

The targeted flutter test commands are currently blocked locally by root package resolution: _flutter_packages depends on webview_flutter_wkwebview 3.26.0, which requires Flutter >=3.44.0, while this checkout reports 3.44.0-1.0.pre-1099.

@mvincentong

mvincentong commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up: I was able to rerun the focused tests with --no-pub. bindings_invariants_test.dart, widget_tester_test.dart, and the focused event_simulation_test.dart case all passed locally.

@justinmc
justinmc requested a review from dkwingsmt June 30, 2026 22:24
@mvincentong
mvincentong force-pushed the fix-target-platform-add-teardown branch from e2cc32e to 7604d01 Compare August 1, 2026 12:52
@mvincentong

mvincentong commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current master at 7604d01f45 with no conflicts; the focused regression suites remain green. The fork account cannot restore the CICD label, so a maintainer may need to start presubmits.

@coderabbitai coderabbitai Bot mentioned this pull request Aug 21, 2026
Merged
@justinmc justinmc mentioned this pull request Aug 31, 2026
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dkwingsmt dkwingsmt Awaiting requested review from dkwingsmt
1 more reviewer
@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow debugDefaultTargetPlatformOverride to be resetted with addTearDown

3 participants

@mvincentong @dkwingsmt @alpesh76