Skip to content

Commit 21288f7

Browse files
odrotbohmodrotbohm
committed
GH-1438 - Polishing.
Original pull request: GH-1484.
1 parent 0119c85 commit 21288f7

File tree

9 files changed

+80
-36
lines changed

9 files changed

+80
-36
lines changed

‎spring-modulith-integration-test/src/test/java/org/springframework/modulith/junit/TestExecutionConditionUnitTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ private void assertEnabled(Class<?> type, boolean expected, String... files) {
9090

9191
private void assertEnabled(Class<?> type, boolean expected, Stream<ModifiedFile> files) {
9292

93-
assertThat(condition.evaluate(new ConditionContext(type, Changes.of(files, OnNoChange.DEFAULT))))
93+
assertThat(condition.evaluate(new ConditionContext(type, Changes.of(files, OnNoChange.EXECUTE_ALL))))
9494
.extracting(ConditionEvaluationResult::isDisabled)
9595
.isNotEqualTo(expected);
9696
}

‎spring-modulith-junit/src/main/java/org/springframework/modulith/junit/Changes.java‎

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
package org.springframework.modulith.junit;
1717

1818
import static java.util.stream.Collectors.*;
19-
import static org.springframework.modulith.junit.Changes.OnNoChange.EXECUTE_NO_TESTS;
19+
import static org.springframework.modulith.junit.Changes.OnNoChange.*;
2020

2121
import java.util.Collection;
2222
import java.util.Collections;
@@ -26,6 +26,7 @@
2626
import java.util.stream.Collectors;
2727
import java.util.stream.Stream;
2828

29+
import org.jspecify.annotations.Nullable;
2930
import org.springframework.modulith.junit.Changes.Change;
3031
import org.springframework.modulith.junit.Changes.Change.OtherFileChange;
3132
import org.springframework.modulith.junit.Changes.Change.SourceChange;
@@ -40,26 +41,27 @@
4041
* @author Lukas Dohmen
4142
* @author David Bilge
4243
* @author Oliver Drotbohm
44+
* @author Valentin Bossi
4345
*/
4446
public class Changes implements Iterable<Change> {
4547

46-
public static final Changes NONE = new Changes(Collections.emptySet(), OnNoChange.DEFAULT);
48+
public static final Changes NONE = new Changes(Collections.emptySet(), OnNoChange.EXECUTE_ALL);
4749

4850
private final Collection<Change> changes;
49-
private final OnNoChange onNoChangeConfig;
51+
private final OnNoChange onNoChange;
5052

5153
/**
5254
* Creates a new {@link Changes} instance from the given {@link Change}s.
5355
*
5456
* @param changes must not be {@literal null}.
5557
*/
56-
private Changes(Collection<Change> changes, OnNoChange config) {
58+
private Changes(Collection<Change> changes, OnNoChange onNoChange) {
5759

5860
Assert.notNull(changes, "Changes must not be null!");
59-
Assert.notNull(config, "OnNoChange must not be null!");
61+
Assert.notNull(onNoChange, "OnNoChange must not be null!");
6062

6163
this.changes = changes;
62-
this.onNoChangeConfig = config;
64+
this.onNoChange = onNoChange;
6365
}
6466

6567
/**
@@ -68,11 +70,12 @@ private Changes(Collection<Change> changes, OnNoChange config) {
6870
* @param files must not be {@literal null}.
6971
* @return will never be {@literal null}.
7072
*/
71-
static Changes of(Stream<ModifiedFile> files, OnNoChange config) {
73+
static Changes of(Stream<ModifiedFile> files, OnNoChange onNoChange) {
7274

7375
Assert.notNull(files, "Modified files must not be null!");
7476

75-
return files.map(Change::of).collect(collectingAndThen(toSet(), changes -> new Changes(changes, config)));
77+
return files.map(Change::of)
78+
.collect(collectingAndThen(toSet(), changes -> new Changes(changes, onNoChange)));
7679
}
7780

7881
/**
@@ -92,8 +95,13 @@ boolean hasClassChanges() {
9295
return !getChangedClasses().isEmpty();
9396
}
9497

98+
/**
99+
* Returns whether we should skip test execution in case of no changes.
100+
*
101+
* @since 2.1
102+
*/
95103
boolean skipTestsOnNoChanges() {
96-
return onNoChangeConfig == EXECUTE_NO_TESTS;
104+
return onNoChange == SKIP_ALL;
97105
}
98106

99107
boolean contains(Class<?> type) {
@@ -337,19 +345,40 @@ public final String toString() {
337345
}
338346
}
339347

348+
/**
349+
* Whether to either execute or skip all test in case no changes where detected.
350+
*
351+
* @author Valentin Bossi
352+
* @author Oliver Drotbohm
353+
* @since 2.1
354+
*/
340355
enum OnNoChange {
341-
EXECUTE_ALL_TESTS("execute-all"),
342-
EXECUTE_NO_TESTS("execute-none");
343356

344-
final String value;
345-
static final OnNoChange DEFAULT = OnNoChange.EXECUTE_ALL_TESTS;
357+
EXECUTE_ALL("execute-all"), SKIP_ALL("skip-all");
358+
359+
private final String value;
346360

347361
OnNoChange(String value) {
348362
this.value = value;
349363
}
350364

351-
public static OnNoChange propertyConfig(String value) {
352-
return EXECUTE_NO_TESTS.value.equals(value) ? EXECUTE_NO_TESTS : DEFAULT;
365+
/**
366+
* Creates a new {@link OnNoChange} from the given configuration value.
367+
*
368+
* @param value can be {@literal null}
369+
* @return will never be {@literal null}.
370+
*/
371+
public static OnNoChange fromConfig(@Nullable String value) {
372+
373+
if (value == null) {
374+
return OnNoChange.EXECUTE_ALL;
375+
}
376+
377+
return Stream.of(OnNoChange.values())
378+
.filter(it -> it.value.equalsIgnoreCase(value))
379+
.findFirst()
380+
.orElseThrow(() -> new IllegalArgumentException("Invalid OnNoChange value! Needs to be any of %s!"
381+
.formatted(Stream.of(OnNoChange.values()).map(it -> it.value).toList())));
353382
}
354383
}
355384
}

‎spring-modulith-junit/src/main/java/org/springframework/modulith/junit/ChangesFactory.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
* Dedicated factory to create a {@link Changes} instance from an {@link Environment}.
2727
*
2828
* @author Oliver Drotbohm
29+
* @author Valentin Bossi
2930
* @since 2.1
3031
*/
3132
class ChangesFactory {
@@ -46,11 +47,11 @@ static Changes getChanges(Environment environment) {
4647
return Changes.NONE;
4748
}
4849

49-
var onNoChangesConfig = OnNoChange.propertyConfig(environment.getProperty("spring.modulith.test.on-no-changes"));
50+
var onNoChanges = OnNoChange.fromConfig(environment.getProperty("spring.modulith.test.on-no-changes"));
5051

5152
// Determine detector
5253
var detector = FileModificationDetector.getDetector(environment);
53-
var result = Changes.of(detector.getModifiedFiles(), onNoChangesConfig);
54+
var result = Changes.of(detector.getModifiedFiles(), onNoChanges);
5455

5556
if (log.isInfoEnabled()) {
5657

‎spring-modulith-junit/src/main/java/org/springframework/modulith/junit/TestExecutionCondition.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
* @author Lukas Dohmen
3838
* @author David Bilge
3939
* @author Oliver Drotbohm
40+
* @author Valentin Bossi
4041
* @see ModulithExecutionCondition
4142
*/
4243
class TestExecutionCondition {
@@ -59,9 +60,11 @@ ConditionEvaluationResult evaluate(ConditionContext context) {
5960
}
6061

6162
if (!changes.hasClassChanges()) {
63+
6264
return changes.skipTestsOnNoChanges()
63-
? disabled("No source file changes detected — tests skipped due to configuration \"on-no-changes=execute-none\".")
64-
: enabled("No source file changes detected — running full test suite due to default configuration.");
65+
? disabled(
66+
"No source file changes detected — tests skipped due to configuration \"on-no-changes=execute-none\".")
67+
: enabled("No source file changes detected — running full test suite due to default configuration.");
6568
}
6669

6770
var changedClasses = changes.getChangedClasses();

‎spring-modulith-junit/src/test/java/org/springframework/modulith/junit/ChangesFactoryUnitTests.java‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,19 @@
1717

1818
import static org.assertj.core.api.Assertions.*;
1919

20+
import java.util.Collections;
2021
import java.util.Map;
2122

2223
import org.junit.jupiter.api.Test;
2324
import org.springframework.core.env.Environment;
2425
import org.springframework.core.env.MapPropertySource;
2526
import org.springframework.core.env.StandardEnvironment;
26-
import org.springframework.modulith.junit.Changes.OnNoChange;
2727

2828
/**
2929
* Unit tests for {@link ChangesFactory}.
3030
*
3131
* @author Oliver Drotbohm
32+
* @author Valentin Bossi
3233
*/
3334
class ChangesFactoryUnitTests {
3435

@@ -44,36 +45,38 @@ void returnsNoChangesIfDisabledByProperty() {
4445
@Test // GH-1438
4546
void skipTestsOnNoChangesSetByProperty() {
4647

47-
var environment = getEnvironment(Map.of("spring.modulith.test.on-no-changes", OnNoChange.EXECUTE_NO_TESTS.value));
48+
var environment = getEnvironment(Map.of("spring.modulith.test.on-no-changes", "skip-all"));
4849

49-
assertThat(ChangesFactory.getChanges(environment).hasClassChanges()).isFalse();
5050
assertThat(ChangesFactory.getChanges(environment).skipTestsOnNoChanges()).isTrue();
5151
}
5252

5353
@Test // GH-1438
5454
void executeTestsOnNoChangesSetByProperty() {
5555

56-
var environment = getEnvironment(Map.of("spring.modulith.test.on-no-changes", OnNoChange.DEFAULT.value));
56+
var environment = getEnvironment(Map.of("spring.modulith.test.on-no-changes", "execute-all"));
5757

58-
assertThat(ChangesFactory.getChanges(environment).hasClassChanges()).isFalse();
5958
assertThat(ChangesFactory.getChanges(environment).skipTestsOnNoChanges()).isFalse();
6059
}
6160

6261
@Test // GH-1438
6362
void executeTestsByDefaultOrInvalidValue() {
6463

65-
var environment = getEnvironment(Map.of("spring.modulith.test.on-no-changes", "1"));
64+
var environment = getEnvironment(Collections.emptyMap());
6665

67-
assertThat(ChangesFactory.getChanges(environment).hasClassChanges()).isFalse();
6866
assertThat(ChangesFactory.getChanges(environment).skipTestsOnNoChanges()).isFalse();
67+
}
68+
69+
@Test // GH-1438
70+
void rejectsInvalidOnNoChangesConfigurationValue() {
6971

70-
environment = getEnvironment(Map.of());
72+
var environment = getEnvironment(Map.of("spring.modulith.test.on-no-changes", "1"));
7173

72-
assertThat(ChangesFactory.getChanges(environment).hasClassChanges()).isFalse();
73-
assertThat(ChangesFactory.getChanges(environment).skipTestsOnNoChanges()).isFalse();
74+
assertThatIllegalArgumentException().isThrownBy(() -> {
75+
ChangesFactory.getChanges(environment);
76+
});
7477
}
7578

76-
private Environment getEnvironment(Map<String, Object> properties) {
79+
private static Environment getEnvironment(Map<String, Object> properties) {
7780

7881
var propertySource = new MapPropertySource("properties", properties);
7982

‎spring-modulith-junit/src/test/java/org/springframework/modulith/junit/ChangesUnitTests.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import org.junit.jupiter.api.DynamicTest;
2323
import org.junit.jupiter.api.Test;
2424
import org.junit.jupiter.api.TestFactory;
25-
import org.springframework.core.env.StandardEnvironment;
2625
import org.springframework.modulith.junit.Changes.Change.JavaSourceChange;
2726
import org.springframework.modulith.junit.Changes.Change.JavaTestSourceChange;
2827
import org.springframework.modulith.junit.Changes.Change.KotlinSourceChange;
@@ -37,6 +36,7 @@
3736
* @author Lukas Dohmen
3837
* @author David Bilge
3938
* @author Oliver Drotbohm
39+
* @author Valentin Bossi
4040
*/
4141
class ChangesUnitTests {
4242

@@ -83,7 +83,7 @@ void shouldInterpredModifiedFilePathsCorrectly() {
8383
.map(ModifiedFile::new);
8484

8585
// when
86-
var result = Changes.of(modifiedFilePaths, OnNoChange.DEFAULT);
86+
var result = Changes.of(modifiedFilePaths, OnNoChange.EXECUTE_ALL);
8787

8888
// then
8989
assertThat(result.hasClasspathResourceChange()).isTrue();

‎spring-modulith-junit/src/test/java/org/springframework/modulith/junit/TestExecutionConditionUnitTests.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,14 @@
2828
* Unit tests for {@link TestExecutionCondition}.
2929
*
3030
* @author Oliver Drotbohm
31+
* @author Valentin Bossi
3132
*/
3233
class TestExecutionConditionUnitTests {
3334

3435
@Test // GH-1391
3536
void fallsBackToEnabledTestIfMultipleMainClassesFound() {
3637

37-
var changes = Changes.of(Stream.of(new ModifiedFile("Foo.java")), OnNoChange.DEFAULT);
38+
var changes = Changes.of(Stream.of(new ModifiedFile("Foo.java")), OnNoChange.EXECUTE_ALL);
3839
var ctx = new TestExecutionCondition.ConditionContext(getClass(), changes);
3940

4041
assertThat(new TestExecutionCondition().evaluate(ctx).isDisabled()).isFalse();
@@ -43,7 +44,7 @@ void fallsBackToEnabledTestIfMultipleMainClassesFound() {
4344
@Test // GH-1438
4445
void disablesForNoClassChangesWithPropertyConfiguration() {
4546

46-
var changes = Changes.of(Stream.of( new ModifiedFile("README.md")), OnNoChange.propertyConfig(OnNoChange.EXECUTE_NO_TESTS.value));
47+
var changes = Changes.of(Stream.of(new ModifiedFile("README.md")), OnNoChange.SKIP_ALL);
4748
var ctx = new TestExecutionCondition.ConditionContext(getClass(), changes);
4849

4950
assertThat(new TestExecutionCondition().evaluate(ctx).isDisabled()).isTrue();

‎src/docs/antora/modules/ROOT/pages/appendix.adoc‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,10 @@ As the name suggests, `uncommitted-changes` will only consider changed files not
170170
`reference-commit` will consider all files changed since a given Git commit provided via `spring.modulith.test.reference-commit`, particularly useful CI environments in which that property could point to the commit hash of the last successful build.
171171
`default` detects all uncomitted changes and ones that have not been pushed to the current branch's tracking branch which is primarily useful for local development.
172172

173+
|`spring.modulith.test.on-no-changes`
174+
|`execute-all`
175+
|Whether to execute (`execute-all`) or skip (`skip-all`) all tests in case no classpath or build resource changes have been detected.
176+
173177
|`spring.modulith.test.reference-commit`
174178
|none
175179
|The commit hash of to which to calculate the set of changed files.

‎src/docs/antora/modules/ROOT/pages/testing.adoc‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,10 @@ The optimization will back off optimizing the execution under the following circ
408408
* The test execution originates from an IDE as we assume the execution is triggered explicitly.
409409
* The set of changes contains a change to a resource related to a build system (`pom.xml`, `build.gradle(.kts)`, `gradle.properties`, and `settings.gradle(.kts)`).
410410
* The set of changes contains a change to any classpath resource.
411-
* The project does not contain a change at all (typical in a CI build).
411+
* The project does not contain a change at all.
412+
413+
If no classpath or build resource changes are detected we will execute all tests by default.
414+
This can be customized by setting the xref:appendix.adoc#configuration-properties[`spring.modulith.test.on-no-changes` property] to `skip-all`.
412415

413416
To optimize the execution in a CI environment, you need to populate the xref:appendix.adoc#configuration-properties[`spring.modulith.test.reference-commit` property] pointing to the commit of the last successful build and make sure that the build checks out all commits up to the reference one.
414417
The algorithm detecting changes to application modules will then consider all files changed in that delta.

0 commit comments

Comments
 (0)