From c7903c1efe23db78526b75e48b1f13694dc71c2c Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 12 Sep 2026 13:17:21 +0700 Subject: [PATCH] fleetd #535: convert FleetdLeadMailboxSelectionTest to CapturedLog Three call sites (captureFleetdLogs at :55) attached a ListAppender to the Fleetd.class logger with addAppender and never detached it, and never called appender.setContext(...) either. Logback Logger instances are cached per class and shared for the whole JVM, and surefire reuses forks, so all three appenders stayed attached for every later test in the fork. Convert all three call sites to CapturedLog.of(Fleetd.class) (added in #533) via try-with-resources, which detaches the appender and sets the context for free. Delete captureFleetdLogs(); nothing calls it now. --- .../fleet/FleetdLeadMailboxSelectionTest.java | 68 ++++++++----------- 1 file changed, 30 insertions(+), 38 deletions(-) diff --git a/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadMailboxSelectionTest.java b/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadMailboxSelectionTest.java index f681bfa..e4bfa2c 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadMailboxSelectionTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/FleetdLeadMailboxSelectionTest.java @@ -1,15 +1,12 @@ package dev.ltms.fleet; import ch.qos.logback.classic.Level; -import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.spi.ILoggingEvent; -import ch.qos.logback.core.read.ListAppender; import dev.ltms.fleet.config.FleetConfig; import dev.ltms.fleet.msg.LeadMailbox; +import dev.ltms.fleet.testing.CapturedLog; import org.junit.jupiter.api.Test; -import org.slf4j.LoggerFactory; -import java.util.List; import java.util.Map; import static org.junit.jupiter.api.Assertions.*; @@ -52,29 +49,22 @@ class FleetdLeadMailboxSelectionTest { } } - private static ListAppender captureFleetdLogs() { - Logger logger = (Logger) LoggerFactory.getLogger(Fleetd.class); - ListAppender appender = new ListAppender<>(); - appender.start(); - logger.addAppender(appender); - return appender; - } - - private static String joined(ListAppender appender, Level level) { - return appender.list.stream().filter(e -> e.getLevel() == level) + private static String joined(CapturedLog captured, Level level) { + return captured.events().stream().filter(e -> e.getLevel() == level) .map(ILoggingEvent::getFormattedMessage).reduce("", (a, b) -> a + "\n" + b); } @Test void noCoordinatorBlockLeavesTheFeatureOffSilently() { - var appender = captureFleetdLogs(); - var opener = new RecordingOpener(); + try (var captured = CapturedLog.of(Fleetd.class)) { + var opener = new RecordingOpener(); - assertNull(Fleetd.openLeadMailbox(null, Map.of(), opener)); + assertNull(Fleetd.openLeadMailbox(null, Map.of(), opener)); - assertNull(opener.offeredUri, "nothing configured means nothing is opened"); - assertEquals("", joined(appender, Level.WARN), - "an opt-in feature nobody asked for must not warn on every boot"); + assertNull(opener.offeredUri, "nothing configured means nothing is opened"); + assertEquals("", joined(captured, Level.WARN), + "an opt-in feature nobody asked for must not warn on every boot"); + } } @Test @@ -114,31 +104,33 @@ class FleetdLeadMailboxSelectionTest { @Test void warnsAndStaysOffWhenSelfIdIsMissing() { - var appender = captureFleetdLogs(); - var opener = new RecordingOpener(); - var coordinator = new FleetConfig.Coordinator(RESOLVED_URI, null, null, null, null); + try (var captured = CapturedLog.of(Fleetd.class)) { + var opener = new RecordingOpener(); + var coordinator = new FleetConfig.Coordinator(RESOLVED_URI, null, null, null, null); - assertNull(Fleetd.openLeadMailbox(coordinator, Map.of(), opener)); + assertNull(Fleetd.openLeadMailbox(coordinator, Map.of(), opener)); - assertNull(opener.offeredUri, "a mailbox with no owning coord-id has no queue to declare"); - String warns = joined(appender, Level.WARN); - assertTrue(warns.contains("coordinator.selfId"), () -> "say which key is missing: " + warns); - assertFalse(warns.contains(SECRET), () -> "the URI's password must never be logged: " + warns); + assertNull(opener.offeredUri, "a mailbox with no owning coord-id has no queue to declare"); + String warns = joined(captured, Level.WARN); + assertTrue(warns.contains("coordinator.selfId"), () -> "say which key is missing: " + warns); + assertFalse(warns.contains(SECRET), () -> "the URI's password must never be logged: " + warns); + } } @Test void warnsAndStaysOffWhenTheBrokerIsUnreachableAtBoot() { - var appender = captureFleetdLogs(); - var opener = new RecordingOpener(); - opener.unreachable = true; - var coordinator = new FleetConfig.Coordinator(RESOLVED_URI, null, "mac-opus", null, null); + try (var captured = CapturedLog.of(Fleetd.class)) { + var opener = new RecordingOpener(); + opener.unreachable = true; + var coordinator = new FleetConfig.Coordinator(RESOLVED_URI, null, "mac-opus", null, null); - assertNull(Fleetd.openLeadMailbox(coordinator, Map.of(), opener), - "a down coordination broker turns the feature off; it must never take the daemon down"); + assertNull(Fleetd.openLeadMailbox(coordinator, Map.of(), opener), + "a down coordination broker turns the feature off; it must never take the daemon down"); - String warns = joined(appender, Level.WARN); - assertTrue(warns.contains("coord.example"), () -> "name the host that failed: " + warns); - assertFalse(warns.contains(SECRET), () -> "with credentials stripped: " + warns); - assertTrue(warns.contains("Connection refused"), () -> "and the real reason: " + warns); + String warns = joined(captured, Level.WARN); + assertTrue(warns.contains("coord.example"), () -> "name the host that failed: " + warns); + assertFalse(warns.contains(SECRET), () -> "with credentials stripped: " + warns); + assertTrue(warns.contains("Connection refused"), () -> "and the real reason: " + warns); + } } } -- 2.52.0