[cupertino_ui] Migrate route_test.dart to SemanticsHandle#11993
[cupertino_ui] Migrate route_test.dart to SemanticsHandle#11993Renzo-Olivares wants to merge 5 commits into
route_test.dart to SemanticsHandle#11993Conversation
There was a problem hiding this comment.
Code Review
This pull request unskips the tests in packages/cupertino_ui/test/route_test.dart and refactors them to use tester.ensureSemantics() and standard semantics matchers instead of SemanticsTester. There are no review comments, and I have no feedback to provide.
There was a problem hiding this comment.
Code Review
This pull request removes a test skip annotation, replaces SemanticsTester with SemanticsHandle and standard semantics matchers, wraps dialog and popup presentation calls in unawaited, and replaces manual throw statements with fail(). The review feedback suggests utilizing addTearDown to guarantee that debugDefaultTargetPlatformOverride and SemanticsHandle are cleaned up if a test fails, allowing the removal of manual cleanup lines.
| debugDefaultTargetPlatformOverride = TargetPlatform.iOS; | ||
| final semantics = SemanticsTester(tester); | ||
| final SemanticsHandle handle = tester.ensureSemantics(); |
There was a problem hiding this comment.
Overriding debugDefaultTargetPlatformOverride without ensuring it is reset in an addTearDown block can leak the iOS platform override to subsequent tests if this test fails. Using addTearDown ensures that both the platform override and the SemanticsHandle are properly cleaned up regardless of test success or failure.
debugDefaultTargetPlatformOverride = TargetPlatform.iOS;
addTearDown(() {
debugDefaultTargetPlatformOverride = null;
});
final SemanticsHandle handle = tester.ensureSemantics();
addTearDown(handle.dispose);| expect(find.semantics.byLabel('Dismiss'), findsNothing); | ||
|
|
||
| debugDefaultTargetPlatformOverride = null; | ||
| semantics.dispose(); | ||
| handle.dispose(); |
There was a problem hiding this comment.
Since debugDefaultTargetPlatformOverride and handle are now cleaned up automatically via addTearDown, these manual cleanup lines can be removed.
expect(find.semantics.byLabel('Dismiss'), findsNothing);| debugDefaultTargetPlatformOverride = TargetPlatform.iOS; | ||
| final semantics = SemanticsTester(tester); | ||
| final SemanticsHandle handle = tester.ensureSemantics(); |
There was a problem hiding this comment.
Overriding debugDefaultTargetPlatformOverride without ensuring it is reset in an addTearDown block can leak the iOS platform override to subsequent tests if this test fails. Using addTearDown ensures that both the platform override and the SemanticsHandle are properly cleaned up regardless of test success or failure.
debugDefaultTargetPlatformOverride = TargetPlatform.iOS;
addTearDown(() {
debugDefaultTargetPlatformOverride = null;
});
final SemanticsHandle handle = tester.ensureSemantics();
addTearDown(handle.dispose);| expect( | ||
| semantics, | ||
| includesNodeWith( | ||
| actions: <SemanticsAction>[SemanticsAction.tap, SemanticsAction.dismiss], | ||
| label: 'Dismiss', | ||
| ), | ||
| find.semantics.byLabel('Dismiss'), | ||
| isSemantics(label: 'Dismiss', hasTapAction: true, hasDismissAction: true), | ||
| ); | ||
|
|
||
| debugDefaultTargetPlatformOverride = null; | ||
| semantics.dispose(); | ||
| handle.dispose(); |
There was a problem hiding this comment.
Since debugDefaultTargetPlatformOverride and handle are now cleaned up automatically via addTearDown, these manual cleanup lines can be removed.
expect(
find.semantics.byLabel('Dismiss'),
isSemantics(label: 'Dismiss', hasTapAction: true, hasDismissAction: true),
);
Part of flutter/flutter#182636 and flutter/flutter#188395
This PR:
widgets/semantics_tester.dart. ReplacedSemanticsTesterwithSemanticsHandle.@Skipannotation, all tests in this file has passed.semantics_tester.darthas existed incupertino_ui, so we can directly importsemantics_tester.dart;test/folder.Pre-Review Checklist
[shared_preferences]///).