Never prune queued/running builds; dedup manual triggers
Two defects seen live on vm4006 when a merged branch was deleted from origin while its last build still ran: - The result prune removed the PENDING/RUNNING entries of branches gone from origin, so the executing build vanished from UI and history and the queue looked stuck. Prune now never touches a PENDING or RUNNING entry (worktrees were already protected). The startup recovery closes out an orphaned PENDING of a gone branch as INTERRUPTED, so the new immunity cannot leak entries. - The apparent hang invited restart clicks, and each click stacked another build of the same commit. startBuild now returns the already queued or executing build of the same branch and commit instead of a duplicate; cancel-requested builds do not block re-queueing, and re-running a finished build stays possible. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -219,6 +219,31 @@ class BuildExecutorTest : FunSpec() {
|
||||
}
|
||||
}
|
||||
|
||||
test("startBuild returns the active build of the same branch and commit instead of stacking a duplicate") {
|
||||
val h = harness("sleep 30")
|
||||
|
||||
val first = h.executor.startBuild("main", "abc123", h.workingDir)
|
||||
// a double-triggered UI restart: same branch, same commit, while queued or running
|
||||
val duplicate = h.executor.startBuild("main", "abc123", h.workingDir)
|
||||
duplicate.artifactKey shouldBe first.artifactKey
|
||||
h.repository.history().map { it.artifactKey } shouldContainExactly listOf(first.artifactKey)
|
||||
|
||||
// another commit of the branch is a distinct build, queued behind the first
|
||||
val newerCommit = h.executor.startBuild("main", "abc124", h.workingDir)
|
||||
newerCommit.artifactKey shouldNotBe first.artifactKey
|
||||
|
||||
// a cancel-requested build no longer blocks re-queueing its commit
|
||||
h.executor.cancel(first.artifactKey).shouldBeTrue()
|
||||
val again = h.executor.startBuild("main", "abc123", h.workingDir)
|
||||
again.artifactKey shouldNotBe first.artifactKey
|
||||
|
||||
h.executor.cancel(newerCommit.artifactKey).shouldBeTrue()
|
||||
h.executor.cancel(again.artifactKey).shouldBeTrue()
|
||||
eventually(30.seconds) {
|
||||
h.executor.currentBuilds().shouldBeEmpty()
|
||||
}
|
||||
}
|
||||
|
||||
test("a build cancelled while still queued records neither runningSince nor a duration") {
|
||||
val h = harness("sleep 30")
|
||||
|
||||
|
||||
@@ -220,6 +220,31 @@ class FileBuildResultRepositoryTest : FunSpec() {
|
||||
)
|
||||
}
|
||||
|
||||
test("prune never removes queued or running results, even of branches gone from origin") {
|
||||
val repository = FileBuildResultRepository(newFile())
|
||||
// a merged branch, deleted from origin while its last build still runs
|
||||
repository.append(result(branch = "merged", status = BuildStatus.FAILED, startedOffsetSeconds = 0))
|
||||
repository.append(result(branch = "merged", status = BuildStatus.RUNNING, startedOffsetSeconds = 60))
|
||||
// a queued build beyond the retention count of its branch
|
||||
repository.append(result(branch = "main", status = BuildStatus.PENDING, startedOffsetSeconds = 0))
|
||||
repository.append(result(branch = "main", startedOffsetSeconds = 60))
|
||||
|
||||
val removed =
|
||||
repository.prune(
|
||||
originBranches = listOf("main"),
|
||||
retentionPerBranch = 1,
|
||||
retentionCutoff = baseTime.plusSeconds(30),
|
||||
)
|
||||
|
||||
removed shouldContainExactly listOf(result(branch = "merged", status = BuildStatus.FAILED, startedOffsetSeconds = 0))
|
||||
repository.history() shouldContainExactlyInAnyOrder
|
||||
listOf(
|
||||
result(branch = "merged", status = BuildStatus.RUNNING, startedOffsetSeconds = 60),
|
||||
result(branch = "main", status = BuildStatus.PENDING, startedOffsetSeconds = 0),
|
||||
result(branch = "main", startedOffsetSeconds = 60),
|
||||
)
|
||||
}
|
||||
|
||||
test("prune drops entries older than the retention cutoff even within the retention count") {
|
||||
val repository = FileBuildResultRepository(newFile())
|
||||
repository.append(result(branch = "main", status = BuildStatus.FAILED, startedOffsetSeconds = 0))
|
||||
|
||||
@@ -420,6 +420,20 @@ class WatcherTest : FunSpec() {
|
||||
harness.startedBuilds shouldContainExactly listOf("main" to "commit-2")
|
||||
}
|
||||
|
||||
test("startup recovery closes out an orphaned PENDING build of a branch gone from origin") {
|
||||
val harness = Harness()
|
||||
val orphan = harness.seed("gone", BuildStatus.PENDING, commit = "commit-1")
|
||||
|
||||
harness.watcher.recoverOnStartup(harness.workingDir)
|
||||
|
||||
// PENDING is prune-immune; left as-is, the gone branch could never be pruned
|
||||
harness.startedBuilds.shouldBeEmpty()
|
||||
harness.repository
|
||||
.history()
|
||||
.first { it.artifactKey == orphan.artifactKey }
|
||||
.status shouldBe BuildStatus.INTERRUPTED
|
||||
}
|
||||
|
||||
test("poll prunes results, artifacts, and worktrees of branches gone from origin") {
|
||||
val harness = Harness()
|
||||
harness.seed("main", BuildStatus.SUCCESS, commit = "commit-1")
|
||||
|
||||
Reference in New Issue
Block a user