Skip to content

Commit d520b64

Browse files
authored
Respect allowlisted count of leaks. (#128823)
Contributes to dart-lang/leak_tracker#59
1 parent 944d6c8 commit d520b64

File tree

2 files changed

+86
-9
lines changed

2 files changed

+86
-9
lines changed

packages/flutter/test/foundation/leak_tracking.dart

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
import 'dart:core';
6+
57
import 'package:flutter/foundation.dart';
68
import 'package:flutter_test/flutter_test.dart';
79
import 'package:leak_tracker/leak_tracker.dart';
@@ -143,28 +145,56 @@ class LeakCleaner {
143145

144146
final LeakTrackingTestConfig config;
145147

148+
static Map<(String, LeakType), int> _countByClassAndType(Leaks leaks) {
149+
final Map<(String, LeakType), int> result = <(String, LeakType), int>{};
150+
151+
for (final MapEntry<LeakType, List<LeakReport>> entry in leaks.byType.entries) {
152+
for (final LeakReport leak in entry.value) {
153+
final (String, LeakType) classAndType = (leak.type, entry.key);
154+
result[classAndType] = (result[classAndType] ?? 0) + 1;
155+
}
156+
}
157+
return result;
158+
}
159+
146160
Leaks clean(Leaks leaks) {
161+
final Map<(String, LeakType), int> countByClassAndType = _countByClassAndType(leaks);
162+
147163
final Leaks result = Leaks(<LeakType, List<LeakReport>>{
148164
for (final LeakType leakType in leaks.byType.keys)
149-
leakType: leaks.byType[leakType]!.where((LeakReport leak) => _shouldReportLeak(leakType, leak)).toList()
165+
leakType: leaks.byType[leakType]!.where((LeakReport leak) => _shouldReportLeak(leakType, leak, countByClassAndType)).toList()
150166
});
151167
return result;
152168
}
153169

154170
/// Returns true if [leak] should be reported as failure.
155-
bool _shouldReportLeak(LeakType leakType, LeakReport leak) {
171+
bool _shouldReportLeak(LeakType leakType, LeakReport leak, Map<(String, LeakType), int> countByClassAndType) {
156172
// Tracking for non-GCed is temporarily disabled.
157173
// TODO(polina-c): turn on tracking for non-GCed after investigating existing leaks.
158174
if (leakType != LeakType.notDisposed) {
159175
return false;
160176
}
161177

178+
final String leakingClass = leak.type;
179+
final (String, LeakType) classAndType = (leakingClass, leakType);
180+
181+
bool isAllowedForClass(Map<String, int?> allowList) {
182+
if (!allowList.containsKey(leakingClass)) {
183+
return false;
184+
}
185+
final int? allowedCount = allowList[leakingClass];
186+
if (allowedCount == null) {
187+
return true;
188+
}
189+
return allowedCount >= countByClassAndType[classAndType]!;
190+
}
191+
162192
switch (leakType) {
163193
case LeakType.notDisposed:
164-
return !config.notDisposedAllowList.containsKey(leak.type);
194+
return !isAllowedForClass(config.notDisposedAllowList);
165195
case LeakType.notGCed:
166196
case LeakType.gcedLate:
167-
return !config.notGCedAllowList.containsKey(leak.type);
197+
return !isAllowedForClass(config.notGCedAllowList);
168198
}
169199
}
170200
}

packages/flutter/test/foundation/leak_tracking_test.dart

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,22 @@ Future<void> main() async {
2929
expect(cleanedLeaks.total, 1);
3030
});
3131

32-
group('Leak tracking works for non-web', () {
32+
test('$LeakCleaner catches extra leaks', () {
33+
Leaks leaks = _leaksOfAllTypes();
34+
final LeakReport leak = leaks.notDisposed.first;
35+
leaks.notDisposed.add(leak);
36+
37+
final LeakTrackingTestConfig config = LeakTrackingTestConfig(
38+
notDisposedAllowList: <String, int?>{leak.type: 1},
39+
);
40+
leaks = LeakCleaner(config).clean(leaks);
41+
42+
expect(leaks.notDisposed, hasLength(2));
43+
});
44+
45+
group('Leak tracking works for non-web, and', () {
3346
testWidgetsWithLeakTracking(
34-
'Leak tracker respects all allow lists',
47+
'respects all allow lists',
3548
(WidgetTester tester) async {
3649
await tester.pumpWidget(_StatelessLeakingWidget());
3750
},
@@ -41,7 +54,41 @@ Future<void> main() async {
4154
),
4255
);
4356

44-
group('Leak tracker respects notGCed allow lists', () {
57+
testWidgetsWithLeakTracking(
58+
'respects count in allow lists',
59+
(WidgetTester tester) async {
60+
await tester.pumpWidget(_StatelessLeakingWidget());
61+
},
62+
leakTrackingConfig: LeakTrackingTestConfig(
63+
notDisposedAllowList: <String, int?>{_leakTrackedClassName: 1},
64+
notGCedAllowList: <String, int?>{_leakTrackedClassName: 1},
65+
),
66+
);
67+
68+
group('fails if number or leaks is more than allowed', () {
69+
// This test cannot run inside other tests because test nesting is forbidden.
70+
// So, `expect` happens outside the tests, in `tearDown`.
71+
late Leaks leaks;
72+
73+
testWidgetsWithLeakTracking(
74+
'for $_StatelessLeakingWidget',
75+
(WidgetTester tester) async {
76+
await tester.pumpWidget(_StatelessLeakingWidget());
77+
await tester.pumpWidget(_StatelessLeakingWidget());
78+
},
79+
leakTrackingConfig: LeakTrackingTestConfig(
80+
onLeaks: (Leaks theLeaks) {
81+
leaks = theLeaks;
82+
},
83+
failTestOnLeaks: false,
84+
notDisposedAllowList: <String, int?>{_leakTrackedClassName: 1},
85+
),
86+
);
87+
88+
tearDown(() => _verifyLeaks(leaks, expectedNotDisposed: 2));
89+
});
90+
91+
group('respects notGCed allow lists', () {
4592
// These tests cannot run inside other tests because test nesting is forbidden.
4693
// So, `expect` happens outside the tests, in `tearDown`.
4794
late Leaks leaks;
@@ -63,8 +110,8 @@ Future<void> main() async {
63110
tearDown(() => _verifyLeaks(leaks, expectedNotDisposed: 1));
64111
});
65112

66-
group('Leak tracker catches that', () {
67-
// These tests cannot run inside other tests because test nesting is forbidden.
113+
group('catches that', () {
114+
// These test cannot run inside other tests because test nesting is forbidden.
68115
// So, `expect` happens outside the tests, in `tearDown`.
69116
late Leaks leaks;
70117

0 commit comments

Comments
 (0)