Repository navigation
[framework] Remove no-shuffle tag definitions and remaining usages - #191499
Conversation
Remove the `no-shuffle` tag and associated TODO comments from: - `packages/flutter/test/material/dropdown_test.dart` - `packages/flutter/test/material/progress_indicator_test.dart` - `packages/flutter/test/material/scrollbar_test.dart` Fix test order dependency and state leakage in `scrollbar_test.dart`: - Clean up active mouse pointers in hover tests using `removePointer()`. - Release gestures and pointers in drag tests. - Provide explicit `Theme` contexts and platform-specific painter assertions in `Scrollbar` painting tests to prevent host-platform fallback theme leakage. Fixes flutter#85160
Remove the `no-shuffle` tag declarations from `dart_test.yaml` configurations and delete obsolete `dart_test.yaml` files that only contained `no-shuffle`. Also remove the remaining `no-shuffle` tags from `text_field_test.dart`, `overall_experience_test.dart`, and `gen_defaults_test.dart`. Fixes flutter#85160
|
This pull request contains changes to Material or Cupertino, which are currently frozen in this repository. Changes should be made in Please refer to #188444 for instructions. |
github-actions
Bot
added
a: tests
github-actions
Bot
removed
a: text input
bkonyi
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the no-shuffle test tag configuration from multiple test configuration files and removes the @Tags(<String>['no-shuffle']) annotation from gen_defaults_test.dart. There are no review comments, and I have no feedback to provide.
Piinks
left a comment
Piinks
left a comment
There was a problem hiding this comment.
Note: Material and Cupertino test tag removals and fixes are being handled in flutter/packages under packages/material_ui and packages/cupertino_ui.
flutter/packages CI does not support no-shuffle, so all good! :)
I am curious, if we find more tests with a shuffle seed problem, won't we still need this? We find these ad hoc when the daily seed reveals them. How do we mark new instances?
If we're able to identify that shuffling is causing a specific set of tests to fail, we should fix them instead of disabling shuffling. Shuffling will only cause test failures if there's state being leaked between tests, which normally should only require some refactoring to fix. LLMs are good enough now at that sort of refactoring work that I don't expect having to fix shuffling issues when they appear to be a burden, so I'd rather not leave a pathway to introduce additional technical debt. |
|
Makes sense! I will defer to @justinmc to approve since I live over in flutter/packages (mostly) now. 😜 |
justinmc
left a comment
justinmc
left a comment
There was a problem hiding this comment.
LGTM 👍 Yay for no more no-shuffle.
auto-submit
Bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/flutter/191499, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label. |
flutter-dashboard
Bot
removed
the
autosubmit
The overlay() and overlayWithEntry() helpers in this file pump a TextField with no Theme ancestor, so Theme.of resolves to the process-wide fallback ThemeData. That fallback is built once, at the first theme-less Theme.of call in the run, and freezes `platform` to whatever defaultTargetPlatform was at that moment. In declaration order that first call happens on android, so every test in the file, including the TargetPlatformVariant iOS/macOS/windows/linux/fuchsia variants, ran with Material selection handles, toolbar, and cursor behavior. Under --test-randomize-ordering-seed=20261008 the first theme-less call happens inside an iOS variant, the fallback becomes iOS for the whole run, and 17 tests fail (handle drags land on the Cupertino toolbar, 'Select all' is not in the Cupertino menu, the cursor animates instead of blinking, the caret is offset by the iOS cursorOffset). Those failures then leak a semantics handle that breaks 28 more semantics tests later in the run. Give the helpers an explicit `theme` parameter that defaults to a ThemeData built for the platform in effect, so each test gets the theme for its own TargetPlatformVariant regardless of test order. The 14 tests that wrapped overlay() in their own Theme pass it through the parameter instead, so an inner Theme does not shadow them. Five variant instances of the two handle-drag tests only passed because of the frozen android fallback: macOS, Linux, and Windows selection controls draw no handles at all, and the Cupertino start handle is not where the test grabbed it. Replace them with tests of what each platform does: iOS drags Cupertino handles (with the toolbar hidden, since it overlaps the start handle), Android and Fuchsia drag Material handles (Fuchsia's desktop-style toolbar otherwise covers them), and desktop platforms show no handles. Also drop the `no-shuffle` tag, whose definition was removed from dart_test.yaml in flutter#191499, along with the stale TODO that pointed at an unrelated closed issue. Fixes flutter#194008
3 participants
Remove
no-shuffletag declarations and remaining usages from the repository.Changes
no-shuffleTag Removals:dev/tools/gen_defaults/test/gen_defaults_test.dart: Removedno-shuffletag.packages/flutter/dart_test.yaml: Removedno-shuffletag definition.packages/flutter_tools/dart_test.yaml: Removedno-shuffletag definition.dart_test.yamlfiles containing onlyno-shuffle(dev/tools/gen_defaults/dart_test.yaml,packages/flutter_driver/dart_test.yaml,packages/flutter_test/dart_test.yaml).Fixes #85160
Pre-launch Checklist
///).