From 9aa04eb88618b9c10efe49fef1a36ff3ed6c0af5 Mon Sep 17 00:00:00 2001
From: Valentin Vignal <32538273+ValentinVignal@users.noreply.github.com>
Date: Tue, 30 Apr 2024 06:22:19 +0800
Subject: [PATCH] [go_router] Don't log if `hierarchicalLoggingEnabled` is
 `true`  (#6019)

Fixes https://github.com/flutter/flutter/issues/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].*
---
 packages/go_router/CHANGELOG.md           |  4 ++
 packages/go_router/lib/src/logging.dart   | 34 ++++++++++-----
 packages/go_router/pubspec.yaml           |  2 +-
 packages/go_router/test/logging_test.dart | 51 ++++++++++++++++++++++-
 4 files changed, 78 insertions(+), 13 deletions(-)

diff --git a/packages/go_router/CHANGELOG.md b/packages/go_router/CHANGELOG.md
index 1405fe78e38..c552046d4b6 100644
--- a/packages/go_router/CHANGELOG.md
+++ b/packages/go_router/CHANGELOG.md
@@ -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
diff --git a/packages/go_router/lib/src/logging.dart b/packages/go_router/lib/src/logging.dart
index 7f0a8ce5a7a..4f116422181 100644
--- a/packages/go_router/lib/src/logging.dart
+++ b/packages/go_router/lib/src/logging.dart
@@ -28,7 +28,7 @@ StreamSubscription<LogRecord>? _subscription;
 void setLogging({bool enabled = false}) {
   _subscription?.cancel();
   _enabled = enabled;
-  if (!enabled) {
+  if (!enabled || hierarchicalLoggingEnabled) {
     return;
   }
 
@@ -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;
diff --git a/packages/go_router/pubspec.yaml b/packages/go_router/pubspec.yaml
index bd53a5f18a2..664fa43d782 100644
--- a/packages/go_router/pubspec.yaml
+++ b/packages/go_router/pubspec.yaml
@@ -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
 
diff --git a/packages/go_router/test/logging_test.dart b/packages/go_router/test/logging_test.dart
index 3ae24561751..1303f2909db 100644
--- a/packages/go_router/test/logging_test.dart
+++ b/packages/go_router/test/logging_test.dart
@@ -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),
@@ -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),
@@ -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),
@@ -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',
       );
     },
   );