From 202e37e3b3bc8039cb8ed09431cabb60ef3d4732 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 12 Sep 2026 13:36:55 +0700 Subject: [PATCH] fleetd #537: pin CapturedLog.close()'s appender-detach and setLevel-immunity contracts Only the level-restore half of close() was pinned before this (WorktreeSessionManagerTest). Deleting logger.detachAppender(appender) from close() left mvn clean install green (1701 tests, 0 failures) -- the appender-detach half of the contract was unmeasured. Adds CapturedLogTest with three tests, each using a logger name no production class uses: - closeDetachesTheAppenderSoALaterLogIsNotCaptured: an event logged after close() must not land in events(). - closeRestoresTheLevelCapturedAtOpen: the helper's headline contract in one place, independent of any production class. - setLevelDuringCaptureDoesNotChangeWhatCloseRestores: setLevel()'s own javadoc claim that close() always restores the level captured at construction, never a value set through setLevel() mid-capture. Test-only change; CapturedLog.java itself is untouched. --- .../ltms/fleet/testing/CapturedLogTest.java | 96 +++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 fleetd/src/test/java/dev/ltms/fleet/testing/CapturedLogTest.java diff --git a/fleetd/src/test/java/dev/ltms/fleet/testing/CapturedLogTest.java b/fleetd/src/test/java/dev/ltms/fleet/testing/CapturedLogTest.java new file mode 100644 index 0000000..456b8a5 --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/testing/CapturedLogTest.java @@ -0,0 +1,96 @@ +package dev.ltms.fleet.testing; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * fleetd #537: pins {@link CapturedLog#close}'s own contract — the appender detach half, the level + * restore half, and that {@link CapturedLog#setLevel} does not change what {@code close} restores. + * Before this, only the level-restore half was pinned (by {@code + * WorktreeSessionManagerTest.sharedSessionManagerLoggerLevelIsRestoredAfterDirtyWorktreeReleasePinsWarn}). + * Measured: deleting {@code logger.detachAppender(appender);} from {@code close()} still left + * {@code mvn clean install} green — 1701 tests, 0 failures — before this file existed. + * + *

Every test below uses a logger name no production class uses, and unique per test, so this + * file cannot become the next entry in fleetd #525's leak family: {@link CapturedLog}'s own + * javadoc re-measure commands (top of that file) would otherwise need to start naming this class. + */ +class CapturedLogTest { + + /** + * The appender must be detached on close: an event logged through the raw logger after close + * must not land in {@link CapturedLog#events()}. Asserting on the observable list (rather than + * {@code logger.iteratorForAppenders()}) is what the ticket asked for, and it is also what a + * real leak would actually break — a later test's own {@code ListAppender} silently gaining + * events emitted by code under test that has nothing to do with it. + */ + @Test + void closeDetachesTheAppenderSoALaterLogIsNotCaptured() { + String loggerName = "capturedlog-test-only.appender-detach"; + Logger rawLogger = (Logger) LoggerFactory.getLogger(loggerName); + + CapturedLog log = CapturedLog.of(loggerName); + rawLogger.info("while open"); + int eventsWhileOpen = log.events().size(); + assertEquals(1, eventsWhileOpen, "the event logged while open must be captured"); + + log.close(); + rawLogger.info("after close"); + + assertEquals(eventsWhileOpen, log.events().size(), + "close() must detach the appender: an event logged after close must not be " + + "captured, but the captured list grew from " + eventsWhileOpen + " to " + + log.events().size()); + } + + /** + * The helper's own headline contract, pinned in one place independent of any production + * class's behaviour: {@code close()} restores the level the logger had before {@link + * CapturedLog#at} pinned it. + */ + @Test + void closeRestoresTheLevelCapturedAtOpen() { + String loggerName = "capturedlog-test-only.level-restore"; + Logger rawLogger = (Logger) LoggerFactory.getLogger(loggerName); + rawLogger.setLevel(Level.DEBUG); + + CapturedLog log = CapturedLog.at(loggerName, Level.ERROR); + assertEquals(Level.ERROR, rawLogger.getLevel(), "the pinned level took effect while open"); + + log.close(); + + assertEquals(Level.DEBUG, rawLogger.getLevel(), + "close() must restore the level captured when at() was called (DEBUG), not leave " + + "the pinned level (ERROR) in place"); + } + + /** + * {@link CapturedLog#setLevel}'s javadoc claims that re-pinning the level mid-capture does not + * change what {@code close()} restores — that restore always uses the level captured when the + * instance was created, never a value set through {@code setLevel}. Nothing checked this + * before: open with a pinned WARN, call {@code setLevel(TRACE)}, close, and the result must be + * the level from BEFORE {@code at} — neither WARN nor TRACE. + */ + @Test + void setLevelDuringCaptureDoesNotChangeWhatCloseRestores() { + String loggerName = "capturedlog-test-only.setlevel-no-effect"; + Logger rawLogger = (Logger) LoggerFactory.getLogger(loggerName); + rawLogger.setLevel(Level.DEBUG); + + CapturedLog log = CapturedLog.at(loggerName, Level.WARN); + log.setLevel(Level.TRACE); + assertEquals(Level.TRACE, rawLogger.getLevel(), "setLevel took effect immediately"); + + log.close(); + + assertEquals(Level.DEBUG, rawLogger.getLevel(), + "close() must restore the level captured at open (DEBUG) regardless of any " + + "later setLevel() call: it must be neither WARN (the level pinned by " + + "at()) nor TRACE (the level set via setLevel() mid-capture), but got " + + rawLogger.getLevel()); + } +} -- 2.52.0