miandmhoennig
978f1e80d8
- cleanupStaleContainers 45 -> 2, UiFormats.cpuTime 43.8 -> 7, monitoredDisks 42 -> 2: tests reaching every branch; `ContainerInfo.isWerkatorLeftover`, `monitoredDisksOf`, shared `InstancePath.resolve`. - Fix for the failed gated mutation build (2 INCONCLUSIVE from MEMORY_ERROR): `ConsoleBuildRunner.awaitTerminal` decides the end of the wait once; the nginx test sleeper fails after 100 rounds. Local PIT: 0 MEMORY_ERROR, 0 INCONCLUSIVE. - Nightly gate 46 -> 36 (highest left: SystemMetricsCollector.sample 35.36). PR-doc: `docs/prs/2026-10-04-PR#110-nginx-cleanup-cpu-time-disks.md` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: mhoennig <michael@hoennig.de> Reviewed-on: #110
107 lines
6.4 KiB
Markdown
107 lines
6.4 KiB
Markdown
> **WARNING:** This document describes only the change applied in this PR.
|
|
> It may already be outdated once the next PR is merged.
|
|
> Historic PR-documentation is not maintained along with new PRs — treat it as a snapshot, not as current documentation.
|
|
|
|
## The Problem
|
|
|
|
After PR#109 the three highest Mutation-CRAP scores of Werkator were unrelated, each small:
|
|
|
|
| Method | Mutation-CRAP | Killed / eligible | Why |
|
|
| ------------------------------------------ | ------------- | ----------------- | ----------------------------------------------------------------------------------------------------- |
|
|
| `NginxProxyManager.cleanupStaleContainers` | 45.04 | 1 / 6 | tested, but the tests passed whatever it did: the start removes the container by name once more later |
|
|
| `UiFormats.cpuTime` | 43.81 | 1 / 11 | no test of its own |
|
|
| `MetricsConfiguration.monitoredDisks` | 42 | 0 / 9 | read the configuration through Spring beans, no test reached it |
|
|
|
|
The first mutation build with an active gate (PR#104) failed on an analysis error instead: two methods were `INCONCLUSIVE`, because a mutant had made a bounded wait endless and filled the memory of PIT's minion (`MEMORY_ERROR`), which werkcrappit refuses to count as killed (its ADR 0006).
|
|
In `NginxProxyManager.waitForPortsFree` the mutant turned `repeat(4)` into an endless loop, and the test's sleeper did not sleep; in `ConsoleBuildRunner.buildAndStream` the end of the wait was asked twice, and a mutant of the first question alone made it endless and without a pause.
|
|
Tonight's mutation build would have failed the same way.
|
|
|
|
The rule for a path in the instance file — `~` is the home directory, anything relative is relative to it — was written twice, for the registry's repositories and for the System page's disks.
|
|
|
|
## Non-Goals
|
|
|
|
- No change in behaviour.
|
|
|
|
## The Scenarios
|
|
|
|
### Feature: Three small decisions, held by tests
|
|
|
|
#### Scenario#110.01: Processor time reads with its factor against the wall clock
|
|
|
|
So that `4:07 (2.4x)` stays what the artifact page shows.
|
|
|
|
- **Given** processor times with and without a duration, a zero duration, nothing measured, and a negative time
|
|
- **When** they are formatted
|
|
- **Then** the factor appears only with a duration above zero, and nothing measured shows nothing
|
|
|
|
##### Verified by
|
|
|
|
- [UiViewsUnitTest](../../src/test/kotlin/de/hoennig/werkator/server/UiViewsUnitTest.kt) — Scenario#110.01
|
|
|
|
#### Scenario#110.02: The System page shows the configured disks, their paths read like every path of the instance file
|
|
|
|
So that `~`, `~/…` and relative paths mean the same for a disk as for a repository.
|
|
|
|
- **Given** no `metrics.disks`, and disks with `~`, an absolute path, `~/…` with a blank label, and a relative path
|
|
- **When** the monitored disks are resolved
|
|
- **Then** without the section the checkout's filesystem is `Disk`; otherwise the disks keep their order, paths resolve against the home directory, and a blank label falls back to the key
|
|
- **And** only `~` and `~/` expand, `~other` is a name like any other
|
|
|
|
##### Verified by
|
|
|
|
- [MonitoredDisksUnitTest](../../src/test/kotlin/de/hoennig/werkator/metrics/MonitoredDisksUnitTest.kt) — Scenario#110.02
|
|
|
|
#### Scenario#110.03: Only Werkator's own leftovers are removed before the managed nginx starts
|
|
|
|
So that a stale container of Werkator never blocks a port, and someone else's container is never touched.
|
|
|
|
- **Given** a container of the configured name, labelled nginx containers of this repository, and containers on and off the nginx ports
|
|
- **When** the managed nginx starts
|
|
- **Then** the container of the name goes first, then the labelled ones; a failed or empty listing removes nothing by label
|
|
- **And** on an nginx port only a container labelled as Werkator's, or named `werkator-…` or `git-watch-origin-and-test-nginx-…`, is removed
|
|
|
|
##### Verified by
|
|
|
|
- [NginxProxyManagerUnitTest](../../src/test/kotlin/de/hoennig/werkator/server/NginxProxyManagerUnitTest.kt) — Scenario#110.03
|
|
|
|
#### Scenario#110.04: A wait that a mutant makes endless fails a test instead of filling the memory
|
|
|
|
So that the mutation gate judges test quality, not memory, and the nightly build can pass.
|
|
|
|
- **Given** the port wait of the managed nginx and the console's wait for a build
|
|
- **When** PIT runs on both classes
|
|
- **Then** no mutant ends with `MEMORY_ERROR`, and no method is `INCONCLUSIVE`
|
|
|
|
##### Verified by
|
|
|
|
- [NginxProxyManagerUnitTest](../../src/test/kotlin/de/hoennig/werkator/server/NginxProxyManagerUnitTest.kt): its sleeper fails after 100 rounds
|
|
- [ConsoleBuildRunnerUnitTest](../../src/test/kotlin/de/hoennig/werkator/commands/ConsoleBuildRunnerUnitTest.kt)
|
|
- A local PIT run on both classes: 0 `MEMORY_ERROR`, 0 `INCONCLUSIVE`
|
|
|
|
## The Solution
|
|
|
|
- `NginxProxyManager.ContainerInfo.isWerkatorLeftover` names the rule for a stale Werkator container; `removeLabelledContainers` the removal by label; `cleanupStaleContainers` reads as the three steps.
|
|
- `MetricsConfiguration.monitoredDisksOf` resolves the disks without Spring; `InstancePath.resolve` is the one rule for a path in the instance file, used by `RepositoryEntry.directory` as well.
|
|
- `UiFormats.cpuTime` gets the test it lacked.
|
|
- `ConsoleBuildRunner.awaitTerminal` decides the end of the wait at one place; the test sleeper of `NginxProxyManagerUnitTest` fails after 100 rounds.
|
|
|
|
Measured with PIT as the nightly build runs it:
|
|
|
|
| Method | Before | After |
|
|
| ------------------------------------------ | ------ | ----- |
|
|
| `NginxProxyManager.cleanupStaleContainers` | 45.04 | 2 |
|
|
| `UiFormats.cpuTime` | 43.81 | 7.04 |
|
|
| `MetricsConfiguration.monitoredDisks` | 42 | 2 |
|
|
| `MetricsConfiguration.monitoredDisksOf` | — | 4 |
|
|
| `ContainerInfo.isWerkatorLeftover` | — | 3.03 |
|
|
|
|
The gate `maxMutationCrap` falls from 46 to 36: the highest score left is `SystemMetricsCollector.sample` at 35.36.
|
|
|
|
## Additional Changes
|
|
|
|
- Version 1.34.7 with its release note.
|
|
|
|
## Follow-up PRs
|
|
|
|
- The methods above 15 by complexity alone: `SystemMetricsCollector.sample` (35), `BuildDefinition.applyTo` (34), `ForgejoWorkflowExporter.settingNotes` (27), `SystemMetricsView.from` (24), and the `ConfigLoader` checks.
|