From 253dc6f8479a14b59a4f3f1e2a8341e4d6873bd5 Mon Sep 17 00:00:00 2001 From: "auto-submit[bot]" <98614782+auto-submit[bot]@users.noreply.github.com> Date: Wed, 3 Jan 2024 22:36:17 +0000 Subject: [PATCH] Reverts "Re-land integrate testWidgets with leak tracking." (#140926) Reverts flutter/flutter#140521 Initiated by: zanderso This change reverts the following previous change: Original Description: Original PR: https://github.com/flutter/flutter/pull/138057 Revert: https://github.com/flutter/flutter/pull/140502 Issue: https://ci.chromium.org/ui/p/flutter/builders/prod/Linux_android%20flutter_test_performance/12787/overview Exception: flutter test rendered unexpected output (1 bad lines) Explanation: leak tracker adds tear down even when there is no leak tracking, because at the moment of adding tear down it is unclear if leak tracking will be used for some tests. Fix: add enabling flag for leak tracker and make creation of tear down conditional. Prerequisites: --- .../flutter/test/flutter_test_config.dart | 29 ++- .../flutter_test/lib/src/test_compat.dart | 24 -- .../flutter_test/lib/src/widget_tester.dart | 16 -- .../test/utils/leaking_classes.dart | 54 ----- .../test/widget_tester_leaks_test.dart | 205 ------------------ 5 files changed, 14 insertions(+), 314 deletions(-) delete mode 100644 packages/flutter_test/test/utils/leaking_classes.dart delete mode 100644 packages/flutter_test/test/widget_tester_leaks_test.dart diff --git a/packages/flutter/test/flutter_test_config.dart b/packages/flutter/test/flutter_test_config.dart index d78d61930a..fac826f612 100644 --- a/packages/flutter/test/flutter_test_config.dart +++ b/packages/flutter/test/flutter_test_config.dart @@ -34,24 +34,23 @@ Future testExecutable(FutureOr Function() testMain) { // receive the event. WidgetController.hitTestWarningShouldBeFatal = true; + LeakTracking.warnForUnsupportedPlatforms = false; + + LeakTesting.settings = LeakTesting + .settings + // TODO(polina-c): clean up leaks and stop ignoring them. + // https://github.com/flutter/flutter/issues/137311 + .withIgnored( + allNotGCed: true, + notDisposed: { + 'OverlayEntry': null, + }, + ); + // Leak tracking is off by default. // To enable it, follow doc for [_kLeakTracking]. if (_kLeakTracking) { - LeakTesting.enable(); - - LeakTracking.warnForUnsupportedPlatforms = false; - - LeakTesting.settings = LeakTesting - .settings - .withTrackedAll() - // TODO(polina-c): clean up leaks and stop ignoring them. - // https://github.com/flutter/flutter/issues/137311 - .withIgnored( - allNotGCed: true, - notDisposed: { - 'OverlayEntry': null, - }, - ); + LeakTesting.settings = LeakTesting.settings.withTrackedAll(); } // Enable golden file testing using Skia Gold. diff --git a/packages/flutter_test/lib/src/test_compat.dart b/packages/flutter_test/lib/src/test_compat.dart index 9e64974893..201f616b19 100644 --- a/packages/flutter_test/lib/src/test_compat.dart +++ b/packages/flutter_test/lib/src/test_compat.dart @@ -4,7 +4,6 @@ import 'dart:async'; -import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart'; import 'package:meta/meta.dart'; import 'package:test_api/scaffolding.dart' show Timeout; import 'package:test_api/src/backend/declarer.dart'; // ignore: implementation_imports @@ -164,7 +163,6 @@ void test( Map? onPlatform, int? retry, }) { - _configureTearDownForTestFile(); _declarer.test( description.toString(), body, @@ -188,7 +186,6 @@ void test( /// of running the group's tests. @isTestGroup void group(Object description, void Function() body, { dynamic skip, int? retry }) { - _configureTearDownForTestFile(); _declarer.group(description.toString(), body, skip: skip, retry: retry); } @@ -204,7 +201,6 @@ void group(Object description, void Function() body, { dynamic skip, int? retry /// Each callback at the top level or in a given group will be run in the order /// they were declared. void setUp(dynamic Function() body) { - _configureTearDownForTestFile(); _declarer.setUp(body); } @@ -222,7 +218,6 @@ void setUp(dynamic Function() body) { /// /// See also [addTearDown], which adds tear-downs to a running test. void tearDown(dynamic Function() body) { - _configureTearDownForTestFile(); _declarer.tearDown(body); } @@ -240,7 +235,6 @@ void tearDown(dynamic Function() body) { /// prefer [setUp], and only use [setUpAll] if the callback is prohibitively /// slow. void setUpAll(dynamic Function() body) { - _configureTearDownForTestFile(); _declarer.setUpAll(body); } @@ -256,27 +250,9 @@ void setUpAll(dynamic Function() body) { /// prefer [tearDown], and only use [tearDownAll] if the callback is /// prohibitively slow. void tearDownAll(dynamic Function() body) { - _configureTearDownForTestFile(); _declarer.tearDownAll(body); } -bool _isTearDownForTestFileConfigured = false; -/// Configures `tearDownAll` after all user defined `tearDownAll` in the test file. -/// -/// This function should be invoked in all functions, that may be invoked by user in the test file, -/// to be invoked before any other `tearDownAll`. -void _configureTearDownForTestFile() { - if (_isTearDownForTestFileConfigured) { - return; - } - _declarer.tearDownAll(_tearDownForTestFile); - _isTearDownForTestFileConfigured = true; -} - -/// Tear down that should happen after all user defined tear down. -Future _tearDownForTestFile() async { - await maybeTearDownLeakTrackingForAll(); -} /// A reporter that prints each test on its own line. /// diff --git a/packages/flutter_test/lib/src/widget_tester.dart b/packages/flutter_test/lib/src/widget_tester.dart index b035088f94..2a8c36dcc7 100644 --- a/packages/flutter_test/lib/src/widget_tester.dart +++ b/packages/flutter_test/lib/src/widget_tester.dart @@ -9,7 +9,6 @@ import 'package:flutter/material.dart' show Tooltip; import 'package:flutter/rendering.dart'; import 'package:flutter/scheduler.dart'; import 'package:flutter/services.dart'; -import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart'; import 'package:matcher/expect.dart' as matcher_expect; import 'package:meta/meta.dart'; import 'package:test_api/scaffolding.dart' as test_package; @@ -117,18 +116,6 @@ E? _lastWhereOrNull(Iterable list, bool Function(E) test) { /// If the [tags] are passed, they declare user-defined tags that are implemented by /// the `test` package. /// -/// The argument [experimentalLeakTesting] is experimental and is not recommended -/// for use outside of the Flutter framework. -/// When [experimentalLeakTesting] is set, it is used to leak track objects created -/// during test execution. -/// Otherwise [LeakTesting.settings] is used. -/// Adjust [LeakTesting.settings] in flutter_test_config.dart -/// (see https://github.com/flutter/flutter/blob/master/packages/flutter_test/lib/flutter_test.dart) -/// for the entire package or folder, or in the test's main for a test file -/// (don't use [setUp] or [setUpAll]). -/// To turn off leak tracking just for one test, set [experimentalLeakTesting] to -/// `LeakTrackingForTests.ignore()`. -/// /// ## Sample code /// /// ```dart @@ -148,7 +135,6 @@ void testWidgets( TestVariant variant = const DefaultTestVariant(), dynamic tags, int? retry, - LeakTesting? experimentalLeakTesting, }) { assert(variant.values.isNotEmpty, 'There must be at least one value to test in the testing variant.'); final TestWidgetsFlutterBinding binding = TestWidgetsFlutterBinding.ensureInitialized(); @@ -179,11 +165,9 @@ void testWidgets( Object? memento; try { memento = await variant.setUp(value); - maybeSetupLeakTrackingForTest(experimentalLeakTesting, combinedDescription); await callback(tester); } finally { await variant.tearDown(value, memento); - maybeTearDownLeakTrackingForTest(); } semanticsHandle?.dispose(); }, diff --git a/packages/flutter_test/test/utils/leaking_classes.dart b/packages/flutter_test/test/utils/leaking_classes.dart deleted file mode 100644 index b49463f22e..0000000000 --- a/packages/flutter_test/test/utils/leaking_classes.dart +++ /dev/null @@ -1,54 +0,0 @@ -// Copyright 2014 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import 'package:flutter/widgets.dart'; -import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart'; - -class LeakTrackedClass { - LeakTrackedClass() { - LeakTracking.dispatchObjectCreated( - library: library, - className: '$LeakTrackedClass', - object: this, - ); - } - - static const String library = 'package:my_package/lib/src/my_lib.dart'; - - void dispose() { - LeakTracking.dispatchObjectDisposed(object: this); - } -} - -final List _notGCedObjects = []; - -class LeakingClass { - LeakingClass() { - _notGCedObjects.add(LeakTrackedClass()..dispose()); - } -} - -class StatelessLeakingWidget extends StatelessWidget { - StatelessLeakingWidget({ - super.key, - this.notGCed = true, - this.notDisposed = true, - }) { - if (notGCed) { - _notGCedObjects.add(LeakTrackedClass()..dispose()); - } - if (notDisposed) { - // ignore: unused_local_variable, it is unused intentionally, to illustrate not disposed object. - final LeakTrackedClass notDisposedObject = LeakTrackedClass(); - } - } - - final bool notGCed; - final bool notDisposed; - - @override - Widget build(BuildContext context) { - return const Placeholder(); - } -} diff --git a/packages/flutter_test/test/widget_tester_leaks_test.dart b/packages/flutter_test/test/widget_tester_leaks_test.dart deleted file mode 100644 index 3a912fba46..0000000000 --- a/packages/flutter_test/test/widget_tester_leaks_test.dart +++ /dev/null @@ -1,205 +0,0 @@ -// Copyright 2014 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import 'package:flutter/cupertino.dart'; -import 'package:flutter/material.dart'; -import 'package:flutter_test/flutter_test.dart'; -import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart'; - -import 'utils/leaking_classes.dart'; - -late final String _test1TrackingOnNoLeaks; -late final String _test2TrackingOffLeaks; -late final String _test3TrackingOnLeaks; -late final String _test4TrackingOnWithCreationStackTrace; -late final String _test5TrackingOnWithDisposalStackTrace; -late final String _test6TrackingOnNoLeaks; -late final String _test7TrackingOnNoLeaks; -late final String _test8TrackingOnNotDisposed; - -void main() { - LeakTesting.enable(); - LeakTesting.collectedLeaksReporter = (Leaks leaks) => verifyLeaks(leaks); - LeakTesting.settings = LeakTesting.settings.copyWith(ignore: false); - - // It is important that the test file starts with group, to test that leaks are collected for all tests after group too. - group('Group', () { - testWidgets('test', (_) async { - StatelessLeakingWidget(); - }); - }); - - testWidgets(_test1TrackingOnNoLeaks = 'test1, tracking-on, no leaks', (WidgetTester widgetTester) async { - expect(LeakTracking.isStarted, true); - expect(LeakTracking.phase.name, _test1TrackingOnNoLeaks); - expect(LeakTracking.phase.ignoreLeaks, false); - await widgetTester.pumpWidget(Container()); - }); - - testWidgets( - _test2TrackingOffLeaks = 'test2, tracking-off, leaks', - experimentalLeakTesting: LeakTesting.settings.withIgnoredAll(), - (WidgetTester widgetTester) async { - expect(LeakTracking.isStarted, true); - expect(LeakTracking.phase.name, null); - expect(LeakTracking.phase.ignoreLeaks, true); - await widgetTester.pumpWidget(StatelessLeakingWidget()); - }); - - testWidgets(_test3TrackingOnLeaks = 'test3, tracking-on, leaks', (WidgetTester widgetTester) async { - expect(LeakTracking.isStarted, true); - expect(LeakTracking.phase.name, _test3TrackingOnLeaks); - expect(LeakTracking.phase.ignoreLeaks, false); - await widgetTester.pumpWidget(StatelessLeakingWidget()); - }); - - testWidgets( - _test4TrackingOnWithCreationStackTrace = 'test4, tracking-on, with creation stack trace', - experimentalLeakTesting: LeakTesting.settings.withCreationStackTrace(), - (WidgetTester widgetTester) async { - expect(LeakTracking.isStarted, true); - expect(LeakTracking.phase.name, _test4TrackingOnWithCreationStackTrace); - expect(LeakTracking.phase.ignoreLeaks, false); - await widgetTester.pumpWidget(StatelessLeakingWidget()); - }, - ); - - testWidgets( - _test5TrackingOnWithDisposalStackTrace = 'test5, tracking-on, with disposal stack trace', - experimentalLeakTesting: LeakTesting.settings.withDisposalStackTrace(), - (WidgetTester widgetTester) async { - expect(LeakTracking.isStarted, true); - expect(LeakTracking.phase.name, _test5TrackingOnWithDisposalStackTrace); - expect(LeakTracking.phase.ignoreLeaks, false); - await widgetTester.pumpWidget(StatelessLeakingWidget()); - }, - ); - - testWidgets(_test6TrackingOnNoLeaks = 'test6, tracking-on, no leaks', (_) async { - LeakTrackedClass().dispose(); - }); - - testWidgets(_test7TrackingOnNoLeaks = 'test7, tracking-on, tear down, no leaks', (_) async { - final LeakTrackedClass myClass = LeakTrackedClass(); - addTearDown(myClass.dispose); - }); - - testWidgets(_test8TrackingOnNotDisposed = 'test8, tracking-on, not disposed leak', (_) async { - expect(LeakTracking.isStarted, true); - expect(LeakTracking.phase.name, _test8TrackingOnNotDisposed); - expect(LeakTracking.phase.ignoreLeaks, false); - LeakTrackedClass(); - }); -} - -int _leakReporterInvocationCount = 0; - -void verifyLeaks(Leaks leaks) { - _leakReporterInvocationCount += 1; - expect(_leakReporterInvocationCount, 1); - - try { - expect(leaks, isLeakFree); - } on TestFailure catch (e) { - expect(e.message, contains('https://github.com/dart-lang/leak_tracker')); - - expect(e.message, isNot(contains(_test1TrackingOnNoLeaks))); - expect(e.message, isNot(contains(_test2TrackingOffLeaks))); - expect(e.message, contains('test: $_test3TrackingOnLeaks')); - expect(e.message, contains('test: $_test4TrackingOnWithCreationStackTrace')); - expect(e.message, contains('test: $_test5TrackingOnWithDisposalStackTrace')); - expect(e.message, isNot(contains(_test6TrackingOnNoLeaks))); - expect(e.message, isNot(contains(_test7TrackingOnNoLeaks))); - expect(e.message, contains('test: $_test8TrackingOnNotDisposed')); - } - - _verifyLeaks( - leaks, - _test3TrackingOnLeaks, - notDisposed: 1, - notGCed: 1, - expectedContextKeys: >{ - LeakType.notGCed: [], - LeakType.notDisposed: [], - }, - ); - _verifyLeaks( - leaks, - _test4TrackingOnWithCreationStackTrace, - notDisposed: 1, - notGCed: 1, - expectedContextKeys: >{ - LeakType.notGCed: ['start'], - LeakType.notDisposed: ['start'], - }, - ); - _verifyLeaks( - leaks, - _test5TrackingOnWithDisposalStackTrace, - notDisposed: 1, - notGCed: 1, - expectedContextKeys: >{ - LeakType.notGCed: ['disposal'], - LeakType.notDisposed: [], - }, - ); - _verifyLeaks( - leaks, - _test8TrackingOnNotDisposed, - notDisposed: 1, - expectedContextKeys: >{}, - ); -} - -/// Verifies [allLeaks] contain expected number of leaks for the test [testDescription]. -/// -/// [notDisposed] and [notGCed] set number for expected leaks by leak type. -/// The method will fail if the leaks context does not contain [expectedContextKeys]. -void _verifyLeaks( - Leaks allLeaks, - String testDescription, { - int notDisposed = 0, - int notGCed = 0, - Map> expectedContextKeys = const >{}, -}) { - final Leaks testLeaks = Leaks( - allLeaks.byType.map( - (LeakType key, List value) => - MapEntry>(key, value.where((LeakReport leak) => leak.phase == testDescription).toList()), - ), - ); - - for (final LeakType type in expectedContextKeys.keys) { - final List leaks = testLeaks.byType[type]!; - final List expectedKeys = expectedContextKeys[type]!..sort(); - for (final LeakReport leak in leaks) { - final List actualKeys = leak.context?.keys.toList() ?? []; - expect(actualKeys..sort(), equals(expectedKeys), reason: '$testDescription, $type'); - } - } - - _verifyLeakList( - testLeaks.notDisposed, - notDisposed, - testDescription, - ); - _verifyLeakList( - testLeaks.notGCed, - notGCed, - testDescription, - ); -} - -void _verifyLeakList( - List list, - int expectedCount, - String testDescription, -) { - expect(list.length, expectedCount, reason: testDescription); - - for (final LeakReport leak in list) { - expect(leak.trackedClass, contains(LeakTrackedClass.library)); - expect(leak.trackedClass, contains('$LeakTrackedClass')); - } -}