Skip to content

Commit

Permalink
[go_router] Don't log if hierarchicalLoggingEnabled is true (#6019)
Browse files Browse the repository at this point in the history
Fixes flutter/flutter#139667

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
  • Loading branch information
ValentinVignal authored Apr 29, 2024
1 parent 8f35774 commit 9aa04eb
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 13 deletions.
4 changes: 4 additions & 0 deletions packages/go_router/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 14.0.2

- Fixes unwanted logs when `hierarchicalLoggingEnabled` was set to `true`.

## 14.0.1

- Updates the redirection documentation for clarity
Expand Down
34 changes: 23 additions & 11 deletions packages/go_router/lib/src/logging.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ StreamSubscription<LogRecord>? _subscription;
void setLogging({bool enabled = false}) {
_subscription?.cancel();
_enabled = enabled;
if (!enabled) {
if (!enabled || hierarchicalLoggingEnabled) {
return;
}

Expand All @@ -47,16 +47,28 @@ void setLogging({bool enabled = false}) {
),
);
} else {
developer.log(
e.message,
time: e.time,
sequenceNumber: e.sequenceNumber,
level: e.level.value,
name: e.loggerName,
zone: e.zone,
error: e.error,
stackTrace: e.stackTrace,
);
_developerLogFunction(e);
}
});
}

void _developerLog(LogRecord record) {
developer.log(
record.message,
time: record.time,
sequenceNumber: record.sequenceNumber,
level: record.level.value,
name: record.loggerName,
zone: record.zone,
error: record.error,
stackTrace: record.stackTrace,
);
}

/// A function that can be set during test to mock the developer log function.
@visibleForTesting
void Function(LogRecord)? testDeveloperLog;

/// The function used to log messages.
void Function(LogRecord) get _developerLogFunction =>
testDeveloperLog ?? _developerLog;
2 changes: 1 addition & 1 deletion packages/go_router/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: go_router
description: A declarative router for Flutter based on Navigation 2 supporting
deep linking, data-driven routes and more
version: 14.0.1
version: 14.0.2
repository: https://github.com/flutter/packages/tree/main/packages/go_router
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+go_router%22

Expand Down
51 changes: 50 additions & 1 deletion packages/go_router/test/logging_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ import 'package:go_router/src/logging.dart';
import 'package:logging/logging.dart';

void main() {
tearDown(() {
// Reset the logging state
hierarchicalLoggingEnabled = false;

// Reset the developer log function.
testDeveloperLog = null;
});
test('setLogging does not clear listeners', () {
final StreamSubscription<LogRecord> subscription = logger.onRecord.listen(
expectAsync1<void, LogRecord>((LogRecord r) {}, count: 2),
Expand All @@ -26,6 +33,7 @@ void main() {
testWidgets(
'It should not log anything the if debugLogDiagnostics is false',
(WidgetTester tester) async {
testDeveloperLog = expectAsync1((LogRecord data) {}, count: 0);
final StreamSubscription<LogRecord> subscription =
Logger.root.onRecord.listen(
expectAsync1((LogRecord data) {}, count: 0),
Expand All @@ -43,8 +51,48 @@ void main() {
);

testWidgets(
'It should not log the known routes and the initial route if debugLogDiagnostics is true',
'It should log the known routes and the initial route if debugLogDiagnostics is true',
(WidgetTester tester) async {
testDeveloperLog = expectAsync1(
(LogRecord data) {},
count: 2,
reason: 'Go router should log the 2 events',
);
final List<String> logs = <String>[];
Logger.root.onRecord.listen(
(LogRecord event) => logs.add(event.message),
);
GoRouter(
debugLogDiagnostics: true,
routes: <RouteBase>[
GoRoute(
path: '/',
builder: (_, GoRouterState state) => const Text('home'),
),
],
);

expect(
logs,
const <String>[
'Full paths for routes:\n => /\n',
'setting initial location null'
],
reason: 'Go router should have sent the 2 events to the logger',
);
},
);

testWidgets(
'Go router should not log itself the known routes but send the events to the logger when hierarchicalLoggingEnabled is true',
(WidgetTester tester) async {
testDeveloperLog = expectAsync1(
(LogRecord data) {},
count: 0,
reason: 'Go router should log the events itself',
);
hierarchicalLoggingEnabled = true;

final List<String> logs = <String>[];
Logger.root.onRecord.listen(
(LogRecord event) => logs.add(event.message),
Expand All @@ -65,6 +113,7 @@ void main() {
'Full paths for routes:\n => /\n',
'setting initial location null'
],
reason: 'Go router should have sent the 2 events to the logger',
);
},
);
Expand Down

0 comments on commit 9aa04eb

Please sign in to comment.