Skip to content

Commit 3db2f91

Browse files
authored
fix: join the build lifecycle with dependsOn, not finalizedBy (#89)
Since 0.1.2 a consumer that declares its own `build.dependsOn(assembleRequirements)` fails at configuration time: Circular dependency between the following tasks: :assembleRequirements \--- :build \--- :assembleRequirements (*) Nothing compiles -- the build dies before any task runs. 0.1.2 added `build.finalizedBy(assembleRequirements)`, which says the task runs AFTER build. The consumer's line says before. Gradle rejects the contradiction. Consumers had every reason to have written that line: 0.1.0 and 0.1.1 shipped no lifecycle wiring at all, while the 0.1.0 README documented the task as depending on `check` and running as part of `build`. They wired by hand to get the documented behaviour, and 0.1.2 added the opposing edge with no migration note. Wiring `build dependsOn assembleRequirements dependsOn check` instead keeps the same execution order -- the task still runs after the tests whose XML it reads -- while composing with those consumers rather than contradicting them. dependsOn is idempotent, so their line is now redundant and harmless, and no consumer has to edit its build file. It also makes `gradle assembleRequirements` on its own meaningful: it now runs `check` first, so the zip contains test results instead of silently omitting them. Covered by BuildLifecycleWiringTest, which drives a real consumer project through TestKit with `--dry-run` -- a circular dependency is raised while the task graph is built, so the regression is reproduced without compiling or resolving anything. Verified to fail against the old wiring before being made to pass. tests/fixtures/test_project cannot cover this: it declares no wiring of its own, so it only ever exercises the case where the plugin is the sole author of these edges. Closes #88 Signed-off-by: Jimisola Laursen <jimisola@jimisola.com>
1 parent d5ee617 commit 3db2f91

8 files changed

Lines changed: 158 additions & 13 deletions

File tree

CLAUDE.md

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ src/main/java/io/github/reqstool/gradle/
2626
2727
src/test/java/io/github/reqstool/gradle/
2828
RequirementsToolTaskTest.java # unit tests (see below)
29+
BuildLifecycleWiringTest.java # TestKit: how the task joins `build` (issue #88)
2930
3031
src/test/resources/{yml,zip}/ # tracked reference fixtures (see "Fixtures" below)
3132
@@ -35,8 +36,13 @@ docs/modules/ROOT/ # Antora documentation source (published to re
3536
```
3637

3738
The main task is `assembleRequirements` (group `build`). When the `java` plugin is applied,
38-
the plugin auto-wires it to depend on `compileJava` and every non-main source set's compile
39-
task, and makes `build` finalized by `assembleRequirements`.
39+
the plugin auto-wires it to depend on `check`, on `compileJava` and on every non-main source
40+
set's compile task, and makes `build` depend on `assembleRequirements`.
41+
42+
Use `dependsOn` rather than `finalizedBy` for that last edge. `dependsOn` is idempotent, so
43+
a consumer that wired the lifecycle by hand — as 0.1.0 and 0.1.1 required — merely repeats
44+
an edge that already exists. `finalizedBy` states the opposite order and Gradle rejects the
45+
pair as a circular dependency at configuration time; that was issue #88.
4046

4147
## Running tests
4248

@@ -61,6 +67,21 @@ Jackson (`ObjectMapper` + `YAMLFactory`) — it does **not** read from `src/test
6167
| `testDeprecatedSvcsAnnotationsFileSetter` | The deprecated singular `setSvcsAnnotationsFile()` still works and delegates to `svcsAnnotationsFiles` |
6268
| `testMissingRequirementsFile` | `task.execute()` throws an exception whose message mentions `requirements.yml` when the dataset directory doesn't contain it |
6369

70+
## Lifecycle tests — `BuildLifecycleWiringTest`
71+
72+
TestKit (`GradleRunner`) over a throwaway consumer project, run with `--dry-run`: a circular
73+
dependency is raised while Gradle builds the task graph, so the failure is reproduced
74+
without compiling or resolving anything.
75+
76+
| Test | Validates |
77+
|---|---|
78+
| `consumerDeclaringBuildDependsOnAssembleRequirementsStillConfigures` | A consumer's own `build.dependsOn(assembleRequirements)` does not collide with the plugin's wiring — the regression in #88 |
79+
| `assembleRequirementsIsPartOfBuildWithoutConsumerWiring` | The plugin alone still puts the task into `build` |
80+
| `assembleRequirementsRunsAfterCheck` | `check` is ordered first, since the task reads test-result XML |
81+
82+
`tests/fixtures/test_project` cannot cover this: it declares no wiring of its own, so it only
83+
ever exercises the case where the plugin is the sole author of these edges.
84+
6485
## Fixtures
6586

6687
### `src/test/resources/{yml,zip}/`

README.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ Gradle build plugin for [reqstool](https://github.com/reqstool/reqstool-client)
1212

1313
Collects `@Requirements` and `@SVCs` annotations from compiled Java code, combines them with test results, and packages everything into a ZIP artifact for analysis by the reqstool CLI. Supports Java 21+.
1414

15-
The plugin automatically wires task dependencies: `assembleRequirements` depends on all `compileJava` tasks, and `build` is finalized by `assembleRequirements`. No manual task wiring is needed in most projects.
15+
The plugin automatically wires task dependencies: `assembleRequirements` depends on `check` and on all `compileJava` tasks, and `build` depends on `assembleRequirements`. No manual task wiring is needed in most projects.
1616

1717
## Installation
1818

@@ -109,7 +109,8 @@ Combines requirements and SVCs annotations from all source sets, writes a merged
109109

110110
- Depends on `compileJava` (main source set)
111111
- Depends on `compileXxxJava` for each non-main source set (unless `svcsAnnotationsFiles` was set explicitly)
112-
- `build` is finalized by `assembleRequirements`
112+
- `assembleRequirements` depends on `check`
113+
- `build` depends on `assembleRequirements`
113114

114115
## Usage
115116

build.gradle

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ dependencies {
5050
testImplementation 'io.github.reqstool:reqstool-java-annotations:1.0.1'
5151
testAnnotationProcessor 'io.github.reqstool:reqstool-java-annotations:1.0.1'
5252

53+
// TestKit runs a real Gradle build, which is the only way to observe a
54+
// configuration-time circular dependency (see BuildLifecycleWiringTest).
55+
testImplementation gradleTestKit()
56+
5357
// Testing - JUnit BOM for version management
5458
testImplementation platform('org.junit:junit-bom:6.1.3')
5559
testImplementation 'org.junit.jupiter:junit-jupiter-api'

docs/modules/ROOT/pages/configuration.adoc

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,12 @@ When the `java` plugin is applied, the plugin automatically wires:
166166

167167
* `assembleRequirements` depends on `compileJava` (main source set)
168168
* `assembleRequirements` depends on `compileXxxJava` for each non-main source set (unless `svcsAnnotationsFiles` was set explicitly via `setSvcsAnnotationsFiles(...)`)
169-
* `build` is finalized by `assembleRequirements`
169+
* `assembleRequirements` depends on `check`, so test results exist when it runs
170+
* `build` depends on `assembleRequirements`
170171

171-
No manual `dependsOn` or `finalizedBy` blocks are needed in the consuming project.
172+
No manual `dependsOn` blocks are needed in the consuming project. If you already have
173+
`build.dependsOn(assembleRequirements)` from an earlier version, it is now redundant but
174+
harmless -- `dependsOn` is idempotent.
172175

173176
If an annotation file is missing at execution time (e.g. when a compile task was excluded), the plugin emits a `WARN` message identifying the missing file.
174177

docs/modules/ROOT/pages/index.adoc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ This Gradle plugin generates a ZIP artifact containing combined annotations and
55
== Features
66

77
* *Zero-configuration*: Works out of the box with sensible defaults
8-
* *Automatic task wiring*: `assembleRequirements` depends on all compile tasks; `build` is finalized by `assembleRequirements`
8+
* *Automatic task wiring*: `assembleRequirements` depends on `check` and all compile tasks; `build` depends on `assembleRequirements`
99
* *Multi-source-set support*: SVCs annotation files are auto-discovered from all non-main source sets
1010
* *Maven publishing support*: Automatically registers ZIP artifact for publication
1111
* *Combines annotations*: Merges requirements annotations from implementation and test code

docs/modules/ROOT/pages/usage.adoc

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ then (unless `skipAssembleZipArtifact` is set) assembles a ZIP artifact at
6767

6868
* Depends on `compileJava` (main source set — generates `requirementsAnnotationsFile`)
6969
* Depends on `compileXxxJava` for each non-main source set (generates entries in `svcsAnnotationsFiles`)
70-
* `build` is finalized by `assembleRequirements`
70+
* Depends on `check`, so the test results it reads have been produced
71+
* `build` depends on `assembleRequirements`
7172

7273
These dependencies ensure annotation processors run before the task executes.
7374
No manual wiring is needed unless you override annotation file paths with `setSvcsAnnotationsFiles(...)`.
@@ -77,10 +78,11 @@ No manual wiring is needed unless you override annotation file paths with `setSv
7778
[source]
7879
----
7980
build
80-
└── (finalizedBy) assembleRequirements
81-
├── (dependsOn) compileJava
82-
├── (dependsOn) compileTestJava
83-
└── (dependsOn) compileIntegrationTestJava (if source set exists)
81+
└── (dependsOn) assembleRequirements
82+
├── (dependsOn) check
83+
├── (dependsOn) compileJava
84+
├── (dependsOn) compileTestJava
85+
└── (dependsOn) compileIntegrationTestJava (if source set exists)
8486
----
8587

8688
ZIP artifact is written to `build/reqstool/<name>-<version>-reqstool.zip`.

src/main/java/io/github/reqstool/gradle/RequirementsToolPlugin.java

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
import org.gradle.api.Plugin;
55
import org.gradle.api.Project;
6+
import org.gradle.api.plugins.JavaBasePlugin;
67
import org.gradle.api.plugins.JavaPlugin;
78
import org.gradle.api.plugins.JavaPluginExtension;
89
import org.gradle.api.publish.PublishingExtension;
@@ -81,7 +82,19 @@ public void apply(Project project) {
8182
.forEach(ss -> assembleTask
8283
.configure(task -> task.dependsOn(project.getTasks().named(ss.getCompileJavaTaskName()))));
8384
}
84-
project.getTasks().named("build").configure(build -> build.finalizedBy(assembleTask));
85+
// `build dependsOn assembleRequirements dependsOn check`, not
86+
// `build finalizedBy assembleRequirements`. Both put the task after the
87+
// tests, which it needs -- it reads their XML output -- but only this
88+
// one composes with a consumer that already wired the lifecycle by
89+
// hand. dependsOn is idempotent, so a consumer repeating either edge
90+
// changes nothing; finalizedBy contradicts a consumer's
91+
// `build.dependsOn(assembleRequirements)` and Gradle rejects the pair
92+
// as a circular dependency at configuration time, failing the build
93+
// before anything compiles. Versions 0.1.0 and 0.1.1 wired nothing at
94+
// all, so consumers had every reason to write that line themselves.
95+
// See reqstool/reqstool-java-gradle-plugin#88.
96+
assembleTask.configure(task -> task.dependsOn(p.getTasks().named(JavaBasePlugin.CHECK_TASK_NAME)));
97+
p.getTasks().named("build").configure(build -> build.dependsOn(assembleTask));
8598
});
8699
});
87100

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
// Copyright © LFV
2+
package io.github.reqstool.gradle;
3+
4+
import java.io.IOException;
5+
import java.nio.file.Files;
6+
import java.nio.file.Path;
7+
8+
import org.gradle.testkit.runner.BuildResult;
9+
import org.gradle.testkit.runner.GradleRunner;
10+
import org.junit.jupiter.api.Test;
11+
import org.junit.jupiter.api.io.TempDir;
12+
13+
import static org.junit.jupiter.api.Assertions.assertFalse;
14+
import static org.junit.jupiter.api.Assertions.assertTrue;
15+
16+
/**
17+
* Covers how the plugin joins the build lifecycle, which the fixture under
18+
* {@code tests/fixtures} cannot: the fixture declares no wiring of its own, so it only
19+
* ever exercises the case where the plugin is the sole author of these edges.
20+
*
21+
* <p>
22+
* {@code --dry-run} is deliberate. A circular dependency is raised while Gradle builds
23+
* the task graph, before any task runs, so the failure these tests guard against is
24+
* reproduced without compiling anything or resolving a single dependency.
25+
*
26+
* @see <a href= "https://github.com/reqstool/reqstool-java-gradle-plugin/issues/88">issue
27+
* #88</a>
28+
*/
29+
class BuildLifecycleWiringTest {
30+
31+
@TempDir
32+
Path projectDir;
33+
34+
/**
35+
* Versions 0.1.0 and 0.1.1 wired nothing, so a consumer that wanted the documented
36+
* behaviour declared it by hand. That line must not collide with the plugin's own
37+
* wiring: 0.1.2 made {@code build} finalized by {@code assembleRequirements}, which
38+
* contradicts it, and every such consumer failed at configuration time with "Circular
39+
* dependency between the following tasks".
40+
*/
41+
@Test
42+
void consumerDeclaringBuildDependsOnAssembleRequirementsStillConfigures() throws IOException {
43+
writeProject("""
44+
tasks.named('assembleRequirements') { dependsOn(tasks.named('check')) }
45+
tasks.named('build') { dependsOn(tasks.named('assembleRequirements')) }
46+
""");
47+
48+
BuildResult result = run();
49+
50+
assertFalse(result.getOutput().contains("Circular dependency"),
51+
"consumer-declared build.dependsOn(assembleRequirements) must not collide "
52+
+ "with the plugin's own wiring:\n" + result.getOutput());
53+
assertTrue(result.getOutput().contains(":assembleRequirements"),
54+
"assembleRequirements should be in the task graph for `build`:\n" + result.getOutput());
55+
}
56+
57+
/** The plugin on its own still puts the task into {@code build}. */
58+
@Test
59+
void assembleRequirementsIsPartOfBuildWithoutConsumerWiring() throws IOException {
60+
writeProject("");
61+
62+
BuildResult result = run();
63+
64+
assertTrue(result.getOutput().contains(":assembleRequirements"),
65+
"assembleRequirements should run as part of `build`:\n" + result.getOutput());
66+
}
67+
68+
/**
69+
* The task reads the test-results XML, so it has to come after the tests rather than
70+
* merely alongside them.
71+
*/
72+
@Test
73+
void assembleRequirementsRunsAfterCheck() throws IOException {
74+
writeProject("");
75+
76+
String output = run().getOutput();
77+
78+
assertTrue(output.indexOf(":check") < output.indexOf(":assembleRequirements"),
79+
"check should be ordered before assembleRequirements:\n" + output);
80+
}
81+
82+
private BuildResult run() {
83+
return GradleRunner.create()
84+
.withProjectDir(projectDir.toFile())
85+
.withPluginClasspath()
86+
.withArguments("build", "--dry-run")
87+
.build();
88+
}
89+
90+
private void writeProject(String extraWiring) throws IOException {
91+
Files.writeString(projectDir.resolve("settings.gradle"), "rootProject.name = 'consumer'\n");
92+
Files.writeString(projectDir.resolve("build.gradle"), """
93+
plugins {
94+
id 'java'
95+
id 'io.github.reqstool.gradle-plugin'
96+
}
97+
98+
""" + extraWiring);
99+
}
100+
101+
}

0 commit comments

Comments
 (0)