From 493148aafc14cf97e908c5c3f99b36b9bb644d33 Mon Sep 17 00:00:00 2001 From: Daniel McCoy Stephenson Date: Fri, 23 Oct 2026 07:15:19 -0600 Subject: [PATCH 1/2] Cover withServerWideTags directly and a valueless enabled: line withServerWideTags drops null tags, lets the event's own tag win, keeps event tags first, and never modifies the caller's map; none of that was asserted on the helper itself, and its null-dropping cannot be reached through report(). The cap is now also pinned for an event already at or over MAX_TAGS. A bare "enabled:" is skipped while a comment-only one is taken as the switch; the test pins that, and #27 asks whether it should. Co-Authored-By: Claude Opus 5.5 --- .../stephenson/trace/TraceClientTest.java | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/src/test/java/software/stephenson/trace/TraceClientTest.java b/src/test/java/software/stephenson/trace/TraceClientTest.java index 963dcab..787a3db 100644 --- a/src/test/java/software/stephenson/trace/TraceClientTest.java +++ b/src/test/java/software/stephenson/trace/TraceClientTest.java @@ -13,6 +13,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -394,6 +395,18 @@ void serverWideConfig_theFirstEnabledLineIsTheSwitch() { "a commented-out line is not the first one"); } + @Test + void serverWideConfig_aValuelessEnabledLineIsSkippedButACommentOnlyOneIsTheSwitch() { + // Characterizes current behaviour, which issue #27 questions: ENABLED_LINE needs a non-blank + // value, so a bare "enabled:" is passed over and a later line is the + // switch; but after "enabled: # ..." the captured value is "#", which + // is not off, so that line is the switch and a later one is ignored. + assertTrue(TraceClient.parseServerWideConfig(Arrays.asList("enabled:", "enabled: false")).disables); + assertTrue(TraceClient.parseServerWideConfig(Arrays.asList("enabled: ", "enabled: false")).disables); + assertFalse(TraceClient.parseServerWideConfig(Arrays.asList("enabled: # set below", "enabled: false")).disables); + assertFalse(TraceClient.parseServerWideConfig(Arrays.asList("enabled:")).disables, "a bare enabled: alone is enabled"); + } + @Test void json_escapesControlCharactersAndSkipsNullTags() { Map tags = new LinkedHashMap<>(); @@ -1243,6 +1256,56 @@ void withServerWideTags_leavesTheEventAloneWhenThereAreNone() { assertNull(TraceClient.withServerWideTags(null, Collections.emptyMap())); } + @Test + void withServerWideTags_dropsNullTagsLetsTheEventWinAndNeverModifiesTheCallersMap() { + // Arrange + Map tags = new LinkedHashMap<>(); + tags.put("name", "home"); + tags.put("nullValue", null); + tags.put(null, "nullKey"); + tags.put("ci", "the-events-own"); + Map serverWide = new LinkedHashMap<>(); + serverWide.put("ci", "true"); + serverWide.put("env", "staging"); + + // Act + Map merged = TraceClient.withServerWideTags(tags, serverWide); + + // Assert + Map expected = new LinkedHashMap<>(); + expected.put("name", "home"); + expected.put("ci", "the-events-own"); + expected.put("env", "staging"); + assertEquals(expected, merged); + assertEquals(new ArrayList<>(expected.keySet()), new ArrayList<>(merged.keySet()), "event tags first, then server-wide"); + assertEquals(4, tags.size(), "the caller's map is not modified"); + assertEquals(serverWide, TraceClient.withServerWideTags(null, serverWide), "an event with no tags gets the server-wide ones"); + } + + @Test + void withServerWideTags_addsNoneToAnEventAlreadyAtOrOverMaxTags() { + // Arrange + Map atMax = new LinkedHashMap<>(); + for (int i = 0; i < TraceClient.MAX_TAGS; i++) { + atMax.put("e" + i, "v"); + } + Map overMax = new LinkedHashMap<>(atMax); + overMax.put("version", "1.2.3"); // what withVersion does today, see issue #13 + Map oneShort = new LinkedHashMap<>(atMax); + oneShort.remove("e0"); + Map serverWide = new LinkedHashMap<>(); + serverWide.put("ci", "true"); + serverWide.put("env", "staging"); + + // Act + Assert + assertEquals(atMax, TraceClient.withServerWideTags(atMax, serverWide)); + assertEquals(overMax, TraceClient.withServerWideTags(overMax, serverWide), "the cap adds nothing, and drops nothing either"); + Map filled = TraceClient.withServerWideTags(oneShort, serverWide); + assertEquals(TraceClient.MAX_TAGS, filled.size()); + assertEquals("true", filled.get("ci")); + assertFalse(filled.containsKey("env"), "only the first server-wide tag fits"); + } + @Test void serverWideConfig_ioFailureIsLoggedFineAndTreatedAsEnabled(@TempDir Path scratch) throws Exception { // Arrange From bcc67be8ab105e2fd38825c014bf80fe2a5bb7ad Mon Sep 17 00:00:00 2001 From: Daniel McCoy Stephenson Date: Fri, 23 Oct 2026 07:16:49 -0600 Subject: [PATCH 2/2] Reflow the #27 characterization comment to the file's width Co-Authored-By: Claude Opus 5.5 --- .../java/software/stephenson/trace/TraceClientTest.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/test/java/software/stephenson/trace/TraceClientTest.java b/src/test/java/software/stephenson/trace/TraceClientTest.java index 787a3db..d80b902 100644 --- a/src/test/java/software/stephenson/trace/TraceClientTest.java +++ b/src/test/java/software/stephenson/trace/TraceClientTest.java @@ -397,10 +397,11 @@ void serverWideConfig_theFirstEnabledLineIsTheSwitch() { @Test void serverWideConfig_aValuelessEnabledLineIsSkippedButACommentOnlyOneIsTheSwitch() { - // Characterizes current behaviour, which issue #27 questions: ENABLED_LINE needs a non-blank - // value, so a bare "enabled:" is passed over and a later line is the - // switch; but after "enabled: # ..." the captured value is "#", which - // is not off, so that line is the switch and a later one is ignored. + // Characterizes current behaviour, which issue #27 questions: + // ENABLED_LINE needs a non-blank value, so a bare "enabled:" is + // passed over and a later line is the switch; but after + // "enabled: # ..." the captured value is "#", which is not off, so + // that line is the switch and a later one is ignored. assertTrue(TraceClient.parseServerWideConfig(Arrays.asList("enabled:", "enabled: false")).disables); assertTrue(TraceClient.parseServerWideConfig(Arrays.asList("enabled: ", "enabled: false")).disables); assertFalse(TraceClient.parseServerWideConfig(Arrays.asList("enabled: # set below", "enabled: false")).disables);