Unverified Commit 4e8014bf authored by Polina Cherkasova's avatar Polina Cherkasova Committed by GitHub

Enable not GCed leak tracking. (#130159)

parent c94e6766
...@@ -54,13 +54,13 @@ void testWidgetsWithLeakTracking( ...@@ -54,13 +54,13 @@ void testWidgetsWithLeakTracking(
bool semanticsEnabled = true, bool semanticsEnabled = true,
TestVariant<Object?> variant = const DefaultTestVariant(), TestVariant<Object?> variant = const DefaultTestVariant(),
dynamic tags, dynamic tags,
LeakTrackingTestConfig leakTrackingConfig = const LeakTrackingTestConfig(), LeakTrackingTestConfig leakTrackingTestConfig = const LeakTrackingTestConfig(),
}) { }) {
Future<void> wrappedCallback(WidgetTester tester) async { Future<void> wrappedCallback(WidgetTester tester) async {
await _withFlutterLeakTracking( await _withFlutterLeakTracking(
() async => callback(tester), () async => callback(tester),
tester, tester,
leakTrackingConfig, leakTrackingTestConfig,
); );
} }
...@@ -169,12 +169,6 @@ class LeakCleaner { ...@@ -169,12 +169,6 @@ class LeakCleaner {
/// Returns true if [leak] should be reported as failure. /// Returns true if [leak] should be reported as failure.
bool _shouldReportLeak(LeakType leakType, LeakReport leak, Map<(String, LeakType), int> countByClassAndType) { 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 leakingClass = leak.type;
final (String, LeakType) classAndType = (leakingClass, leakType); final (String, LeakType) classAndType = (leakingClass, leakType);
......
...@@ -18,7 +18,7 @@ Leaks _leaksOfAllTypes() => Leaks(<LeakType, List<LeakReport>> { ...@@ -18,7 +18,7 @@ Leaks _leaksOfAllTypes() => Leaks(<LeakType, List<LeakReport>> {
}); });
Future<void> main() async { Future<void> main() async {
test('Trivial $LeakCleaner returns only non-disposed leaks.', () { test('Trivial $LeakCleaner returns all leaks.', () {
final LeakCleaner leakCleaner = LeakCleaner(const LeakTrackingTestConfig()); final LeakCleaner leakCleaner = LeakCleaner(const LeakTrackingTestConfig());
final Leaks leaks = _leaksOfAllTypes(); final Leaks leaks = _leaksOfAllTypes();
final int leakTotal = leaks.total; final int leakTotal = leaks.total;
...@@ -26,7 +26,7 @@ Future<void> main() async { ...@@ -26,7 +26,7 @@ Future<void> main() async {
final Leaks cleanedLeaks = leakCleaner.clean(leaks); final Leaks cleanedLeaks = leakCleaner.clean(leaks);
expect(leaks.total, leakTotal); expect(leaks.total, leakTotal);
expect(cleanedLeaks.total, 1); expect(cleanedLeaks.total, 3);
}); });
test('$LeakCleaner catches extra leaks', () { test('$LeakCleaner catches extra leaks', () {
...@@ -48,7 +48,7 @@ Future<void> main() async { ...@@ -48,7 +48,7 @@ Future<void> main() async {
(WidgetTester tester) async { (WidgetTester tester) async {
await tester.pumpWidget(_StatelessLeakingWidget()); await tester.pumpWidget(_StatelessLeakingWidget());
}, },
leakTrackingConfig: LeakTrackingTestConfig( leakTrackingTestConfig: LeakTrackingTestConfig(
notDisposedAllowList: <String, int?>{_leakTrackedClassName: null}, notDisposedAllowList: <String, int?>{_leakTrackedClassName: null},
notGCedAllowList: <String, int?>{_leakTrackedClassName: null}, notGCedAllowList: <String, int?>{_leakTrackedClassName: null},
), ),
...@@ -59,7 +59,7 @@ Future<void> main() async { ...@@ -59,7 +59,7 @@ Future<void> main() async {
(WidgetTester tester) async { (WidgetTester tester) async {
await tester.pumpWidget(_StatelessLeakingWidget()); await tester.pumpWidget(_StatelessLeakingWidget());
}, },
leakTrackingConfig: LeakTrackingTestConfig( leakTrackingTestConfig: LeakTrackingTestConfig(
notDisposedAllowList: <String, int?>{_leakTrackedClassName: 1}, notDisposedAllowList: <String, int?>{_leakTrackedClassName: 1},
notGCedAllowList: <String, int?>{_leakTrackedClassName: 1}, notGCedAllowList: <String, int?>{_leakTrackedClassName: 1},
), ),
...@@ -76,7 +76,7 @@ Future<void> main() async { ...@@ -76,7 +76,7 @@ Future<void> main() async {
await tester.pumpWidget(_StatelessLeakingWidget()); await tester.pumpWidget(_StatelessLeakingWidget());
await tester.pumpWidget(_StatelessLeakingWidget()); await tester.pumpWidget(_StatelessLeakingWidget());
}, },
leakTrackingConfig: LeakTrackingTestConfig( leakTrackingTestConfig: LeakTrackingTestConfig(
onLeaks: (Leaks theLeaks) { onLeaks: (Leaks theLeaks) {
leaks = theLeaks; leaks = theLeaks;
}, },
...@@ -85,7 +85,7 @@ Future<void> main() async { ...@@ -85,7 +85,7 @@ Future<void> main() async {
), ),
); );
tearDown(() => _verifyLeaks(leaks, expectedNotDisposed: 2)); tearDown(() => _verifyLeaks(leaks, expectedNotDisposed: 2, expectedNotGCed: 2, shouldContainDebugInfo: false));
}); });
group('respects notGCed allow lists', () { group('respects notGCed allow lists', () {
...@@ -98,7 +98,7 @@ Future<void> main() async { ...@@ -98,7 +98,7 @@ Future<void> main() async {
(WidgetTester tester) async { (WidgetTester tester) async {
await tester.pumpWidget(_StatelessLeakingWidget()); await tester.pumpWidget(_StatelessLeakingWidget());
}, },
leakTrackingConfig: LeakTrackingTestConfig( leakTrackingTestConfig: LeakTrackingTestConfig(
onLeaks: (Leaks theLeaks) { onLeaks: (Leaks theLeaks) {
leaks = theLeaks; leaks = theLeaks;
}, },
...@@ -107,7 +107,7 @@ Future<void> main() async { ...@@ -107,7 +107,7 @@ Future<void> main() async {
), ),
); );
tearDown(() => _verifyLeaks(leaks, expectedNotDisposed: 1)); tearDown(() => _verifyLeaks(leaks, expectedNotDisposed: 1, shouldContainDebugInfo: false));
}); });
group('catches that', () { group('catches that', () {
...@@ -120,7 +120,7 @@ Future<void> main() async { ...@@ -120,7 +120,7 @@ Future<void> main() async {
(WidgetTester tester) async { (WidgetTester tester) async {
await tester.pumpWidget(_StatelessLeakingWidget()); await tester.pumpWidget(_StatelessLeakingWidget());
}, },
leakTrackingConfig: LeakTrackingTestConfig( leakTrackingTestConfig: LeakTrackingTestConfig(
onLeaks: (Leaks theLeaks) { onLeaks: (Leaks theLeaks) {
leaks = theLeaks; leaks = theLeaks;
}, },
...@@ -128,7 +128,7 @@ Future<void> main() async { ...@@ -128,7 +128,7 @@ Future<void> main() async {
), ),
); );
tearDown(() => _verifyLeaks(leaks, expectedNotDisposed: 1)); tearDown(() => _verifyLeaks(leaks, expectedNotDisposed: 1, expectedNotGCed: 1, shouldContainDebugInfo: false));
}); });
}, },
skip: isBrowser); // [intended] Leak detection is off for web. skip: isBrowser); // [intended] Leak detection is off for web.
...@@ -140,7 +140,12 @@ Future<void> main() async { ...@@ -140,7 +140,12 @@ Future<void> main() async {
} }
/// Verifies [leaks] contains expected number of leaks for [_LeakTrackedClass]. /// Verifies [leaks] contains expected number of leaks for [_LeakTrackedClass].
void _verifyLeaks(Leaks leaks, { int expectedNotDisposed = 0, int expectedNotGCed = 0 }) { void _verifyLeaks(
Leaks leaks, {
int expectedNotDisposed = 0,
int expectedNotGCed = 0,
required bool shouldContainDebugInfo,
}) {
const String linkToLeakTracker = 'https://github.com/dart-lang/leak_tracker'; const String linkToLeakTracker = 'https://github.com/dart-lang/leak_tracker';
expect( expect(
...@@ -152,14 +157,20 @@ void _verifyLeaks(Leaks leaks, { int expectedNotDisposed = 0, int expectedNotGC ...@@ -152,14 +157,20 @@ void _verifyLeaks(Leaks leaks, { int expectedNotDisposed = 0, int expectedNotGC
), ),
); );
_verifyLeakList(leaks.notDisposed, expectedNotDisposed); _verifyLeakList(leaks.notDisposed, expectedNotDisposed, shouldContainDebugInfo);
_verifyLeakList(leaks.notGCed, expectedNotGCed); _verifyLeakList(leaks.notGCed, expectedNotGCed, shouldContainDebugInfo);
} }
void _verifyLeakList(List<LeakReport> list, int expectedCount){ void _verifyLeakList(List<LeakReport> list, int expectedCount, bool shouldContainDebugInfo){
expect(list.length, expectedCount); expect(list.length, expectedCount);
for (final LeakReport leak in list) { for (final LeakReport leak in list) {
if (shouldContainDebugInfo) {
expect(leak.context, isNotEmpty);
} else {
expect(leak.context ?? <String, dynamic>{}, isEmpty);
}
expect(leak.trackedClass, contains(_LeakTrackedClass.library)); expect(leak.trackedClass, contains(_LeakTrackedClass.library));
expect(leak.trackedClass, contains(_leakTrackedClassName)); expect(leak.trackedClass, contains(_leakTrackedClassName));
} }
......
...@@ -201,7 +201,15 @@ void main() { ...@@ -201,7 +201,15 @@ void main() {
await tester.tap(find.text('Another package')); await tester.tap(find.text('Another package'));
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(find.text('Another license'), findsOneWidget); expect(find.text('Another license'), findsOneWidget);
}); },
leakTrackingTestConfig: const LeakTrackingTestConfig(
// TODO(polina-c): remove after fixing
// https://github.com/flutter/flutter/issues/130354
notGCedAllowList: <String, int?>{
'ValueNotifier<_OverlayEntryWidgetState?>':2,
'ValueNotifier<String?>': 1,
},
));
testWidgetsWithLeakTracking('LicensePage control test with all properties', (WidgetTester tester) async { testWidgetsWithLeakTracking('LicensePage control test with all properties', (WidgetTester tester) async {
const FlutterLogo logo = FlutterLogo(); const FlutterLogo logo = FlutterLogo();
...@@ -278,7 +286,15 @@ void main() { ...@@ -278,7 +286,15 @@ void main() {
await tester.tap(find.text('Another package')); await tester.tap(find.text('Another package'));
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(find.text('Another license'), findsOneWidget); expect(find.text('Another license'), findsOneWidget);
}); },
leakTrackingTestConfig: const LeakTrackingTestConfig(
// TODO(polina-c): remove after fixing
// https://github.com/flutter/flutter/issues/130354
notGCedAllowList: <String, int?>{
'ValueNotifier<_OverlayEntryWidgetState?>':2,
'ValueNotifier<String?>': 1,
},
));
testWidgetsWithLeakTracking('_PackageLicensePage title style without AppBarTheme', (WidgetTester tester) async { testWidgetsWithLeakTracking('_PackageLicensePage title style without AppBarTheme', (WidgetTester tester) async {
LicenseRegistry.addLicense(() { LicenseRegistry.addLicense(() {
......
...@@ -10,7 +10,6 @@ ...@@ -10,7 +10,6 @@
library; library;
import 'dart:convert'; import 'dart:convert';
import 'dart:developer';
import 'dart:math'; import 'dart:math';
import 'dart:ui' as ui; import 'dart:ui' as ui;
...@@ -20,6 +19,7 @@ import 'package:flutter/gestures.dart' show DragStartBehavior; ...@@ -20,6 +19,7 @@ import 'package:flutter/gestures.dart' show DragStartBehavior;
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart'; import 'package:flutter/rendering.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import 'package:leak_tracker/leak_tracker.dart';
import 'widget_inspector_test_utils.dart'; import 'widget_inspector_test_utils.dart';
...@@ -241,29 +241,6 @@ extension TextFromString on String { ...@@ -241,29 +241,6 @@ extension TextFromString on String {
} }
} }
/// Forces garbage collection by aggressive memory allocation.
///
/// Copied from internal code of
/// https://github.com/dart-lang/leak_tracker
Future<void> _forceGC() async {
const int gcCycles = 3; // 1 should be enough, but we do 3 to make sure test is not flaky.
final int barrier = reachabilityBarrier;
final List<List<DateTime>> storage = <List<DateTime>>[];
void allocateMemory() {
storage.add(Iterable<DateTime>.generate(10000, (_) => DateTime.now()).toList());
if (storage.length > 100) {
storage.removeAt(0);
}
}
while (reachabilityBarrier < barrier + gcCycles) {
await Future<void>.delayed(Duration.zero);
allocateMemory();
}
}
final List<Object> _weakValueTests = <Object>[1, 1.0, 'hello', true, false, Object(), <int>[3, 4], DateTime(2023)]; final List<Object> _weakValueTests = <Object>[1, 1.0, 'hello', true, false, Object(), <int>[3, 4], DateTime(2023)];
void main() { void main() {
...@@ -331,7 +308,9 @@ class _TestWidgetInspectorService extends TestWidgetInspectorService { ...@@ -331,7 +308,9 @@ class _TestWidgetInspectorService extends TestWidgetInspectorService {
final WeakReference<Object> ref = WeakReference<Object>(someObject); final WeakReference<Object> ref = WeakReference<Object>(someObject);
someObject = null; someObject = null;
await _forceGC();
// 1 should be enough for [fullGcCycles], but it is 3 to make sure tests are not flaky.
await forceGC(fullGcCycles: 3);
expect(ref.target, null); expect(ref.target, null);
}); });
......
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