> Historic PR-documentation is not maintained along with new PRs — treat it as a snapshot, not as current documentation. ## The Problem PR#117 added a fourth condition to `ConfigLoader.isTriggered`. A local PIT run of the classes changed by PR#117 to PR#119 measured these methods above the Kotlin target of 15: | Method | Mutation-CRAP | Complexity | Mutation score | |------------------------------------------------|---------------|------------|----------------| | `ConfigLoader.isTriggered` | 34.12 | 13 | 0.50 | | `ConfigLoader.withoutPullRequestTargets` | 17.58 | 7 | 0.40 | | `ConfigLoader.checkPullRequestTargetSandboxes` | 15.05 | 15 | 0.94 | `isTriggered` is above the nightly gate of 31, so tonight's mutation build would fail. Its only effect is a warning, and no test looked at that warning; the same holds for the warnings of `withoutPullRequestTargets`. ## Non-Goals - No change in behaviour: every check refuses and warns exactly as before, with the same texts. - The other methods above 15 follow in their own PRs. ## The Scenarios ### Feature: the checks that only warn are tested by their warnings #### Scenario#120.01: definitions that nothing starts are said once, and every kind of trigger silences it So that the warning on an instance that never builds keeps coming exactly when it should. - **Given** `builds.default` without a trigger - **and** a definition `nightly` - **When** `nightly` has no trigger, an empty `trigger` block, or one whose `onPush`, `atTimes` and `afterSuccessOf` say nothing - **Then** the configuration loads with one warning that the watcher will never start a build, also when loaded twice - **When** `nightly` has `onPush: true`, an `atTimes` entry, an `afterSuccessOf`, or `onPullRequestTarget: true` - **Then** there is no such warning ##### Verified by - [ConfigLoaderUnitTest](../../src/test/kotlin/de/hoennig/werkator/config/ConfigLoaderUnitTest.kt) `definitions that nothing starts are said once, and every kind of trigger silences it (Scenario#120.01)` #### Scenario#120.02: a branch's pull-request target definitions are ignored with one warning each So that a branch author learns why the change to the review build has no effect. - **Given** the host defines the pull-request target build `review` - **When** a branch layer changes `review` and adds its own pull-request target `sneaky` - **Then** each of the two is ignored with its own warning, naming the branch, or "this branch" without a name - **and** the same warning is not repeated for the same branch ##### Verified by - [PullRequestTargetConfigUnitTest](../../src/test/kotlin/de/hoennig/werkator/config/PullRequestTargetConfigUnitTest.kt) `a branch's pull-request target definitions are ignored with one warning each, naming the branch (Scenario#120.02)`, and the Scenario#117.04 tests for what is ignored #### Scenario#120.03: the sandbox check reads each pull-request target on its own So that the check of PR#118 stays exact after the split. - **Given** a pull-request target build in werkdock with a disabled `docker` block that says `socket: true` - **When** the configuration loads - **Then** it is accepted, since the socket belongs to a container that does not run - **And** a target build without a sandbox, or in Docker with the socket, is still refused ##### Verified by - [PullRequestTargetConfigUnitTest](../../src/test/kotlin/de/hoennig/werkator/config/PullRequestTargetConfigUnitTest.kt) `a pull-request target build in werkdock passes, whatever a disabled docker block says (Scenario#120.03)`, and the Scenario#118.03 tests ## The Solution `triggerOf` reads the `trigger` block of a raw definition once for `isTriggered`, `predecessorOf` and `isPullRequestTarget`. `isTriggered` asks three questions in one line; the push and the schedule moved to `startsItself`. `withoutPullRequestTargets` gives both kinds of warning through one `warnIgnored`, instead of two loops. `checkPullRequestTargetSandboxes` merges `builds.default` into each target and hands it to `checkPullRequestTargetSandbox`, which holds the two refusals. The tests capture the log of `ConfigLoader` through `captureConfigLog`. A local PIT run of `ConfigLoader` afterwards: | Method | Mutation-CRAP | Complexity | Mutation score | |------------------------------------------------|---------------|------------|----------------| | `ConfigLoader.isTriggered` | 4 | 4 | 1.00 | | `ConfigLoader.startsItself` | 7 | 7 | 1.00 | | `ConfigLoader.triggerOf` | 4 | 4 | 1.00 | | `ConfigLoader.withoutPullRequestTargets` | 5 | 5 | 1.00 | | `ConfigLoader.warnIgnored` | 1 | 1 | 1.00 | | `ConfigLoader.checkPullRequestTargetSandboxes` | 5.03 | 5 | 0.89 | | `ConfigLoader.checkPullRequestTargetSandbox` | 10 | 10 | 1.00 | The highest score left in `ConfigLoader` is `checkRestartsInstance` at 15. The project maximum stays `GitService.authenticated` at 30.12, so the gate stays at 31. ## Additional Changes - Version 1.37.1 with its entry in `releases.html`. - The integration test of Scenario#119.01 failed on CI: its review ran `echo` and ended before the test polled the running builds. The review now sleeps 2 s, so the test sees both builds running at once. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: mhoennig <michael@hoennig.de> Reviewed-on: #120
103 lines
5.8 KiB
Markdown
103 lines
5.8 KiB
Markdown
# PR#120: Trigger and Pull-Request Target Checks of the Configuration Split Up
|
|
|
|
> **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
|
|
|
|
PR#117 added a fourth condition to `ConfigLoader.isTriggered`.
|
|
A local PIT run of the classes changed by PR#117 to PR#119 measured these methods above the Kotlin target of 15:
|
|
|
|
| Method | Mutation-CRAP | Complexity | Mutation score |
|
|
|------------------------------------------------|---------------|------------|----------------|
|
|
| `ConfigLoader.isTriggered` | 34.12 | 13 | 0.50 |
|
|
| `ConfigLoader.withoutPullRequestTargets` | 17.58 | 7 | 0.40 |
|
|
| `ConfigLoader.checkPullRequestTargetSandboxes` | 15.05 | 15 | 0.94 |
|
|
|
|
`isTriggered` is above the nightly gate of 31, so tonight's mutation build would fail.
|
|
Its only effect is a warning, and no test looked at that warning; the same holds for the warnings of `withoutPullRequestTargets`.
|
|
|
|
## Non-Goals
|
|
|
|
- No change in behaviour: every check refuses and warns exactly as before, with the same texts.
|
|
- The other methods above 15 follow in their own PRs.
|
|
|
|
## The Scenarios
|
|
|
|
### Feature: the checks that only warn are tested by their warnings
|
|
|
|
#### Scenario#120.01: definitions that nothing starts are said once, and every kind of trigger silences it
|
|
|
|
So that the warning on an instance that never builds keeps coming exactly when it should.
|
|
|
|
- **Given** `builds.default` without a trigger
|
|
- **and** a definition `nightly`
|
|
- **When** `nightly` has no trigger, an empty `trigger` block, or one whose `onPush`, `atTimes` and `afterSuccessOf` say nothing
|
|
- **Then** the configuration loads with one warning that the watcher will never start a build, also when loaded twice
|
|
- **When** `nightly` has `onPush: true`, an `atTimes` entry, an `afterSuccessOf`, or `onPullRequestTarget: true`
|
|
- **Then** there is no such warning
|
|
|
|
##### Verified by
|
|
|
|
- [ConfigLoaderUnitTest](../../src/test/kotlin/de/hoennig/werkator/config/ConfigLoaderUnitTest.kt) `definitions that nothing starts are said once, and every kind of trigger silences it (Scenario#120.01)`
|
|
|
|
#### Scenario#120.02: a branch's pull-request target definitions are ignored with one warning each
|
|
|
|
So that a branch author learns why the change to the review build has no effect.
|
|
|
|
- **Given** the host defines the pull-request target build `review`
|
|
- **When** a branch layer changes `review` and adds its own pull-request target `sneaky`
|
|
- **Then** each of the two is ignored with its own warning, naming the branch, or "this branch" without a name
|
|
- **and** the same warning is not repeated for the same branch
|
|
|
|
##### Verified by
|
|
|
|
- [PullRequestTargetConfigUnitTest](../../src/test/kotlin/de/hoennig/werkator/config/PullRequestTargetConfigUnitTest.kt) `a branch's pull-request target definitions are ignored with one warning each, naming the branch (Scenario#120.02)`, and the Scenario#117.04 tests for what is ignored
|
|
|
|
#### Scenario#120.03: the sandbox check reads each pull-request target on its own
|
|
|
|
So that the check of PR#118 stays exact after the split.
|
|
|
|
- **Given** a pull-request target build in werkdock with a disabled `docker` block that says `socket: true`
|
|
- **When** the configuration loads
|
|
- **Then** it is accepted, since the socket belongs to a container that does not run
|
|
- **And** a target build without a sandbox, or in Docker with the socket, is still refused
|
|
|
|
##### Verified by
|
|
|
|
- [PullRequestTargetConfigUnitTest](../../src/test/kotlin/de/hoennig/werkator/config/PullRequestTargetConfigUnitTest.kt) `a pull-request target build in werkdock passes, whatever a disabled docker block says (Scenario#120.03)`, and the Scenario#118.03 tests
|
|
|
|
## The Solution
|
|
|
|
`triggerOf` reads the `trigger` block of a raw definition once for `isTriggered`, `predecessorOf` and `isPullRequestTarget`.
|
|
`isTriggered` asks three questions in one line; the push and the schedule moved to `startsItself`.
|
|
`withoutPullRequestTargets` gives both kinds of warning through one `warnIgnored`, instead of two loops.
|
|
`checkPullRequestTargetSandboxes` merges `builds.default` into each target and hands it to `checkPullRequestTargetSandbox`, which holds the two refusals.
|
|
|
|
The tests capture the log of `ConfigLoader` through `captureConfigLog`.
|
|
|
|
A local PIT run of `ConfigLoader` afterwards:
|
|
|
|
| Method | Mutation-CRAP | Complexity | Mutation score |
|
|
|------------------------------------------------|---------------|------------|----------------|
|
|
| `ConfigLoader.isTriggered` | 4 | 4 | 1.00 |
|
|
| `ConfigLoader.startsItself` | 7 | 7 | 1.00 |
|
|
| `ConfigLoader.triggerOf` | 4 | 4 | 1.00 |
|
|
| `ConfigLoader.withoutPullRequestTargets` | 5 | 5 | 1.00 |
|
|
| `ConfigLoader.warnIgnored` | 1 | 1 | 1.00 |
|
|
| `ConfigLoader.checkPullRequestTargetSandboxes` | 5.03 | 5 | 0.89 |
|
|
| `ConfigLoader.checkPullRequestTargetSandbox` | 10 | 10 | 1.00 |
|
|
|
|
The highest score left in `ConfigLoader` is `checkRestartsInstance` at 15.
|
|
The project maximum stays `GitService.authenticated` at 30.12, so the gate stays at 31.
|
|
|
|
## Additional Changes
|
|
|
|
- Version 1.37.1 with its entry in `releases.html`.
|
|
- The integration test of Scenario#119.01 failed on CI: its review ran `echo` and ended before the test polled the running builds.
|
|
The review now sleeps 2 s, so the test sees both builds running at once.
|
|
|
|
🤖 Generated with [Claude Code](https://claude.com/claude-code)
|