diff --git a/packages/flutter/test/foundation/leak_tracking.dart b/packages/flutter/test/foundation/leak_tracking.dart index eea74ab020..26a3dbd7a0 100644 --- a/packages/flutter/test/foundation/leak_tracking.dart +++ b/packages/flutter/test/foundation/leak_tracking.dart @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:core'; + import 'package:flutter/foundation.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:leak_tracker/leak_tracker.dart'; @@ -143,28 +145,56 @@ class LeakCleaner { final LeakTrackingTestConfig config; + static Map<(String, LeakType), int> _countByClassAndType(Leaks leaks) { + final Map<(String, LeakType), int> result = <(String, LeakType), int>{}; + + for (final MapEntry> entry in leaks.byType.entries) { + for (final LeakReport leak in entry.value) { + final (String, LeakType) classAndType = (leak.type, entry.key); + result[classAndType] = (result[classAndType] ?? 0) + 1; + } + } + return result; + } + Leaks clean(Leaks leaks) { + final Map<(String, LeakType), int> countByClassAndType = _countByClassAndType(leaks); + final Leaks result = Leaks(>{ for (final LeakType leakType in leaks.byType.keys) - leakType: leaks.byType[leakType]!.where((LeakReport leak) => _shouldReportLeak(leakType, leak)).toList() + leakType: leaks.byType[leakType]!.where((LeakReport leak) => _shouldReportLeak(leakType, leak, countByClassAndType)).toList() }); return result; } /// Returns true if [leak] should be reported as failure. - bool _shouldReportLeak(LeakType leakType, LeakReport leak) { + bool _shouldReportLeak(LeakType leakType, LeakReport leak, Map<(String, LeakType), int> countByClassAndType) { // Tracking for non-GCed is temporarily disabled. // TODO(polina-c): turn on tracking for non-GCed after investigating existing leaks. if (leakType != LeakType.notDisposed) { return false; } + final String leakingClass = leak.type; + final (String, LeakType) classAndType = (leakingClass, leakType); + + bool isAllowedForClass(Map allowList) { + if (!allowList.containsKey(leakingClass)) { + return false; + } + final int? allowedCount = allowList[leakingClass]; + if (allowedCount == null) { + return true; + } + return allowedCount >= countByClassAndType[classAndType]!; + } + switch (leakType) { case LeakType.notDisposed: - return !config.notDisposedAllowList.containsKey(leak.type); + return !isAllowedForClass(config.notDisposedAllowList); case LeakType.notGCed: case LeakType.gcedLate: - return !config.notGCedAllowList.containsKey(leak.type); + return !isAllowedForClass(config.notGCedAllowList); } } } diff --git a/packages/flutter/test/foundation/leak_tracking_test.dart b/packages/flutter/test/foundation/leak_tracking_test.dart index 09f9e49446..fc71814d26 100644 --- a/packages/flutter/test/foundation/leak_tracking_test.dart +++ b/packages/flutter/test/foundation/leak_tracking_test.dart @@ -29,9 +29,22 @@ Future main() async { expect(cleanedLeaks.total, 1); }); - group('Leak tracking works for non-web', () { + test('$LeakCleaner catches extra leaks', () { + Leaks leaks = _leaksOfAllTypes(); + final LeakReport leak = leaks.notDisposed.first; + leaks.notDisposed.add(leak); + + final LeakTrackingTestConfig config = LeakTrackingTestConfig( + notDisposedAllowList: {leak.type: 1}, + ); + leaks = LeakCleaner(config).clean(leaks); + + expect(leaks.notDisposed, hasLength(2)); + }); + + group('Leak tracking works for non-web, and', () { testWidgetsWithLeakTracking( - 'Leak tracker respects all allow lists', + 'respects all allow lists', (WidgetTester tester) async { await tester.pumpWidget(_StatelessLeakingWidget()); }, @@ -41,7 +54,41 @@ Future main() async { ), ); - group('Leak tracker respects notGCed allow lists', () { + testWidgetsWithLeakTracking( + 'respects count in allow lists', + (WidgetTester tester) async { + await tester.pumpWidget(_StatelessLeakingWidget()); + }, + leakTrackingConfig: LeakTrackingTestConfig( + notDisposedAllowList: {_leakTrackedClassName: 1}, + notGCedAllowList: {_leakTrackedClassName: 1}, + ), + ); + + group('fails if number or leaks is more than allowed', () { + // This test cannot run inside other tests because test nesting is forbidden. + // So, `expect` happens outside the tests, in `tearDown`. + late Leaks leaks; + + testWidgetsWithLeakTracking( + 'for $_StatelessLeakingWidget', + (WidgetTester tester) async { + await tester.pumpWidget(_StatelessLeakingWidget()); + await tester.pumpWidget(_StatelessLeakingWidget()); + }, + leakTrackingConfig: LeakTrackingTestConfig( + onLeaks: (Leaks theLeaks) { + leaks = theLeaks; + }, + failTestOnLeaks: false, + notDisposedAllowList: {_leakTrackedClassName: 1}, + ), + ); + + tearDown(() => _verifyLeaks(leaks, expectedNotDisposed: 2)); + }); + + group('respects notGCed allow lists', () { // These tests cannot run inside other tests because test nesting is forbidden. // So, `expect` happens outside the tests, in `tearDown`. late Leaks leaks; @@ -63,8 +110,8 @@ Future main() async { tearDown(() => _verifyLeaks(leaks, expectedNotDisposed: 1)); }); - group('Leak tracker catches that', () { - // These tests cannot run inside other tests because test nesting is forbidden. + group('catches that', () { + // These test cannot run inside other tests because test nesting is forbidden. // So, `expect` happens outside the tests, in `tearDown`. late Leaks leaks;