Mark the logs that carry a failure line on the artifact index
The artifact index listed the stored logs as bare file names, so a red build gave no hint which of build.log, build.stdout.log and build.stderr.log actually explains it — with stdout and stderr stored separately, the failure is usually in only some of them. Each log of a non-green build is now scanned for an upper-case FAILED/FAILURE, which covers BUILD FAILED, Maven's BUILD FAILURE and Gradle's per-test "SomeTest > works() FAILED"; lower-case prose does not count. The scan streams line by line with an early exit and reads ISO-8859-1, so no byte sequence of a build log can fail to decode. Logs of a successful build are not scanned at all — that saves reading megabytes per page view and avoids an alarming badge on a green build whose log mentions a deliberately failing sub-build. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -17,6 +17,7 @@ import org.springframework.web.bind.annotation.GetMapping
|
||||
import org.springframework.web.bind.annotation.PathVariable
|
||||
import org.springframework.web.server.ResponseStatusException
|
||||
import org.springframework.web.servlet.view.RedirectView
|
||||
import java.nio.charset.StandardCharsets
|
||||
import java.nio.file.Files
|
||||
import java.nio.file.Path
|
||||
import java.nio.file.Paths
|
||||
@@ -188,7 +189,11 @@ class UiController(
|
||||
model.addAttribute("result", result?.let { BuildRowView.from(it, links) })
|
||||
model.addAttribute("hasArtifacts", artifactDir != null)
|
||||
model.addAttribute("buildCommand", result?.let { branchBuildCommand(it.branch) })
|
||||
model.addAttribute("logs", artifactDir?.let { logFiles(it) } ?: emptyList<String>())
|
||||
model.addAttribute(
|
||||
"logs",
|
||||
artifactDir?.let { logFiles(it, scanForFailure = result != null && result.status != BuildStatus.SUCCESS) }
|
||||
?: emptyList<LogFileView>(),
|
||||
)
|
||||
model.addAttribute("reportIndexes", artifactDir?.let { reportIndexes(it) } ?: emptyList<String>())
|
||||
return "artifact"
|
||||
}
|
||||
@@ -220,17 +225,39 @@ class UiController(
|
||||
return (branches[branch] ?: branches["default"])?.buildCommand ?: ""
|
||||
}
|
||||
|
||||
/** The stored log files: all top-level regular files of the artifact directory. */
|
||||
private fun logFiles(artifactDir: Path): List<String> =
|
||||
/**
|
||||
* The stored log files: all top-level regular files of the artifact directory.
|
||||
* With [scanForFailure] each one is searched for a failure line, so that a red build marks
|
||||
* the logs that explain it — a build split over stdout/stderr logs usually leaves the
|
||||
* failure in only some of them, and a failed test is reported far from `BUILD FAILED`.
|
||||
*/
|
||||
private fun logFiles(
|
||||
artifactDir: Path,
|
||||
scanForFailure: Boolean,
|
||||
): List<LogFileView> =
|
||||
Files.list(artifactDir).use { children ->
|
||||
children
|
||||
.asSequence()
|
||||
.filter { Files.isRegularFile(it) }
|
||||
.map { it.name }
|
||||
.sorted()
|
||||
.sortedBy { it.name }
|
||||
.map { LogFileView(name = it.name, failed = scanForFailure && containsFailureMarker(it)) }
|
||||
.toList()
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a log carries a build tool's failure line. Read as ISO-8859-1 and line by line:
|
||||
* the markers are ASCII, so no byte sequence of a build log can fail to decode, and the
|
||||
* scan stops at the first hit instead of pulling a multi-megabyte log into memory.
|
||||
*/
|
||||
private fun containsFailureMarker(logFile: Path): Boolean =
|
||||
try {
|
||||
Files.newBufferedReader(logFile, StandardCharsets.ISO_8859_1).use { reader ->
|
||||
reader.lineSequence().any { FAILURE_MARKER.containsMatchIn(it) }
|
||||
}
|
||||
} catch (_: Exception) {
|
||||
false
|
||||
}
|
||||
|
||||
/**
|
||||
* The browsable report pages under `reports/`, shallowest first; pages nested
|
||||
* below an already-listed report index are skipped — like the legacy artifact index.
|
||||
@@ -319,6 +346,13 @@ class UiController(
|
||||
companion object {
|
||||
private val FAILURES_COUNTER = Regex("""id="failures">\s*<div class="counter">(\d+)""")
|
||||
|
||||
/**
|
||||
* A failure line of a build tool or of a single test — `BUILD FAILED`, `BUILD FAILURE`,
|
||||
* `FAILURE: Build failed …`, and Gradle's per-test `SomeTest > works() FAILED`.
|
||||
* Upper case only, on purpose: a prose "failed" says nothing, the shouted word does.
|
||||
*/
|
||||
private val FAILURE_MARKER = Regex("""\b(FAILED|FAILURE)\b""")
|
||||
|
||||
/** Legacy page name → new route; about/license had no successor pages and land on the start page. */
|
||||
private val LEGACY_PAGE_TARGETS =
|
||||
mapOf(
|
||||
|
||||
@@ -161,6 +161,16 @@ data class ReportIndexView(
|
||||
val failures: Int?,
|
||||
)
|
||||
|
||||
/**
|
||||
* One log link of the artifact index. [failed] marks the logs that actually carry the
|
||||
* build tool's failure line, so a red build points at the log worth opening; it stays
|
||||
* false for successful builds, whose logs are not scanned at all.
|
||||
*/
|
||||
data class LogFileView(
|
||||
val name: String,
|
||||
val failed: Boolean,
|
||||
)
|
||||
|
||||
/** One card of the current-builds view; the live log is fetched by `gittally.js`. */
|
||||
data class CurrentBuildView(
|
||||
val branch: String,
|
||||
|
||||
@@ -50,8 +50,10 @@
|
||||
<h2>Logs</h2>
|
||||
<ul th:if="${!#lists.isEmpty(logs)}">
|
||||
<li th:each="log : ${logs}">
|
||||
<a th:href="${filesBase} + '/' + ${log}" target="_blank"
|
||||
rel="noopener noreferrer" th:text="${log}">build.log</a>
|
||||
<a th:href="${filesBase} + '/' + ${log.name}" target="_blank"
|
||||
rel="noopener noreferrer" th:text="${log.name}">build.log</a>
|
||||
<span th:if="${log.failed}" class="status status-failed"
|
||||
title="this log contains the build tool's failure line">failed</span>
|
||||
</li>
|
||||
</ul>
|
||||
<p th:if="${#lists.isEmpty(logs)}" class="muted">
|
||||
|
||||
@@ -335,6 +335,46 @@ class UiControllerTest : FunSpec() {
|
||||
Regex(""" failed</span>""").findAll(page).count() shouldBe 1
|
||||
}
|
||||
|
||||
test("of a failed build, only the logs carrying a failure line get a failed-badge") {
|
||||
val artifactDir = Files.createDirectories(tempDir.resolve("failed-key"))
|
||||
Files.writeString(artifactDir.resolve("build.log"), "compiling\nBUILD FAILED in 20s\n")
|
||||
// a failed test, far away from any BUILD FAILED line
|
||||
Files.writeString(artifactDir.resolve("build.stdout.log"), "SomeTest > works() FAILED\ncompiling\n")
|
||||
Files.writeString(artifactDir.resolve("build.stderr.log"), "warning: this test has failed before\n")
|
||||
every { repository.history() } returns
|
||||
listOf(successResult.copy(status = BuildStatus.FAILED, artifactKey = "failed-key"))
|
||||
every { artifactStore.artifactDir("failed-key") } returns artifactDir
|
||||
|
||||
val page =
|
||||
mockMvc
|
||||
.perform(get("/builds/failed-key"))
|
||||
.andExpect(status().isOk)
|
||||
.andReturn()
|
||||
.response.contentAsString
|
||||
|
||||
// two badges for the logs, plus the one of the build status itself
|
||||
Regex(""">failed</span>""").findAll(page).count() shouldBe 3
|
||||
// lower-case prose about failing is not a failure line
|
||||
page.substringAfter("build.stderr.log</a>").substringBefore("</li>") shouldNotContain "status-failed"
|
||||
}
|
||||
|
||||
test("logs of a successful build are not scanned for failure lines") {
|
||||
val artifactDir = Files.createDirectories(tempDir.resolve("green-key"))
|
||||
Files.writeString(artifactDir.resolve("build.log"), "BUILD FAILED in a nested build\nBUILD SUCCESSFUL\n")
|
||||
every { repository.history() } returns listOf(successResult.copy(artifactKey = "green-key"))
|
||||
every { artifactStore.artifactDir("green-key") } returns artifactDir
|
||||
|
||||
val page =
|
||||
mockMvc
|
||||
.perform(get("/builds/green-key"))
|
||||
.andExpect(status().isOk)
|
||||
.andReturn()
|
||||
.response.contentAsString
|
||||
|
||||
page shouldContain "build.log"
|
||||
page shouldNotContain "status-failed"
|
||||
}
|
||||
|
||||
test("legacy page names redirect permanently to the new routes") {
|
||||
mockMvc
|
||||
.perform(get("/index.html"))
|
||||
|
||||
Reference in New Issue
Block a user