Unverified Commit d520b64c authored by Polina Cherkasova's avatar Polina Cherkasova Committed by GitHub

Respect allowlisted count of leaks. (#128823)

Contributes to https://github.com/dart-lang/leak_tracker/issues/59
parent 944d6c8f
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:core';
import 'package:flutter/foundation.dart'; import 'package:flutter/foundation.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import 'package:leak_tracker/leak_tracker.dart'; import 'package:leak_tracker/leak_tracker.dart';
...@@ -143,28 +145,56 @@ class LeakCleaner { ...@@ -143,28 +145,56 @@ class LeakCleaner {
final LeakTrackingTestConfig config; final LeakTrackingTestConfig config;
static Map<(String, LeakType), int> _countByClassAndType(Leaks leaks) {
final Map<(String, LeakType), int> result = <(String, LeakType), int>{};
for (final MapEntry<LeakType, List<LeakReport>> 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) { Leaks clean(Leaks leaks) {
final Map<(String, LeakType), int> countByClassAndType = _countByClassAndType(leaks);
final Leaks result = Leaks(<LeakType, List<LeakReport>>{ final Leaks result = Leaks(<LeakType, List<LeakReport>>{
for (final LeakType leakType in leaks.byType.keys) 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; return result;
} }
/// Returns true if [leak] should be reported as failure. /// 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. // Tracking for non-GCed is temporarily disabled.
// TODO(polina-c): turn on tracking for non-GCed after investigating existing leaks. // TODO(polina-c): turn on tracking for non-GCed after investigating existing leaks.
if (leakType != LeakType.notDisposed) { if (leakType != LeakType.notDisposed) {
return false; return false;
} }
final String leakingClass = leak.type;
final (String, LeakType) classAndType = (leakingClass, leakType);
bool isAllowedForClass(Map<String, int?> allowList) {
if (!allowList.containsKey(leakingClass)) {
return false;
}
final int? allowedCount = allowList[leakingClass];
if (allowedCount == null) {
return true;
}
return allowedCount >= countByClassAndType[classAndType]!;
}
switch (leakType) { switch (leakType) {
case LeakType.notDisposed: case LeakType.notDisposed:
return !config.notDisposedAllowList.containsKey(leak.type); return !isAllowedForClass(config.notDisposedAllowList);
case LeakType.notGCed: case LeakType.notGCed:
case LeakType.gcedLate: case LeakType.gcedLate:
return !config.notGCedAllowList.containsKey(leak.type); return !isAllowedForClass(config.notGCedAllowList);
} }
} }
} }
...@@ -29,9 +29,22 @@ Future<void> main() async { ...@@ -29,9 +29,22 @@ Future<void> main() async {
expect(cleanedLeaks.total, 1); 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: <String, int?>{leak.type: 1},
);
leaks = LeakCleaner(config).clean(leaks);
expect(leaks.notDisposed, hasLength(2));
});
group('Leak tracking works for non-web, and', () {
testWidgetsWithLeakTracking( testWidgetsWithLeakTracking(
'Leak tracker respects all allow lists', 'respects all allow lists',
(WidgetTester tester) async { (WidgetTester tester) async {
await tester.pumpWidget(_StatelessLeakingWidget()); await tester.pumpWidget(_StatelessLeakingWidget());
}, },
...@@ -41,7 +54,41 @@ Future<void> main() async { ...@@ -41,7 +54,41 @@ Future<void> 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: <String, int?>{_leakTrackedClassName: 1},
notGCedAllowList: <String, int?>{_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: <String, int?>{_leakTrackedClassName: 1},
),
);
tearDown(() => _verifyLeaks(leaks, expectedNotDisposed: 2));
});
group('respects notGCed allow lists', () {
// These tests cannot run inside other tests because test nesting is forbidden. // These tests cannot run inside other tests because test nesting is forbidden.
// So, `expect` happens outside the tests, in `tearDown`. // So, `expect` happens outside the tests, in `tearDown`.
late Leaks leaks; late Leaks leaks;
...@@ -63,8 +110,8 @@ Future<void> main() async { ...@@ -63,8 +110,8 @@ Future<void> main() async {
tearDown(() => _verifyLeaks(leaks, expectedNotDisposed: 1)); tearDown(() => _verifyLeaks(leaks, expectedNotDisposed: 1));
}); });
group('Leak tracker catches that', () { group('catches that', () {
// These tests cannot run inside other tests because test nesting is forbidden. // These test cannot run inside other tests because test nesting is forbidden.
// So, `expect` happens outside the tests, in `tearDown`. // So, `expect` happens outside the tests, in `tearDown`.
late Leaks leaks; late Leaks leaks;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment