Skip to content

Move CheckedModeBanner tests to material and remove Material import from widgets banner_test #181261

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 10 commits into from
Feb 4, 2026
Merged

Move CheckedModeBanner tests to material and remove Material import from widgets banner_test #181261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
5ef2485
test: move CheckedModeBanner tests to material and remove Material im…
kazbeksultanov Jan 21, 2026
ced9c6e
Merge branch 'master' into fix-remove-material-import-banner-test
kazbeksultanov Jan 22, 2026
c5ddadb
Returned the tests and changed Material with WidgetApp with default p…
kazbeksultanov Jan 22, 2026
ae50f81
rename test name
kazbeksultanov Jan 22, 2026
7ea3247
removed material import
kazbeksultanov Jan 22, 2026
80c2de9
Update packages/flutter/test/widgets/banner_test.dart
kazbeksultanov Jan 22, 2026
c44f47a
Merge branch 'master' into fix-remove-material-import-banner-test
kazbeksultanov Jan 30, 2026
95beaa4
refactored using TestWidgetsApp
kazbeksultanov Jan 30, 2026
606fa0d
Merge branch 'master' into fix-remove-material-import-banner-test
kazbeksultanov Feb 3, 2026
745ed30
remove the Color
kazbeksultanov Feb 3, 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
1 change: 0 additions & 1 deletion dev/bots/check_tests_cross_imports.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@ class TestsCrossImportChecker {
'packages/flutter/test/widgets/system_context_menu_test.dart',
'packages/flutter/test/widgets/scrollable_fling_test.dart',
'packages/flutter/test/widgets/debug_test.dart',
'packages/flutter/test/widgets/banner_test.dart',
'packages/flutter/test/widgets/transformed_scrollable_test.dart',
'packages/flutter/test/widgets/run_app_async_test.dart',
'packages/flutter/test/widgets/scrollable_in_overlay_test.dart',
Expand Down
8 changes: 5 additions & 3 deletions packages/flutter/test/widgets/banner_test.dart
View file
Open in desktop

@navaronbracke navaronbracke Jan 21, 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.

Merely "moving" the file does not address this issue.

The checked mode banner widget is also used for CupertinoApp, so the proper fix is to replace MaterialApp and friends in this file with their flutter/widgets counterpart. This is blocked on #180456

@justinmc justinmc Jan 21, 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.

I agree with this.

@navaronbracke navaronbracke Jan 21, 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.

I did manage to fix this without moving the file in #181274

Maybe we should prefer that PR?

@kazbeksultanov kazbeksultanov Jan 22, 2026 •
edited
Loading

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.

It was my bad; it seems like I misunderstood this task.

I agree that TestWidgetsApp is really needed. But I don't see it yet on the master.

I expected that TestWidgetsApp is required because, there will be a lot of defaults values to be passed.
But for this exact case I was able to use WidgetsApp with default parameters to simulate Material.
Now, I think we can create private _TestWidgetApp in each file where it's required by tests, and adjust it for that use case only without generating one universal TestWidgetsApp for all cases.

What do you think? Is it ok?
@justinmc @navaronbracke

@navaronbracke navaronbracke Jan 22, 2026 •
edited
Loading

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.

There is a TestWidgetsApp that will be introduced soon, but that PR is not yet merged. If it works for banner_test.dart by providing WidgetsApp with some defaults, then LGTM.

@kazbeksultanov kazbeksultanov Jan 23, 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.

For this case seems like it's working fine.
@navaronbracke @justinmc can you take a look?

@justinmc justinmc Jan 27, 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.

Whoops I should have left comment https://github.com/flutter/flutter/pull/181261/files#r2733374357 here. I think we should wait for TestWidgetsApp.

Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@

import 'dart:math' as math;

import 'package:flutter/material.dart';
import 'package:flutter/widgets.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart';

import 'widgets_app_tester.dart';

class TestCanvas implements Canvas {
final List<Invocation> invocations = <Invocation>[];

Expand Down Expand Up @@ -276,9 +278,9 @@ void main() {
debugDisableShadows = true;
});

testWidgets('Banner widget in MaterialApp', (WidgetTester tester) async {
testWidgets('Banner widget in WidgetsApp', (WidgetTester tester) async {
debugDisableShadows = false;
await tester.pumpWidget(const MaterialApp(home: Placeholder()));
await tester.pumpWidget(const TestWidgetsApp(home: Placeholder()));
expect(
find.byType(CheckedModeBanner),
paints
Expand Down