From 3865c75fc97a2df90c03f0b122416a42ab1f1681 Mon Sep 17 00:00:00 2001 From: Andrew Oberstar Date: Sun, 15 Apr 2018 13:09:22 -0500 Subject: [PATCH] Support reckon.stage for snapshot strategy The reckon.snapshot property is a little confusing to set the value of, since true means snapshot and final is false. It seems a little backwards. The reckon.stage approach is a little more natural, explicitly passing in 'snapshot' or 'final'. This will deprecate use of reckon.snapshot, to be removed in 1.0.0. This fixes #71. --- .../strategy/SnapshotPreReleaseStrategy.java | 18 ++++++---- .../reckon/core/ReckonerTest.groovy | 8 ++--- .../SnapshotPreReleaseStrategyTest.groovy | 34 ++++++++++++++----- .../gradle/CompositeBuildCompatTest.groovy | 4 +-- .../reckon/gradle/ReckonExtension.java | 32 ++++++++++------- 5 files changed, 64 insertions(+), 32 deletions(-) diff --git a/reckon-core/src/main/java/org/ajoberstar/reckon/core/strategy/SnapshotPreReleaseStrategy.java b/reckon-core/src/main/java/org/ajoberstar/reckon/core/strategy/SnapshotPreReleaseStrategy.java index 2d671e35..f882f58d 100644 --- a/reckon-core/src/main/java/org/ajoberstar/reckon/core/strategy/SnapshotPreReleaseStrategy.java +++ b/reckon-core/src/main/java/org/ajoberstar/reckon/core/strategy/SnapshotPreReleaseStrategy.java @@ -7,22 +7,28 @@ import org.ajoberstar.reckon.core.VcsInventory; public final class SnapshotPreReleaseStrategy implements PreReleaseStrategy { - private final BiFunction> snapshotCalc; + public static final String FINAL_STAGE = "final"; + public static final String SNAPSHOT_STAGE = "snapshot"; - public SnapshotPreReleaseStrategy(BiFunction> snapshotCalc) { - this.snapshotCalc = snapshotCalc; + private final BiFunction> stageCalc; + + public SnapshotPreReleaseStrategy(BiFunction> stageCalc) { + this.stageCalc = stageCalc; } @Override public Version reckonTargetVersion(VcsInventory inventory, Version targetNormal) { - Optional maybeSnapshot = snapshotCalc.apply(inventory, targetNormal); + Optional maybeStage = stageCalc.apply(inventory, targetNormal); - if (inventory.isClean() && inventory.getCurrentVersion().isPresent() && !maybeSnapshot.isPresent()) { + if (inventory.isClean() && inventory.getCurrentVersion().isPresent() && !maybeStage.isPresent()) { // rebuild return inventory.getCurrentVersion().get(); } else { - if (maybeSnapshot.orElse(true)) { + String stage = maybeStage.orElse(SNAPSHOT_STAGE); + if (stage.equals(SNAPSHOT_STAGE)) { return targetNormal.setPreReleaseVersion("SNAPSHOT"); + } else if (!stage.equals(FINAL_STAGE)) { + throw new IllegalArgumentException(String.format("Stage \"%s\" is not one of: [snapshot, final]", stage)); } else if (!inventory.isClean()) { throw new IllegalStateException("Cannot release a final version without a clean repo."); } else { diff --git a/reckon-core/src/test/groovy/org/ajoberstar/reckon/core/ReckonerTest.groovy b/reckon-core/src/test/groovy/org/ajoberstar/reckon/core/ReckonerTest.groovy index 77580246..be10960e 100644 --- a/reckon-core/src/test/groovy/org/ajoberstar/reckon/core/ReckonerTest.groovy +++ b/reckon-core/src/test/groovy/org/ajoberstar/reckon/core/ReckonerTest.groovy @@ -168,7 +168,7 @@ class ReckonerTest extends Specification { ) expect: reckonStage(inventory2, 'minor', 'final') == '1.3.0' - reckonSnapshot(inventory2, 'major', false) == '2.0.0' + reckonSnapshot(inventory2, 'major', 'final') == '2.0.0' } def 'if current version is present and normal, repo is clean, not allowed to release an incremented pre-release stage'() { @@ -202,7 +202,7 @@ class ReckonerTest extends Specification { [Version.valueOf('1.2.2'), Version.valueOf('1.2.3')] as Set ) when: - reckonSnapshot(inventory2, null, true) + reckonSnapshot(inventory2, null, 'snapshot') then: thrown(IllegalStateException) } @@ -229,9 +229,9 @@ class ReckonerTest extends Specification { return Reckoner.reckon({ -> inventory }, normal, preRelease) } - private String reckonSnapshot(inventory, scope, snapshot) { + private String reckonSnapshot(inventory, scope, stage) { ScopeNormalStrategy normal = new ScopeNormalStrategy({ i -> Optional.ofNullable(scope) }) - SnapshotPreReleaseStrategy preRelease = new SnapshotPreReleaseStrategy({ i, v -> Optional.ofNullable(snapshot) }) + SnapshotPreReleaseStrategy preRelease = new SnapshotPreReleaseStrategy({ i, v -> Optional.ofNullable(stage) }) return Reckoner.reckon({ -> inventory }, normal, preRelease) } } diff --git a/reckon-core/src/test/groovy/org/ajoberstar/reckon/core/strategy/SnapshotPreReleaseStrategyTest.groovy b/reckon-core/src/test/groovy/org/ajoberstar/reckon/core/strategy/SnapshotPreReleaseStrategyTest.groovy index ad78b646..42f04114 100644 --- a/reckon-core/src/test/groovy/org/ajoberstar/reckon/core/strategy/SnapshotPreReleaseStrategyTest.groovy +++ b/reckon-core/src/test/groovy/org/ajoberstar/reckon/core/strategy/SnapshotPreReleaseStrategyTest.groovy @@ -20,7 +20,25 @@ import org.ajoberstar.reckon.core.VcsInventory import spock.lang.Specification class SnapshotPreReleaseStrategyTest extends Specification { - def 'if snapshot is false, return the target normal'() { + def 'if stage supplier returns an invalid stage, throw'() { + given: + def inventory = new VcsInventory( + 'abcdef', + true, + null, + Version.valueOf('1.2.3-milestone.1'), + Version.valueOf('1.2.2'), + 1, + [] as Set, + [] as Set + ) + when: + strategy('not').reckonTargetVersion(inventory, Version.valueOf('2.0.0')) + then: + thrown(IllegalArgumentException) + } + + def 'if stage is final, return the target normal'() { given: def inventory = new VcsInventory( 'abcdef', @@ -33,10 +51,10 @@ class SnapshotPreReleaseStrategyTest extends Specification { [] as Set ) expect: - strategy(false).reckonTargetVersion(inventory, Version.valueOf('2.0.0')).toString() == '2.0.0' + strategy('final').reckonTargetVersion(inventory, Version.valueOf('2.0.0')).toString() == '2.0.0' } - def 'if snapshot is true, set pre-release to snapshot'() { + def 'if stage is snapshot or null, set pre-release to snapshot'() { given: def inventory = new VcsInventory( 'abcdef', @@ -49,11 +67,11 @@ class SnapshotPreReleaseStrategyTest extends Specification { [] as Set ) expect: - strategy(true).reckonTargetVersion(inventory, Version.valueOf('2.0.0')).toString() == '2.0.0-SNAPSHOT' + strategy('snapshot').reckonTargetVersion(inventory, Version.valueOf('2.0.0')).toString() == '2.0.0-SNAPSHOT' strategy(null).reckonTargetVersion(inventory, Version.valueOf('2.0.0')).toString() == '2.0.0-SNAPSHOT' } - def 'if repo has uncommitted changes, fail if snapshot is false'() { + def 'if repo has uncommitted changes, fail if stage is final'() { given: def inventory = new VcsInventory( 'abcdef', @@ -66,12 +84,12 @@ class SnapshotPreReleaseStrategyTest extends Specification { [] as Set ) when: - strategy(false).reckonTargetVersion(inventory, Version.valueOf('2.0.0')) + strategy('final').reckonTargetVersion(inventory, Version.valueOf('2.0.0')) then: thrown(IllegalStateException) } - private SnapshotPreReleaseStrategy strategy(snapshot) { - return new SnapshotPreReleaseStrategy({ i, v -> Optional.ofNullable(snapshot) }) + private SnapshotPreReleaseStrategy strategy(stage) { + return new SnapshotPreReleaseStrategy({ i, v -> Optional.ofNullable(stage) }) } } diff --git a/reckon-gradle/src/compatTest/groovy/org/ajoberstar/reckon/gradle/CompositeBuildCompatTest.groovy b/reckon-gradle/src/compatTest/groovy/org/ajoberstar/reckon/gradle/CompositeBuildCompatTest.groovy index e0adebef..e3aa4da0 100644 --- a/reckon-gradle/src/compatTest/groovy/org/ajoberstar/reckon/gradle/CompositeBuildCompatTest.groovy +++ b/reckon-gradle/src/compatTest/groovy/org/ajoberstar/reckon/gradle/CompositeBuildCompatTest.groovy @@ -26,7 +26,7 @@ class CompositeBuildCompatTest extends Specification { build2File = projectFile(project2Dir, 'build.gradle') def grgit1 = Grgit.init(dir: project1Dir) - projectFile(project1Dir, 'file.txt') << 'stuff' + projectFile(project1Dir, 'settings.gradle') << 'rootProject.name = "project1"' projectFile(project1Dir, '.gitignore') << '.gradle\nbuild\n' build1File << '''\ plugins { @@ -51,7 +51,7 @@ task printVersion { grgit1.close() def grgit2 = Grgit.init(dir: project2Dir) - projectFile(project2Dir, 'file.txt') << 'stuff' + projectFile(project2Dir, 'settings.gradle') << 'rootProject.name = "project2"' projectFile(project2Dir, '.gitignore') << '.gradle\nbuild\n' build2File << '''\ plugins { diff --git a/reckon-gradle/src/main/java/org/ajoberstar/reckon/gradle/ReckonExtension.java b/reckon-gradle/src/main/java/org/ajoberstar/reckon/gradle/ReckonExtension.java index 5c3526c9..30a0276e 100644 --- a/reckon-gradle/src/main/java/org/ajoberstar/reckon/gradle/ReckonExtension.java +++ b/reckon-gradle/src/main/java/org/ajoberstar/reckon/gradle/ReckonExtension.java @@ -52,29 +52,37 @@ public VcsInventorySupplier git(Grgit grgit) { } public NormalStrategy scopeFromProp() { - Function> supplier = ignored -> Optional.ofNullable(project.findProperty(SCOPE_PROP)) - // composite builds have a parent Gradle build and can't trust the values of these properties - .filter(value -> project.getGradle().getParent() == null) - .map(Object::toString); + Function> supplier = inventory -> findProperty(SCOPE_PROP); return new ScopeNormalStrategy(supplier); } public PreReleaseStrategy stageFromProp(String... stages) { Set stageSet = Arrays.stream(stages).collect(Collectors.toSet()); - BiFunction> supplier = (inventory, targetNormal) -> Optional.ofNullable(project.findProperty(STAGE_PROP)) - // composite builds have a parent Gradle build and can't trust the values of these properties - .filter(value -> project.getGradle().getParent() == null) - .map(Object::toString); + BiFunction> supplier = (inventory, targetNormal) -> findProperty(STAGE_PROP); return new StagePreReleaseStrategy(stageSet, supplier); } public PreReleaseStrategy snapshotFromProp() { - BiFunction> supplier = (inventory, targetNormal) -> Optional.ofNullable(project.findProperty(SNAPSHOT_PROP)) + BiFunction> supplier = (inventory, targetNormal) -> { + Optional stageProp = findProperty(STAGE_PROP); + Optional snapshotProp = findProperty(SNAPSHOT_PROP) + .map(Boolean::parseBoolean) + .map(isSnapshot -> isSnapshot ? "snapshot" : "final"); + + snapshotProp.ifPresent(val -> { + project.getLogger().warn("Property {} is deprecated and will be removed in 1.0.0. Use {} set to one of [snapshot, final].", SNAPSHOT_PROP, STAGE_PROP); + }); + + return stageProp.isPresent() ? stageProp : snapshotProp; + }; + return new SnapshotPreReleaseStrategy(supplier); + } + + private Optional findProperty(String name) { + return Optional.ofNullable(project.findProperty(name)) // composite builds have a parent Gradle build and can't trust the values of these properties .filter(value -> project.getGradle().getParent() == null) - .map(Object::toString) - .map(Boolean::parseBoolean); - return new SnapshotPreReleaseStrategy(supplier); + .map(Object::toString); } Grgit getGrgit() {