Add testing for ServerLogging.java. Configure ServerLogging.java for tests.

[bamsemats]

Closes #30

Summary
This PR improves the testability and robustness of the ServerLogging utility and adds comprehensive unit tests to verify its behavior.

Tests added
The following aspects are now covered by ServerLoggingTest:

Verifies that getLogger() returns the same logger instance (singleton behavior)

Ensures a ConsoleHandler is configured on the logger

Confirms that duplicate handlers are not added on repeated access

Asserts that the default log level is INFO

Verifies that parent handlers are disabled

Ensures the log level can be configured via the log.level system property in an isolated and deterministic way

Changes to ServerLogging
Extracted logger configuration into a separate configure(Logger logger) method

Prevents duplicate handler registration

Decouples configuration logic from static initialization, making the class easier to test and safer to reuse

Result
The logging setup is now deterministic, testable, and more resilient to future changes.

Summary by CodeRabbit

• Tests

  • Added comprehensive tests for logger behavior: singleton consistency, handler management (including avoiding duplicates), default log level, parent-handler usage, and system-property-driven level configuration.

• Refactor

  • Reorganized logger initialization into a dedicated configuration routine, added validation of configured log level with a fallback and warning on invalid values, and cleaned up class structure for clearer internal organization.

[coderabbitai]

📝 Walkthrough

Walkthrough

Extracts logger setup from a static initializer into a package-private configure(Logger) method in ServerLogging, adds validation for log.level input and ensures a ConsoleHandler when none exist, and introduces JUnit 5 tests validating logger reuse, handler management, levels, and parent-handler behavior.

Changes

Cohort / File(s)	Summary
Logger Configuration Refactor src/main/java/org/juv25d/logging/ServerLogging.java	Moved static-block logger initialization into a package-private configure(Logger) method; sets useParentHandlers=false, ensures a ConsoleHandler with SimpleFormatter if no handlers exist, reads log.level from system property or LOG_LEVEL env var (uppercased) and parses to Level, and catches invalid level values to default to INFO. Minor formatting and constructor compaction.
Logger Tests src/test/java/org/juv25d/logging/ServerLoggingTest.java	Adds JUnit 5 tests that reset handlers between runs and assert: singleton logger instance, console handler presence, no duplicate handlers on repeated config calls, default level INFO, parent handlers disabled, and respect for log.level system property (with restoration).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nudged the logger, tidy and spry,
Pulled setup to a method, gave levels a try.
Handlers lined up, no echoes to chase,
Tests thump their feet in a neat little race.
✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main changes: adding tests for ServerLogging and refactoring it for testability.
Linked Issues check	✅ Passed	The PR implements comprehensive unit tests for ServerLogging covering singleton behavior, handler configuration, log levels, and parent handlers, directly addressing issue #30.
Out of Scope Changes check	✅ Passed	All changes are directly related to testing ServerLogging and refactoring it for testability. No unrelated modifications detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feature/serverlogging-test

---

No actionable comments were generated in the recent review. 🎉

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@src/main/java/org/juv25d/logging/ServerLogging.java`:
- Around line 16-32: The Level.parse call in configure can throw
IllegalArgumentException for invalid level strings; wrap the parse and
logger.setLevel calls in a try-catch that catches IllegalArgumentException (and
optionally NullPointerException), fallback to Level.INFO when parsing fails, and
emit a warning via the provided Logger (e.g., logger.warning or logger.log)
describing the invalid levelName; ensure this change is applied inside the
static configure(Logger logger) method around Level.parse(...) and
logger.setLevel(...).

In `@src/test/java/org/juv25d/logging/ServerLoggingTest.java`:
- Around line 76-82: The test is brittle because it relies on global state;
instead create an isolated logger and explicitly control properties: use
Logger.getLogger("test.default.level") to obtain a fresh logger, clear the
system property "log.level" and unset any LOG_LEVEL env var (or mock/ensure it's
absent), then call ServerLogging.configure() (or the configure method used in
other tests) to initialize defaults, and finally assert that logger.getLevel()
equals Level.INFO; reference ServerLogging.getLogger(),
ServerLogging.configure(), and the "log.level"/LOG_LEVEL settings when making
these changes.
- Around line 62-74: The test currently calls ServerLogging.getLogger() twice
which returns the same static logger and doesn't exercise the duplicate-handler
guard; update the test to obtain the logger once (Logger logger =
ServerLogging.getLogger()), then call ServerLogging.configure(logger) twice and
assert that logger.getHandlers().length remains 1 (or equals the initial handler
count) to verify the if (logger.getHandlers().length == 0) protection in
ServerLogging.configure.

🧹 Nitpick comments (1)

src/test/java/org/juv25d/logging/ServerLoggingTest.java (1)

18-27: setUp removes handlers but doesn't re-apply configure().

After stripping all handlers, tests that don't explicitly call configure() (e.g., logger_shouldHaveInfoLevelByDefault, logger_shouldNotUseParentHandlers) rely on state left over from the static initializer rather than a known-good baseline. This is fragile — consider calling ServerLogging.configure(logger) at the end of setUp() so every test starts from a fully configured state, or intentionally leave it unconfigured and call configure() explicitly in each test that needs it.