-
Notifications
You must be signed in to change notification settings - Fork 2
Fix logger subscription cleanup on reconfiguration #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
20ca5a1
99b8c66
54384c9
bbd1fe0
b59d830
9163264
b6f95af
90a4fdd
b866cdd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ | |
| // Suhas Virmani <[email protected]> | ||
| // Max Korbel <[email protected]> | ||
|
|
||
| import 'dart:async'; | ||
| import 'dart:io'; | ||
| import 'package:logging/logging.dart'; | ||
| import 'package:rohd_bridge/rohd_bridge.dart'; | ||
|
|
@@ -68,6 +69,13 @@ abstract class RohdBridgeLogger { | |
| bool continueOnError = false, | ||
| bool enableDebugMesage = false, | ||
| }) { | ||
| // Clean up existing logger subscriptions and file sink | ||
| Logger.root.clearListeners(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry I didn't notice this before, but I think we actually want to capture the |
||
| if (fileSink != null) { | ||
| unawaited(fileSink!.close()); | ||
| } | ||
| fileSink = null; | ||
|
|
||
| RohdBridgeLogger.continueOnError = continueOnError; | ||
| Logger.root.level = rootLevel; | ||
| _printLevel = printLevel; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| // Copyright (C) 2024-2025 Intel Corporation | ||
| // SPDX-License-Identifier: BSD-3-Clause | ||
| // | ||
| // rohd_bridge_logger_test.dart | ||
cnaples79 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // Tests for ROHD Bridge Logger functionality. | ||
|
|
||
| import 'dart:io'; | ||
|
|
||
| import 'package:rohd_bridge/src/rohd_bridge_logger.dart'; | ||
| import 'package:test/test.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'; | ||
|
|
||
| const firstConfigurationMessage = 'First configuration'; | ||
| const secondConfigurationMessage = 'Second configuration'; | ||
|
|
||
| // Configure logger first time | ||
| RohdBridgeLogger.configureLogger(logFile1); | ||
|
|
||
| // Log a message | ||
| 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(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(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(secondConfigurationMessage)); | ||
|
|
||
| // 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) | ||
| expect(firstLogContent, isNot(contains(secondConfigurationMessage))); | ||
| }); | ||
|
|
||
| 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 = <String>[]; | ||
|
|
||
| // Configure logger multiple times | ||
| for (var 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 (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 (var j = 0; j < 3; j++) { | ||
| if (i != j) { | ||
| expect(content, isNot(contains('Message $j'))); | ||
| } | ||
| } | ||
| } | ||
| }); | ||
| }); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.