Skip to content

[cupertino_ui] Re-enable dialog_test.dart #12057

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
auto-submit merged 7 commits into from
Jul 1, 2026
Merged

[cupertino_ui] Re-enable dialog_test.dart #12057

Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c2bbd60
Re-enabled dialog_test.
justinmc Jun 29, 2026
cf7cc29
Unawaited
justinmc Jun 29, 2026
ab8a7ce
SemanticsHandle
justinmc Jun 29, 2026
b335d18
Simplify byPredicate to byLabel
justinmc Jun 29, 2026
d977332
No ensureSemantics needed.
justinmc Jun 30, 2026
fa16d3f
Merge branch 'main' into cross-imports-dialog
justinmc Jul 1, 2026
4ee13ef
Test whole tree.
justinmc Jul 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
174 changes: 67 additions & 107 deletions ...mporarily_disabled_tests/dialog_test.dart → packages/cupertino_ui/test/dialog_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,12 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

@Skip(
'This file is skipped due to a cross-import that needs to be fixed. Tracked in https://github.com/flutter/flutter/issues/177028.',
)
// This file is run as part of a reduced test set in CI on Mac and Windows
// machines.
@Tags(<String>['reduced-test-set'])
library;

import 'dart:async' show unawaited;
import 'dart:math';
import 'dart:ui';

Expand All @@ -19,8 +17,6 @@ import 'package:flutter/rendering.dart';
import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';

import '../widgets/semantics_tester.dart';

void main() {
testWidgets('Overall appearance is correct for the light theme', (WidgetTester tester) async {
await tester.pumpWidget(
Expand Down Expand Up @@ -575,16 +571,18 @@ void main() {

final BuildContext context = tester.element(find.text('Go'));

showCupertinoDialog<void>(
context: context,
builder: (BuildContext context) {
return Container(
width: 100.0,
height: 100.0,
alignment: Alignment.center,
child: const Text('Dialog'),
);
},
unawaited(
showCupertinoDialog<void>(
context: context,
builder: (BuildContext context) {
return Container(
width: 100.0,
height: 100.0,
alignment: Alignment.center,
child: const Text('Dialog'),
);
},
),
);

await tester.pumpAndSettle(const Duration(seconds: 1));
Expand All @@ -602,17 +600,19 @@ void main() {

final BuildContext context = tester.element(find.text('Go'));

showCupertinoDialog<void>(
context: context,
barrierDismissible: true,
builder: (BuildContext context) {
return Container(
width: 100.0,
height: 100.0,
alignment: Alignment.center,
child: const Text('Dialog'),
);
},
unawaited(
showCupertinoDialog<void>(
context: context,
barrierDismissible: true,
builder: (BuildContext context) {
return Container(
width: 100.0,
height: 100.0,
alignment: Alignment.center,
child: const Text('Dialog'),
);
},
),
);

await tester.pumpAndSettle(const Duration(seconds: 1));
Expand Down Expand Up @@ -683,7 +683,6 @@ void main() {
});

testWidgets('Has semantic annotations', (WidgetTester tester) async {
final semantics = SemanticsTester(tester);

@Renzo-Olivares Renzo-Olivares Jun 29, 2026

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.

nit: Should we use SemanticsHandle to ensure semantics is initialized like below (plus dispose at the end of the test)? I'm not sure how necessary this is but I've been doing it in the migrations I've done.

final SemanticsHandle handle = tester.ensureSemantics();

@justinmc justinmc Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in ab8a7ce.

@chunhtai chunhtai Jun 30, 2026

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.

semantics is enabled by default, I think this is not needed

await tester.pumpWidget(
const CupertinoApp(
home: CupertinoAlertDialog(
Expand All @@ -697,63 +696,26 @@ void main() {
),
);

final Finder dialogFinder = find.bySemanticsLabel('Alert');

@chunhtai chunhtai Jul 1, 2026

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.

this can use the isSemantics(children:) to assert the subtree

@justinmc justinmc Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 4ee13ef

I was hoping to use your new isSemantics role parameter too, but I guess we'll have to wait until we bump the SDK.

final SemanticsNode dialog = tester.semantics.find(find.bySemanticsLabel('Alert'));
expect(dialog.role, SemanticsRole.alertDialog);
expect(dialog, isSemantics(namesRoute: true, scopesRoute: true));
expect(
semantics,
hasSemantics(
TestSemantics.root(
children: <TestSemantics>[
TestSemantics(
children: <TestSemantics>[
TestSemantics(
children: <TestSemantics>[
TestSemantics(
flags: <SemanticsFlag>[SemanticsFlag.scopesRoute],
children: <TestSemantics>[
TestSemantics(
flags: <SemanticsFlag>[
SemanticsFlag.scopesRoute,
SemanticsFlag.namesRoute,
],
role: SemanticsRole.alertDialog,
label: 'Alert',
children: <TestSemantics>[
TestSemantics(
flags: <SemanticsFlag>[SemanticsFlag.hasImplicitScrolling],
children: <TestSemantics>[
TestSemantics(label: 'The Title'),
TestSemantics(label: 'Content'),
],
),
TestSemantics(
flags: <SemanticsFlag>[SemanticsFlag.hasImplicitScrolling],
children: <TestSemantics>[
TestSemantics(
flags: <SemanticsFlag>[SemanticsFlag.isButton],
label: 'Cancel',
),
TestSemantics(
flags: <SemanticsFlag>[SemanticsFlag.isButton],
label: 'OK',
),
],
),
],
),
],
),
],
),
],
),
],
),
ignoreId: true,
ignoreRect: true,
ignoreTransform: true,
),
find.descendant(of: dialogFinder, matching: find.bySemanticsLabel('The Title')),
findsOneWidget,
);

semantics.dispose();
expect(
find.descendant(of: dialogFinder, matching: find.bySemanticsLabel('Content')),
findsOneWidget,
);
final SemanticsNode buttonOK = tester.semantics.find(
find.descendant(of: dialogFinder, matching: find.bySemanticsLabel('OK')),
);
expect(buttonOK, isSemantics(isButton: true));
final SemanticsNode buttonCancel = tester.semantics.find(
find.descendant(of: dialogFinder, matching: find.bySemanticsLabel('Cancel')),
);
expect(buttonCancel, isSemantics(isButton: true));

@gemini-code-assist gemini-code-assist Bot Jun 29, 2026

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.

medium

During the migration from SemanticsTester to tester.semantics.find, the assertions verifying the hasImplicitScrolling flag on the scrollable containers (the title/content section and the actions section) were lost. This reduces the test coverage for critical accessibility behaviors of the dialog.\n\nAdditionally, dialogFinder is defined but not reused when retrieving the dialog semantics node.\n\nThis suggestion reuses dialogFinder, extracts other finders for readability, and restores the assertions for hasImplicitScrolling by verifying the parent nodes of the title and buttons.

    final Finder dialogFinder = find.bySemanticsLabel('Alert');\n    final SemanticsNode dialog = tester.semantics.find(dialogFinder);\n    expect(dialog.role, SemanticsRole.alertDialog);\n    expect(dialog, isSemantics(namesRoute: true, scopesRoute: true));\n\n    final Finder titleFinder = find.descendant(of: dialogFinder, matching: find.bySemanticsLabel('The Title'));\n    expect(titleFinder, findsOneWidget);\n    final SemanticsNode titleNode = tester.semantics.find(titleFinder);\n    expect(titleNode.parent, isSemantics(hasImplicitScrolling: true));\n\n    expect(\n      find.descendant(of: dialogFinder, matching: find.bySemanticsLabel('Content')),\n      findsOneWidget,\n    );\n\n    final Finder okFinder = find.descendant(of: dialogFinder, matching: find.bySemanticsLabel('OK'));\n    final SemanticsNode buttonOK = tester.semantics.find(okFinder);\n    expect(buttonOK, isSemantics(isButton: true));\n    expect(buttonOK.parent, isSemantics(hasImplicitScrolling: true));\n\n    final Finder cancelFinder = find.descendant(of: dialogFinder, matching: find.bySemanticsLabel('Cancel'));\n    final SemanticsNode buttonCancel = tester.semantics.find(cancelFinder);\n    expect(buttonCancel, isSemantics(isButton: true));

});

testWidgets('Dialog default action style', (WidgetTester tester) async {
Expand Down Expand Up @@ -1699,8 +1661,6 @@ void main() {
});

testWidgets('showCupertinoDialog - custom barrierLabel', (WidgetTester tester) async {
final semantics = SemanticsTester(tester);

@Renzo-Olivares Renzo-Olivares Jun 29, 2026

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.

https://github.com/flutter/packages/pull/12057/changes#r3494863310


await tester.pumpWidget(
CupertinoApp(
home: Builder(
Expand Down Expand Up @@ -1731,13 +1691,7 @@ void main() {
),
);

expect(
semantics,
isNot(
includesNodeWith(label: 'Custom label', flags: <SemanticsFlag>[SemanticsFlag.namesRoute]),
),
);
semantics.dispose();
expect(find.semantics.byLabel('Custom label'), findsNothing);
});

testWidgets('showCupertinoDialog - custom barrierColor', (WidgetTester tester) async {
Expand Down Expand Up @@ -1913,12 +1867,14 @@ void main() {
);

final BuildContext context = tester.element(find.text('Test'));
showCupertinoDialog<void>(
context: context,
builder: (BuildContext context) {
return const Placeholder();
},
anchorPoint: const Offset(1000, 0),
unawaited(
showCupertinoDialog<void>(
context: context,
builder: (BuildContext context) {
return const Placeholder();
},
anchorPoint: const Offset(1000, 0),
),
);
await tester.pumpAndSettle();

Expand Down Expand Up @@ -1951,11 +1907,13 @@ void main() {
);

final BuildContext context = tester.element(find.text('Test'));
showCupertinoDialog<void>(
context: context,
builder: (BuildContext context) {
return const Placeholder();
},
unawaited(
showCupertinoDialog<void>(
context: context,
builder: (BuildContext context) {
return const Placeholder();
},
),
);
await tester.pumpAndSettle();

Expand Down Expand Up @@ -1988,11 +1946,13 @@ void main() {
);

final BuildContext context = tester.element(find.text('Test'));
showCupertinoDialog<void>(
context: context,
builder: (BuildContext context) {
return const Placeholder();
},
unawaited(
showCupertinoDialog<void>(
context: context,
builder: (BuildContext context) {
return const Placeholder();
},
),
);
await tester.pumpAndSettle();

Expand Down
Loading