From 34f1f5f19e63dc757443fbb86693131be5df6db9 Mon Sep 17 00:00:00 2001 From: Polina Cherkasova Date: Wed, 10 Jan 2024 11:04:28 -0800 Subject: [PATCH] Improve testing for leak tracking. (#140553) --- .../flutter/test/flutter_test_config.dart | 1 + .../test/utils/memory_leak_tests.dart | 95 ++++++++ .../test/widget_tester_leaks_test.dart | 225 ++++-------------- 3 files changed, 140 insertions(+), 181 deletions(-) create mode 100644 packages/flutter_test/test/utils/memory_leak_tests.dart diff --git a/packages/flutter/test/flutter_test_config.dart b/packages/flutter/test/flutter_test_config.dart index 9baf5a0181d..5ce404e8c83 100644 --- a/packages/flutter/test/flutter_test_config.dart +++ b/packages/flutter/test/flutter_test_config.dart @@ -44,6 +44,7 @@ Future testExecutable(FutureOr Function() testMain) { LeakTesting.settings = LeakTesting .settings .withIgnored( + createdByTestHelpers: true, allNotGCed: true, ); } diff --git a/packages/flutter_test/test/utils/memory_leak_tests.dart b/packages/flutter_test/test/utils/memory_leak_tests.dart new file mode 100644 index 00000000000..7b90eb9a4d1 --- /dev/null +++ b/packages/flutter_test/test/utils/memory_leak_tests.dart @@ -0,0 +1,95 @@ +// 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'; + +/// Objects that should not be GCed during test run. +final List _retainer = []; + +/// Test cases for memory leaks. +/// +/// They are separate from test execution to allow +/// excluding them from test helpers. +final List memoryLeakTests = [ + LeakTestCase( + name: 'no leaks', + body: (PumpWidgetsCallback? pumpWidgets, + RunAsyncCallback? runAsync) async { + await pumpWidgets!(Container()); + }, + ), + LeakTestCase( + name: 'not disposed disposable', + body: (PumpWidgetsCallback? pumpWidgets, + RunAsyncCallback? runAsync) async { + InstrumentedDisposable(); + }, + notDisposedTotal: 1, + ), + LeakTestCase( + name: 'not GCed disposable', + body: (PumpWidgetsCallback? pumpWidgets, + RunAsyncCallback? runAsync) async { + _retainer.add(InstrumentedDisposable()..dispose()); + }, + notGCedTotal: 1, + ), + LeakTestCase( + name: 'leaking widget', + body: (PumpWidgetsCallback? pumpWidgets, + RunAsyncCallback? runAsync) async { + StatelessLeakingWidget(); + }, + notDisposedTotal: 1, + notGCedTotal: 1, + ), + LeakTestCase( + name: 'dispose in tear down', + body: (PumpWidgetsCallback? pumpWidgets, + RunAsyncCallback? runAsync) async { + final InstrumentedDisposable myClass = InstrumentedDisposable(); + addTearDown(myClass.dispose); + }, + ), + LeakTestCase( + name: 'pumped leaking widget', + body: (PumpWidgetsCallback? pumpWidgets, + RunAsyncCallback? runAsync) async { + await pumpWidgets!(StatelessLeakingWidget()); + }, + notDisposedTotal: 1, + notGCedTotal: 1, + ), + LeakTestCase( + name: 'leaking widget in runAsync', + body: (PumpWidgetsCallback? pumpWidgets, + RunAsyncCallback? runAsync) async { + await runAsync!(() async { + StatelessLeakingWidget(); + }); + }, + notDisposedTotal: 1, + notGCedTotal: 1, + ), + LeakTestCase( + name: 'pumped in runAsync', + body: (PumpWidgetsCallback? pumpWidgets, + RunAsyncCallback? runAsync) async { + await runAsync!(() async { + await pumpWidgets!(StatelessLeakingWidget()); + }); + }, + notDisposedTotal: 1, + notGCedTotal: 1, + ), +]; + +String memoryLeakTestsFilePath() { + return RegExp(r'(\/[^\/]*.dart):') + .firstMatch(StackTrace.current.toString())! + .group(1).toString(); +} diff --git a/packages/flutter_test/test/widget_tester_leaks_test.dart b/packages/flutter_test/test/widget_tester_leaks_test.dart index eb6b54f3841..13012d8fe7b 100644 --- a/packages/flutter_test/test/widget_tester_leaks_test.dart +++ b/packages/flutter_test/test/widget_tester_leaks_test.dart @@ -2,199 +2,62 @@ // 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'; -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; +import 'utils/memory_leak_tests.dart'; + +class _TestExecution { + _TestExecution( + {required this.settings, required this.settingName, required this.test}); + + final String settingName; + final LeakTesting settings; + final LeakTestCase test; + + String get name => '${test.name}, $settingName'; +} + +final List<_TestExecution> _testExecutions = <_TestExecution>[]; void main() { + LeakTesting.collectedLeaksReporter = _verifyLeaks; 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(), // this test is not tracked by design - (WidgetTester widgetTester) async { - 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()); - }, + LeakTesting.settings = LeakTesting.settings + .withTrackedAll() + .withTracked(allNotDisposed: true, allNotGCed: true) + .withIgnored( + createdByTestHelpers: true, + testHelperExceptions: [ + RegExp(RegExp.escape(memoryLeakTestsFilePath())) + ], ); - 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 { - InstrumentedDisposable().dispose(); - }); - - testWidgets(_test7TrackingOnNoLeaks = 'test7, tracking-on, tear down, no leaks', (_) async { - final InstrumentedDisposable myClass = InstrumentedDisposable(); - 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); - InstrumentedDisposable(); - }); -} - -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'); + for (final LeakTestCase test in memoryLeakTests) { + for (final MapEntry settingsCase + in leakTestingSettingsCases.entries) { + final LeakTesting settings = settingsCase.value(LeakTesting.settings); + if (settings.leakDiagnosticConfig.collectRetainingPathForNotGCed) { + // Retaining path requires vm to be started, so skipping. + continue; + } + final _TestExecution execution = _TestExecution( + settingName: settingsCase.key, test: test, settings: settings); + _testExecutions.add(execution); + testWidgets(execution.name, experimentalLeakTesting: settings, + (WidgetTester tester) async { + await test.body(tester.pumpWidget, tester.runAsync); + }); } } - - _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(InstrumentedDisposable.library)); - expect(leak.trackedClass, contains('$InstrumentedDisposable')); +void _verifyLeaks(Leaks leaks) { + for (final _TestExecution execution in _testExecutions) { + final Leaks testLeaks = leaks.byPhase[execution.name] ?? Leaks.empty(); + execution.test.verifyLeaks(testLeaks, execution.settings, + testDescription: execution.name); } }