From 20ca5a17ee947e74fff36725f61e11f77d25d807 Mon Sep 17 00:00:00 2001 From: Chase Naples Date: Mon, 29 Sep 2025 18:59:44 -0400 Subject: [PATCH 1/9] Fix logger subscription cleanup on reconfiguration Clean up existing logger subscriptions and file sink before creating new ones in configureLogger to prevent duplicates and resource leaks. - Clear existing Logger.root listeners before adding new ones - Close existing fileSink before creating new one - Set fileSink to null for clean state Fixes #6 --- lib/src/rohd_bridge_logger.dart | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/src/rohd_bridge_logger.dart b/lib/src/rohd_bridge_logger.dart index d791ee2..91a5411 100644 --- a/lib/src/rohd_bridge_logger.dart +++ b/lib/src/rohd_bridge_logger.dart @@ -68,6 +68,11 @@ abstract class RohdBridgeLogger { bool continueOnError = false, bool enableDebugMesage = false, }) { + // Clean up existing logger subscriptions and file sink + Logger.root.clearListeners(); + fileSink?.close(); + fileSink = null; + RohdBridgeLogger.continueOnError = continueOnError; Logger.root.level = rootLevel; _printLevel = printLevel; From 99b8c66ee93546535c8c8d2efddbcd073b39bdff Mon Sep 17 00:00:00 2001 From: Chase Naples Date: Mon, 29 Sep 2025 19:36:34 -0400 Subject: [PATCH 2/9] Fix CI error and add comprehensive test for logger cleanup - Add meta import and use unawaited() to fix discarded future warning - Create comprehensive test for configureLogger reconfiguration - Test verifies cleanup prevents duplicate logging and resource leaks - Test covers edge cases including null fileSink handling - Test ensures multiple reconfigurations work correctly Addresses maintainer feedback on #6 --- lib/src/rohd_bridge_logger.dart | 5 +- test/rohd_bridge_logger_test.dart | 99 +++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 test/rohd_bridge_logger_test.dart diff --git a/lib/src/rohd_bridge_logger.dart b/lib/src/rohd_bridge_logger.dart index 91a5411..49d0942 100644 --- a/lib/src/rohd_bridge_logger.dart +++ b/lib/src/rohd_bridge_logger.dart @@ -12,6 +12,7 @@ import 'dart:io'; import 'package:logging/logging.dart'; +import 'package:meta/meta.dart'; import 'package:rohd_bridge/rohd_bridge.dart'; /// Extension on [Logger] to throw an exception if an [error] is encountered. @@ -70,7 +71,9 @@ abstract class RohdBridgeLogger { }) { // Clean up existing logger subscriptions and file sink Logger.root.clearListeners(); - fileSink?.close(); + if (fileSink != null) { + unawaited(fileSink!.close()); + } fileSink = null; RohdBridgeLogger.continueOnError = continueOnError; diff --git a/test/rohd_bridge_logger_test.dart b/test/rohd_bridge_logger_test.dart new file mode 100644 index 0000000..8a2162c --- /dev/null +++ b/test/rohd_bridge_logger_test.dart @@ -0,0 +1,99 @@ +// Copyright (C) 2024-2025 Intel Corporation +// SPDX-License-Identifier: BSD-3-Clause +// +// rohd_bridge_logger_test.dart +// Tests for ROHD Bridge Logger functionality. +// +// 2025 +// Author: Claude Code contribution + +import 'dart:io'; +import 'package:test/test.dart'; +import 'package:logging/logging.dart'; +import 'package:rohd_bridge/src/rohd_bridge_logger.dart'; + +void main() { + group('RohdBridgeLogger', () { + late Directory tempDir; + + setUp(() async { + tempDir = await Directory.systemTemp.createTemp('rohd_bridge_test'); + }); + + tearDown(() async { + await RohdBridgeLogger.terminate(); + if (tempDir.existsSync()) { + tempDir.deleteSync(recursive: true); + } + }); + + test('configureLogger cleans up previous subscriptions', () async { + final logFile1 = '${tempDir.path}/test1.log'; + final logFile2 = '${tempDir.path}/test2.log'; + + // Configure logger first time + RohdBridgeLogger.configureLogger(logFile1); + + // Log a message + RohdBridgeLogger.logger.info('First configuration'); + await RohdBridgeLogger.flush(); + + // Reconfigure logger - this should clean up previous subscriptions + RohdBridgeLogger.configureLogger(logFile2); + + // Log another message after reconfiguration + RohdBridgeLogger.logger.info('Second configuration'); + await RohdBridgeLogger.flush(); + + // Verify first log file exists and has content from first configuration + expect(File(logFile1).existsSync(), isTrue); + final firstLogContent = await File(logFile1).readAsString(); + expect(firstLogContent, contains('First configuration')); + + // Verify second log file exists and has content from second configuration + expect(File(logFile2).existsSync(), isTrue); + final secondLogContent = await File(logFile2).readAsString(); + expect(secondLogContent, contains('Second configuration')); + + // Second log should not contain first message (proves no duplicate logging) + expect(secondLogContent, isNot(contains('First configuration'))); + + // First log should not contain second message (proves cleanup worked) + expect(firstLogContent, isNot(contains('Second configuration'))); + }); + + test('configureLogger handles null fileSink gracefully', () { + // This should not throw even when fileSink is initially null + expect(() => RohdBridgeLogger.configureLogger('${tempDir.path}/test.log'), + returnsNormally); + }); + + test('multiple reconfigurations work correctly', () async { + final logFiles = []; + + // Configure logger multiple times + for (int i = 0; i < 3; i++) { + final logFile = '${tempDir.path}/test$i.log'; + logFiles.add(logFile); + + RohdBridgeLogger.configureLogger(logFile); + RohdBridgeLogger.logger.info('Message $i'); + await RohdBridgeLogger.flush(); + } + + // Each log file should exist and contain only its respective message + for (int i = 0; i < 3; i++) { + expect(File(logFiles[i]).existsSync(), isTrue); + final content = await File(logFiles[i]).readAsString(); + expect(content, contains('Message $i')); + + // Should not contain messages from other configurations + for (int j = 0; j < 3; j++) { + if (i != j) { + expect(content, isNot(contains('Message $j'))); + } + } + } + }); + }); +} \ No newline at end of file From 54384c978214a5af8f6bd0e30022aaeeea3ce86c Mon Sep 17 00:00:00 2001 From: Chase Naples Date: Mon, 29 Sep 2025 19:39:44 -0400 Subject: [PATCH 3/9] Update rohd_bridge_logger_test.dart --- test/rohd_bridge_logger_test.dart | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/test/rohd_bridge_logger_test.dart b/test/rohd_bridge_logger_test.dart index 8a2162c..415679f 100644 --- a/test/rohd_bridge_logger_test.dart +++ b/test/rohd_bridge_logger_test.dart @@ -1,11 +1,7 @@ -// Copyright (C) 2024-2025 Intel Corporation -// SPDX-License-Identifier: BSD-3-Clause -// + // rohd_bridge_logger_test.dart // Tests for ROHD Bridge Logger functionality. -// -// 2025 -// Author: Claude Code contribution + import 'dart:io'; import 'package:test/test.dart'; @@ -96,4 +92,4 @@ void main() { } }); }); -} \ No newline at end of file +} From bbd1fe033a1b53eb81e11a102f18c8aecdf5bb53 Mon Sep 17 00:00:00 2001 From: Chase Naples Date: Mon, 29 Sep 2025 19:41:24 -0400 Subject: [PATCH 4/9] Update rohd_bridge_logger_test.dart --- test/rohd_bridge_logger_test.dart | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/rohd_bridge_logger_test.dart b/test/rohd_bridge_logger_test.dart index 415679f..1cbb05f 100644 --- a/test/rohd_bridge_logger_test.dart +++ b/test/rohd_bridge_logger_test.dart @@ -1,8 +1,6 @@ - // rohd_bridge_logger_test.dart // Tests for ROHD Bridge Logger functionality. - import 'dart:io'; import 'package:test/test.dart'; import 'package:logging/logging.dart'; From b59d830ea3ac0c135e1852550cb7d1120da4cb91 Mon Sep 17 00:00:00 2001 From: Chase Naples Date: Mon, 29 Sep 2025 20:14:21 -0400 Subject: [PATCH 5/9] Update rohd_bridge_logger_test.dart Added Header --- test/rohd_bridge_logger_test.dart | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/rohd_bridge_logger_test.dart b/test/rohd_bridge_logger_test.dart index 1cbb05f..33d4743 100644 --- a/test/rohd_bridge_logger_test.dart +++ b/test/rohd_bridge_logger_test.dart @@ -1,3 +1,6 @@ +// Copyright (C) 2024-2025 Intel Corporation +// SPDX-License-Identifier: BSD-3-Clause +// // rohd_bridge_logger_test.dart // Tests for ROHD Bridge Logger functionality. From 91632646eff9e25e4d2880d9dcff7ab09058db76 Mon Sep 17 00:00:00 2001 From: Chase Naples Date: Tue, 30 Sep 2025 18:42:37 -0400 Subject: [PATCH 6/9] Apply dart format to rohd_bridge_logger_test.dart only Fix formatting issues in the newly added test file to pass CI checks. Only format the test file that was added in this PR, not all files. --- test/rohd_bridge_logger_test.dart | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/rohd_bridge_logger_test.dart b/test/rohd_bridge_logger_test.dart index 33d4743..c7d9d78 100644 --- a/test/rohd_bridge_logger_test.dart +++ b/test/rohd_bridge_logger_test.dart @@ -61,8 +61,10 @@ void main() { test('configureLogger handles null fileSink gracefully', () { // This should not throw even when fileSink is initially null - expect(() => RohdBridgeLogger.configureLogger('${tempDir.path}/test.log'), - returnsNormally); + expect( + () => RohdBridgeLogger.configureLogger('${tempDir.path}/test.log'), + returnsNormally, + ); }); test('multiple reconfigurations work correctly', () async { From b6f95afa95bea1486c3d60ce7d50febea9b51fae Mon Sep 17 00:00:00 2001 From: Chase Naples Date: Wed, 1 Oct 2025 17:02:40 -0400 Subject: [PATCH 7/9] Fix logger lint errors --- lib/src/rohd_bridge_logger.dart | 5 +++-- test/rohd_bridge_logger_test.dart | 25 ++++++++++++++----------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/lib/src/rohd_bridge_logger.dart b/lib/src/rohd_bridge_logger.dart index 49d0942..b549ab8 100644 --- a/lib/src/rohd_bridge_logger.dart +++ b/lib/src/rohd_bridge_logger.dart @@ -10,9 +10,9 @@ // Suhas Virmani // Max Korbel +import 'dart:async'; import 'dart:io'; import 'package:logging/logging.dart'; -import 'package:meta/meta.dart'; import 'package:rohd_bridge/rohd_bridge.dart'; /// Extension on [Logger] to throw an exception if an [error] is encountered. @@ -83,7 +83,8 @@ abstract class RohdBridgeLogger { fileSink = File(filePath).openWrite(); Logger.root.onRecord.listen((record) { - final message = '${record.time}: ${record.level.name}: ' + final message = + '${record.time}: ${record.level.name}: ' '${record.message}\n'; _handleMessage(message, record.level); diff --git a/test/rohd_bridge_logger_test.dart b/test/rohd_bridge_logger_test.dart index c7d9d78..d8b9fcb 100644 --- a/test/rohd_bridge_logger_test.dart +++ b/test/rohd_bridge_logger_test.dart @@ -5,9 +5,9 @@ // Tests for ROHD Bridge Logger functionality. import 'dart:io'; -import 'package:test/test.dart'; -import 'package:logging/logging.dart'; + import 'package:rohd_bridge/src/rohd_bridge_logger.dart'; +import 'package:test/test.dart'; void main() { group('RohdBridgeLogger', () { @@ -28,35 +28,38 @@ void main() { final logFile1 = '${tempDir.path}/test1.log'; final logFile2 = '${tempDir.path}/test2.log'; + const firstConfigurationMessage = 'First configuration'; + const secondConfigurationMessage = 'Second configuration'; + // Configure logger first time RohdBridgeLogger.configureLogger(logFile1); // Log a message - RohdBridgeLogger.logger.info('First configuration'); + RohdBridgeLogger.logger.info(firstConfigurationMessage); await RohdBridgeLogger.flush(); // Reconfigure logger - this should clean up previous subscriptions RohdBridgeLogger.configureLogger(logFile2); // Log another message after reconfiguration - RohdBridgeLogger.logger.info('Second configuration'); + RohdBridgeLogger.logger.info(secondConfigurationMessage); await RohdBridgeLogger.flush(); // Verify first log file exists and has content from first configuration expect(File(logFile1).existsSync(), isTrue); final firstLogContent = await File(logFile1).readAsString(); - expect(firstLogContent, contains('First configuration')); + expect(firstLogContent, contains(firstConfigurationMessage)); // Verify second log file exists and has content from second configuration expect(File(logFile2).existsSync(), isTrue); final secondLogContent = await File(logFile2).readAsString(); - expect(secondLogContent, contains('Second configuration')); + expect(secondLogContent, contains(secondConfigurationMessage)); // Second log should not contain first message (proves no duplicate logging) - expect(secondLogContent, isNot(contains('First configuration'))); + expect(secondLogContent, isNot(contains(firstConfigurationMessage))); // First log should not contain second message (proves cleanup worked) - expect(firstLogContent, isNot(contains('Second configuration'))); + expect(firstLogContent, isNot(contains(secondConfigurationMessage))); }); test('configureLogger handles null fileSink gracefully', () { @@ -71,7 +74,7 @@ void main() { final logFiles = []; // Configure logger multiple times - for (int i = 0; i < 3; i++) { + for (var i = 0; i < 3; i++) { final logFile = '${tempDir.path}/test$i.log'; logFiles.add(logFile); @@ -81,13 +84,13 @@ void main() { } // Each log file should exist and contain only its respective message - for (int i = 0; i < 3; i++) { + for (var i = 0; i < 3; i++) { expect(File(logFiles[i]).existsSync(), isTrue); final content = await File(logFiles[i]).readAsString(); expect(content, contains('Message $i')); // Should not contain messages from other configurations - for (int j = 0; j < 3; j++) { + for (var j = 0; j < 3; j++) { if (i != j) { expect(content, isNot(contains('Message $j'))); } From 90a4fdd4a8b261e450722a36e68ee5c817553912 Mon Sep 17 00:00:00 2001 From: Chase Naples Date: Wed, 1 Oct 2025 21:05:04 -0400 Subject: [PATCH 8/9] style: apply dart format to rohd_bridge_logger.dart --- lib/src/rohd_bridge_logger.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/src/rohd_bridge_logger.dart b/lib/src/rohd_bridge_logger.dart index b549ab8..9573a5b 100644 --- a/lib/src/rohd_bridge_logger.dart +++ b/lib/src/rohd_bridge_logger.dart @@ -83,8 +83,7 @@ abstract class RohdBridgeLogger { fileSink = File(filePath).openWrite(); Logger.root.onRecord.listen((record) { - final message = - '${record.time}: ${record.level.name}: ' + final message = '${record.time}: ${record.level.name}: ' '${record.message}\n'; _handleMessage(message, record.level); From b866cddfcd73c932f5ab7e050b216f79871badc4 Mon Sep 17 00:00:00 2001 From: Chase Naples Date: Thu, 2 Oct 2025 19:50:01 -0400 Subject: [PATCH 9/9] style: fix line length to meet 80-character limit --- test/rohd_bridge_logger_test.dart | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/rohd_bridge_logger_test.dart b/test/rohd_bridge_logger_test.dart index d8b9fcb..e057141 100644 --- a/test/rohd_bridge_logger_test.dart +++ b/test/rohd_bridge_logger_test.dart @@ -55,10 +55,12 @@ void main() { final secondLogContent = await File(logFile2).readAsString(); expect(secondLogContent, contains(secondConfigurationMessage)); - // Second log should not contain first message (proves no duplicate logging) + // Second log should not contain first message + // (proves no duplicate logging) expect(secondLogContent, isNot(contains(firstConfigurationMessage))); - // First log should not contain second message (proves cleanup worked) + // First log should not contain second message + // (proves cleanup worked) expect(firstLogContent, isNot(contains(secondConfigurationMessage))); });