fleetd #529 follow-up: the new helper's javadoc claimed a reach it does not have
CapturedLog's class javadoc said it is "the one way to pin or capture a logger's
level and output in this test tree". Measured on main at af95897, that is false:
nine test files still hand-roll the ListAppender + setLevel + finally
detachAppender pattern, with 42 setLevel calls on a raw logback Logger between
them.
None of those nine is a defect. Every one pairs its pin with a restore, so none
is the fleetd #525 leak, and #529's scope was the 19 unrestored pins only. The
problem is the sentence, not the code: a reader who believes "the one way" and
then greps finds nine counter-examples and cannot tell a leftover from a
violation. That is the same shape as a wrong reason in a comment — the text
survives while the fact under it moves.
Replaced with what is actually true: new code must use the helper, the pattern
still exists elsewhere, and here is the list plus the two commands that
re-measure it. The paragraph says to delete itself once the first command comes
back empty, rather than to keep a count up to date.
Javadoc only. mvn -f fleetd/pom.xml test-compile exit 0.
This commit is contained in:
@@ -21,9 +21,29 @@ import java.util.List;
|
||||
* fork. try-with-resources makes "restored the appender but not the level" impossible to write,
|
||||
* because there is only one thing to close.
|
||||
*
|
||||
* <p>This is the one way to pin or capture a logger's level and output in this test tree — do not
|
||||
* hand-roll the {@code ListAppender} + {@code setLevel} + {@code finally detachAppender} pattern;
|
||||
* use {@link #at} or {@link #of} instead.
|
||||
* <p>New code must use this rather than hand-rolling the {@code ListAppender} + {@code setLevel} +
|
||||
* {@code finally detachAppender} pattern: use {@link #at} or {@link #of}. It is <em>not</em> yet
|
||||
* the only instance of the pattern in this test tree, and the earlier wording here said it was —
|
||||
* which would leave a reader who greps unable to tell a leftover from a violation.
|
||||
*
|
||||
* <p>Measured on main at af95897 (2026-09-12): nine test files still hand-roll it, with 42
|
||||
* {@code setLevel} calls on a raw logback {@code Logger} between them — {@code
|
||||
* FleetdStartupReportTest}, {@code GitHostShapeReportTest}, {@code MemberCredentialsGapReportTest},
|
||||
* {@code MemberTrustModelReportTest}, {@code FleetHealthMonitorTest}, {@code
|
||||
* ClaudeCodeLauncherTest}, {@code HerdrPeerLauncherAllowListWiringTest}, {@code
|
||||
* HerdrPeerLauncherCharterTest} and {@code OpenCodeLauncherTest}. Every one of them pairs its pin
|
||||
* with a restore, so none is the fleetd #525 leak and none was in fleetd #529's scope, which was
|
||||
* the 19 <em>unrestored</em> pins only. They are unmigrated, not broken.
|
||||
*
|
||||
* <p>Re-measure with the two commands below, from the repo root. A file that appears in the first
|
||||
* list and not the second still hand-rolls the pattern. When the first list comes back empty, this
|
||||
* paragraph is spent and the sentence above can go back to saying "the one way" — delete the
|
||||
* paragraph then rather than updating the count.
|
||||
*
|
||||
* <pre>{@code
|
||||
* grep -rlE '\.setLevel\(' fleetd/src/test/java --include='*.java' | grep -v CapturedLog.java
|
||||
* grep -rl 'CapturedLog' fleetd/src/test/java --include='*.java'
|
||||
* }</pre>
|
||||
*/
|
||||
public final class CapturedLog implements AutoCloseable {
|
||||
private final Logger logger;
|
||||
|
||||
Reference in New Issue
Block a user