Skip to content

Commit

Permalink
Avoid duplicate leak tracking and remove dependency on logging. (dart…
Browse files Browse the repository at this point in the history
  • Loading branch information
polina-c authored Oct 24, 2023
1 parent c414aff commit 3b666af
Show file tree
Hide file tree
Showing 10 changed files with 34 additions and 79 deletions.
5 changes: 5 additions & 0 deletions pkgs/leak_tracker/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
# 9.0.11

* Remove dependency on logging.
* Avoid double leak tracking.

# 9.0.10

* Use `IgnoredLeaks`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,9 @@
import 'dart:async';
import 'dart:developer';

import 'package:logging/logging.dart';
import 'package:vm_service/vm_service.dart';
import 'package:web_socket_channel/web_socket_channel.dart';

final _log = Logger('_connection.dart');

Future<Uri> _serviceUri() async {
Uri? uri = (await Service.getInfo()).serverWebSocketUri;

Expand All @@ -31,19 +28,15 @@ Future<Uri> _serviceUri() async {
///
/// If the VM service is not found, tries to start it.
Future<VmService> connect() async {
_log.info('Connecting to VM service protocol...');

final uri = await _serviceUri();

final service = await _connectWithWebSocket(uri, _handleError);
await service.getVersion(); // Warming up and validating the connection.

_log.info('Connected to vm service protocol.');
return service;
}

void _handleError(Object? error) {
_log.info('Error in vm service protocol: $error');
throw error ?? Exception('Unknown error');
}

Expand Down
6 changes: 0 additions & 6 deletions pkgs/leak_tracker/lib/src/leak_tracking/helpers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,10 @@
import 'dart:async';
import 'dart:developer';

import 'package:logging/logging.dart';

import '../shared/_formatting.dart';
import '_primitives/_retaining_path/_connection.dart';
import '_primitives/_retaining_path/_retaining_path.dart';

final _log = Logger('orchestration.dart');

/// Forces garbage collection by aggressive memory allocation.
///
/// Verifies that garbage collection happened using [reachabilityBarrier].
Expand All @@ -30,7 +26,6 @@ Future<void> forceGC({
Duration? timeout,
int fullGcCycles = 1,
}) async {
_log.info('Forcing garbage collection with fullGcCycles = $fullGcCycles...');
final Stopwatch? stopwatch = timeout == null ? null : (Stopwatch()..start());
final int barrier = reachabilityBarrier;

Expand All @@ -50,7 +45,6 @@ Future<void> forceGC({
await Future<void>.delayed(Duration.zero);
allocateMemory();
}
_log.info('Done forcing garbage collection.');
}

/// Returns nicely formatted retaining path for the [ref.target].
Expand Down
6 changes: 0 additions & 6 deletions pkgs/leak_tracker/lib/src/leak_tracking/leak_tracking.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// for details. 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:logging/logging.dart';

import '../devtools_integration/_registration.dart';
import '../shared/_primitives.dart';
import '../shared/shared_model.dart';
Expand All @@ -12,8 +10,6 @@ import '_leak_tracker.dart';
import '_primitives/_dispatcher.dart' as dispatcher;
import '_primitives/model.dart';

final _log = Logger('leak_tracking.dart');

/// Provides leak tracking functionality.
abstract class LeakTracking {
static Baseliner? _baseliner;
Expand Down Expand Up @@ -81,7 +77,6 @@ abstract class LeakTracking {
} else {
registerLeakTrackingServiceExtension();
}
_log.info('started leak tracking');
return true;
}());
}
Expand All @@ -94,7 +89,6 @@ abstract class LeakTracking {
_leakTracker?.dispose();
_leakTracker = null;
Baseliner.finishOldAndStartNew(_baseliner, null);
_log.info('stopped leak tracking');
return true;
}());
}
Expand Down
3 changes: 1 addition & 2 deletions pkgs/leak_tracker/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: leak_tracker
version: 9.0.10
version: 9.0.11
description: A framework for memory leak tracking for Dart and Flutter applications.
repository: https://github.com/dart-lang/leak_tracker/tree/main/pkgs/leak_tracker

Expand All @@ -10,7 +10,6 @@ dependencies:
clock: ^1.1.1
collection: ^1.15.0
intl: ^0.18.0
logging: ^1.1.1
meta: ^1.8.0
path: ^1.8.3
vm_service: '>=11.10.0 <13.0.0'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import 'dart:async';

import 'package:leak_tracker/src/leak_tracking/_primitives/_retaining_path/_connection.dart';
import 'package:leak_tracker/src/leak_tracking/_primitives/_retaining_path/_retaining_path.dart';
import 'package:logging/logging.dart';
import 'package:test/test.dart';

class MyClass {
Expand All @@ -17,23 +16,7 @@ class MyArgClass<T> {
MyArgClass();
}

final _logs = <String>[];
late StreamSubscription<LogRecord> subscription;

void main() {
setUpAll(() {
subscription = Logger.root.onRecord
.listen((LogRecord record) => _logs.add(record.message));
});

setUp(() {
_logs.clear();
});

tearDownAll(() async {
await subscription.cancel();
});

test('Path for $MyClass instance is found.', () async {
final instance = MyClass();
final connection = await connect();
Expand Down Expand Up @@ -67,11 +50,8 @@ void main() {
retainingPath(connection, instance2),
];

await Future.wait(obtainers);
final result = await Future.wait(obtainers);

expect(
_logs.where((item) => item == 'Connecting to VM service protocol...'),
hasLength(1),
);
expect(result.where((p) => p == null), hasLength(0));
});
}
4 changes: 4 additions & 0 deletions pkgs/leak_tracker_flutter_testing/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# 1.0.9

* Update `testWidgetsWithLeakTracking` to avoid duplicated leak tracking by testWidgets.

# 1.0.8

* Make configuration adjustable.
Expand Down
28 changes: 18 additions & 10 deletions pkgs/leak_tracker_flutter_testing/lib/src/test_widgets.dart
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ Future<void> _tearDownTestingWithLeakTracking(LeaksCallback? onLeaks) async {
}
}

// TODO(polina-c): retire this class after migration to `testWidgets`.
// TODO(polina-c): retire this function after migration to `testWidgets`.
// https://github.com/flutter/flutter/issues/135856
/// Wrapper for [testWidgets] with memory leak tracking.
///
Expand Down Expand Up @@ -111,15 +111,23 @@ void testWidgetsWithLeakTracking(
LeakTracking.phase = const PhaseSettings.ignored();
}

testWidgets(
description,
wrappedCallBack,
skip: skip,
timeout: timeout,
semanticsEnabled: semanticsEnabled,
variant: variant,
tags: tags,
);
// Temporarily turn off leak tracking, so that `testWidgests` does not track leaks in addition.
final originalGlobalSettings = LeakTesting.settings;
LeakTesting.settings = LeakTesting.settings.withIgnoredAll();

try {
testWidgets(
description,
wrappedCallBack,
skip: skip,
timeout: timeout,
semanticsEnabled: semanticsEnabled,
variant: variant,
tags: tags,
);
} finally {
LeakTesting.settings = originalGlobalSettings;
}
}

bool _notSupportedWarningPrinted = false;
Expand Down
3 changes: 1 addition & 2 deletions pkgs/leak_tracker_flutter_testing/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: leak_tracker_flutter_testing
version: 1.0.8
version: 1.0.9
description: Flutter specific helpers for dart memory leak tracking.
repository: https://github.com/dart-lang/leak_tracker/tree/main/pkgs/leak_tracker_flutter_testing

Expand All @@ -17,7 +17,6 @@ dependencies:

dev_dependencies:
flutter_lints: ^3.0.0
logging: ^1.1.1

dependency_overrides:
leak_tracker:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,9 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:async';

import 'package:flutter_test/flutter_test.dart';
import 'package:leak_tracker/src/leak_tracking/_primitives/_retaining_path/_connection.dart';
import 'package:leak_tracker/src/leak_tracking/_primitives/_retaining_path/_retaining_path.dart';
import 'package:logging/logging.dart';

// We duplicate testing for retaining path here,
// because there were cases when the tests were passing for dart,
Expand All @@ -21,23 +18,7 @@ class MyArgClass<T> {
MyArgClass();
}

final _logs = <String>[];
late StreamSubscription<LogRecord> subscription;

void main() {
setUpAll(() {
subscription = Logger.root.onRecord
.listen((LogRecord record) => _logs.add(record.message));
});

setUp(() {
_logs.clear();
});

tearDownAll(() async {
await subscription.cancel();
});

testWidgets('Path for $MyClass instance is found.',
(WidgetTester tester) async {
final instance = MyClass();
Expand Down Expand Up @@ -75,11 +56,9 @@ void main() {
retainingPath(connection, instance2),
];

await Future.wait(obtainers);
final paths = await Future.wait(obtainers);

expect(
_logs.where((item) => item == 'Connecting to VM service protocol...'),
hasLength(1),
);
expect(paths, hasLength(2));
expect(paths.where((p) => p == null).toList(), hasLength(0));
});
}

0 comments on commit 3b666af

Please sign in to comment.