Compare commits
3 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 8308c0b68f | |||
| 3b3063eb2b | |||
| df9086263d |
@@ -7,6 +7,7 @@ import java.util.Locale;
|
||||
import java.util.Set;
|
||||
import java.util.regex.Matcher;
|
||||
import java.util.regex.Pattern;
|
||||
import java.util.stream.Collectors;
|
||||
import org.junit.jupiter.api.DisplayName;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
@@ -68,6 +69,32 @@ class RestRouteInventoryTest {
|
||||
"GET /tasks/{ticket}"
|
||||
);
|
||||
|
||||
private static final Pattern ROUTE_CALL =
|
||||
Pattern.compile("app\\.(get|post|delete|put|patch)\\(\\s*\"([^\"]+)\"");
|
||||
|
||||
/**
|
||||
* Drop whole-line comments before scraping. Without this the scrape reads commented-out code as
|
||||
* live: a registration disabled with {@code //} still matched, so the route stayed in the
|
||||
* inventory while the server no longer served it — a silent false PASS, measured on 2026-09-03
|
||||
* by commenting out {@code app.get("/tasks/{ticket}", ...)} and watching this test stay green.
|
||||
* Deleting the same line was caught correctly, so only the commented-out shape was blind.
|
||||
*
|
||||
* <p>Only lines whose first non-blank characters are {@code //}, {@code *} or {@code /*} are
|
||||
* dropped — deliberately NOT every {@code //} anywhere on a line, because that would also cut a
|
||||
* string literal containing {@code //} (a URL) and could silently delete a real registration
|
||||
* sharing that line. The remaining gap is a trailing comment on the same line as real code; no
|
||||
* registration in this file has that shape, and the vacuity test below would catch a scrape that
|
||||
* lost registrations wholesale.
|
||||
*/
|
||||
private static String withoutCommentLines(String source) {
|
||||
return source.lines()
|
||||
.filter(line -> {
|
||||
String t = line.stripLeading();
|
||||
return !(t.startsWith("//") || t.startsWith("*") || t.startsWith("/*"));
|
||||
})
|
||||
.collect(Collectors.joining("\n"));
|
||||
}
|
||||
|
||||
/**
|
||||
* Every {@code app.<verb>("<path>")} call in {@link FleetApp}'s source, as {@code "VERB path"}.
|
||||
* This matches inside an {@code if (...) { ... }} block just as well as a top-level statement —
|
||||
@@ -75,8 +102,7 @@ class RestRouteInventoryTest {
|
||||
* catches {@code GET /metrics} (registered conditionally on {@code metrics != null}).
|
||||
*/
|
||||
private static Set<String> routesTheServerRegisters() throws Exception {
|
||||
String source = Files.readString(REST_SOURCE);
|
||||
Matcher m = Pattern.compile("app\\.(get|post|delete|put|patch)\\(\\s*\"([^\"]+)\"").matcher(source);
|
||||
Matcher m = ROUTE_CALL.matcher(withoutCommentLines(Files.readString(REST_SOURCE)));
|
||||
Set<String> found = new LinkedHashSet<>();
|
||||
while (m.find()) {
|
||||
found.add(m.group(1).toUpperCase(Locale.ROOT) + " " + m.group(2));
|
||||
|
||||
@@ -45,6 +45,9 @@ public final class FakeWorktrees implements Worktrees {
|
||||
private final List<String> overlayShareOrder = new CopyOnWriteArrayList<>();
|
||||
private final Set<String> existingPaths = ConcurrentHashMap.newKeySet();
|
||||
private final Set<String> trackedPaths = ConcurrentHashMap.newKeySet();
|
||||
/** Worktree paths that currently exist, mirroring GitWorktrees' {@code Files.exists} check for
|
||||
* the already-gone case (CB-576 review, fleetd #116). */
|
||||
private final Set<String> worktreePaths = ConcurrentHashMap.newKeySet();
|
||||
private final AtomicLong snapshotSeq = new AtomicLong();
|
||||
private volatile RuntimeException addFailure;
|
||||
private volatile RuntimeException snapshotFailure;
|
||||
@@ -115,7 +118,15 @@ public final class FakeWorktrees implements Worktrees {
|
||||
}
|
||||
// The branch already carries a unique nonce, so the derived path is distinct per acquire
|
||||
// without an extra counter — keep it a pure function of the branch the test can predict.
|
||||
return prefix + "/" + branch.replace('/', '_');
|
||||
String path = prefix + "/" + branch.replace('/', '_');
|
||||
worktreePaths.add(path);
|
||||
return path;
|
||||
}
|
||||
|
||||
/** Model an operator / {@code git worktree prune} removing the worktree before release. */
|
||||
public FakeWorktrees markGone(String worktreePath) {
|
||||
worktreePaths.remove(worktreePath);
|
||||
return this;
|
||||
}
|
||||
|
||||
@Override
|
||||
@@ -125,6 +136,11 @@ public final class FakeWorktrees implements Worktrees {
|
||||
|
||||
@Override
|
||||
public boolean hasUncommitted(String worktreePath) {
|
||||
// A path that was never added, or was marked gone, is reported clean, mirroring
|
||||
// GitWorktrees' already-gone guard — never an error, so teardown still completes.
|
||||
if (!worktreePaths.contains(worktreePath)) {
|
||||
return false;
|
||||
}
|
||||
return dirty;
|
||||
}
|
||||
|
||||
|
||||
@@ -258,6 +258,38 @@ class WorktreeSessionManagerTest {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* CB-576 review (fleetd #116). A worktree that is already gone (operator cleanup,
|
||||
* {@code git worktree prune}, an earlier half-completed release) must not break teardown.
|
||||
* {@code hasUncommitted} reports the missing path clean (mirroring {@code GitWorktrees}), so
|
||||
* {@code release} still runs {@code notifyReleased} (the CB-516 fast-fail for a blocked
|
||||
* {@code fleet_send} caller) and {@code launcher.stop} (so the pane is not orphaned), and falls
|
||||
* through to the already-gone-tolerant {@code remove}. Since CB-581 this all happens because the
|
||||
* notify-and-stop work sits in {@code release}'s {@code finally}/post-try block rather than a
|
||||
* checked branch — this test pins that shape by construction.
|
||||
*/
|
||||
@Test
|
||||
void releaseStillStopsPaneAndNotifiesWhenWorktreeIsGone() {
|
||||
FakeHerdr herdr = new FakeHerdr();
|
||||
FakeWorktrees worktrees = new FakeWorktrees().withRepoRoot("/repo").withPrefix("/wt");
|
||||
SessionManager sessions = new SessionManager(workerService(herdr), worktrees);
|
||||
List<SessionManager.ReleaseDetail> released = new java.util.concurrent.CopyOnWriteArrayList<>();
|
||||
sessions.onRelease(released::add);
|
||||
MemberSession s = sessions.acquire("ltms-local", null, "/caller/proj", null,
|
||||
new WorktreeRequest("cb-576g", null));
|
||||
|
||||
worktrees.markGone(s.worktree());
|
||||
sessions.release(s.paneId());
|
||||
|
||||
assertEquals(1, released.size(),
|
||||
"notifyReleased must still fire when the worktree is already gone (CB-516)");
|
||||
assertEquals(s.terminalId(), released.getFirst().terminalId());
|
||||
assertTrue(herdr.called("pane.close"),
|
||||
"the pane must still be stopped when the worktree is already gone");
|
||||
assertEquals(1, worktrees.removeCalls().size(),
|
||||
"release still calls the already-gone-tolerant remove");
|
||||
}
|
||||
|
||||
@Test
|
||||
void drainAllPreservesWorktreeOfIdleSession() {
|
||||
FakeHerdr herdr = new FakeHerdr();
|
||||
|
||||
Reference in New Issue
Block a user