diff --git a/build.gradle.kts b/build.gradle.kts index 7ab0dc6..c547a42 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -9,8 +9,9 @@ plugins { } group = "de.hoennig" -// bump at least the patch version for every deployment, so the UI footer -// (BuildProperties) and --version identify what is actually running +// bump at least the patch version for every deployment — and only then, not per commit — +// so the UI footer (BuildProperties), --version and the release notes identify what is +// actually running; a deployment bundles whatever was committed since the last one version = "0.9.10" java { diff --git a/src/main/kotlin/de/hoennig/gittally/server/UiController.kt b/src/main/kotlin/de/hoennig/gittally/server/UiController.kt index 1406d47..82f7e5e 100644 --- a/src/main/kotlin/de/hoennig/gittally/server/UiController.kt +++ b/src/main/kotlin/de/hoennig/gittally/server/UiController.kt @@ -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()) + model.addAttribute( + "logs", + artifactDir?.let { logFiles(it, scanForFailure = result != null && result.status != BuildStatus.SUCCESS) } + ?: emptyList(), + ) model.addAttribute("reportIndexes", artifactDir?.let { reportIndexes(it) } ?: emptyList()) 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 = + /** + * 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 = 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*
(\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( diff --git a/src/main/kotlin/de/hoennig/gittally/server/UiViews.kt b/src/main/kotlin/de/hoennig/gittally/server/UiViews.kt index 3a164af..f1de909 100644 --- a/src/main/kotlin/de/hoennig/gittally/server/UiViews.kt +++ b/src/main/kotlin/de/hoennig/gittally/server/UiViews.kt @@ -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, diff --git a/src/main/resources/templates/artifact.html b/src/main/resources/templates/artifact.html index b282dd9..08b7b64 100644 --- a/src/main/resources/templates/artifact.html +++ b/src/main/resources/templates/artifact.html @@ -50,8 +50,10 @@

Logs

diff --git a/src/test/kotlin/de/hoennig/gittally/server/UiControllerTest.kt b/src/test/kotlin/de/hoennig/gittally/server/UiControllerTest.kt index f3047bc..e71301b 100644 --- a/src/test/kotlin/de/hoennig/gittally/server/UiControllerTest.kt +++ b/src/test/kotlin/de/hoennig/gittally/server/UiControllerTest.kt @@ -335,6 +335,46 @@ class UiControllerTest : FunSpec() { Regex(""" failed""").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""").findAll(page).count() shouldBe 3 + // lower-case prose about failing is not a failure line + page.substringAfter("build.stderr.log").substringBefore("") 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"))