Merge pull request #24 from shipkit/sf

Documentation, edge cases and improved messages

build.gradle

@@ -10,14 +10,9 @@ buildscript {
     }
 }
 
-apply plugin: 'java-gradle-plugin'
 apply plugin: 'maven-publish'
 apply plugin: 'idea'
 apply plugin: 'groovy'
-apply plugin: "com.gradle.plugin-publish"
-apply plugin: "org.shipkit.shipkit-auto-version"
-apply plugin: "org.shipkit.shipkit-changelog"
-apply plugin: "org.shipkit.shipkit-gh-release"
 
 group = "org.shipkit"
 

gradle/release.gradle

@@ -1,3 +1,9 @@
+apply plugin: "java-gradle-plugin"
+apply plugin: "com.gradle.plugin-publish"
+apply plugin: "org.shipkit.shipkit-auto-version"
+apply plugin: "org.shipkit.shipkit-changelog"
+apply plugin: "org.shipkit.shipkit-gh-release"
+
 // docs: https://plugins.gradle.org/docs/publish-plugin
 gradlePlugin {
     plugins {

src/main/java/org/shipkit/auto/version/AutoVersion.java

@@ -29,6 +29,9 @@ class AutoVersion {
         this(new ProcessRunner(projectDir), new File(projectDir, "version.properties"));
     }
 
+    /**
+     * Deduct version based on existing tags (will run 'git tag'), and the version spec from versionFile field.
+     */
     DeductedVersion deductVersion() {
         return deductVersion(LOG);
     }
@@ -38,6 +41,7 @@ private static void explainVersion(Logger log, String version, String reason) {
                 "  - reason: shipkit-auto-version " + reason);
     }
 
+    //Exposed for testing so that 'log' can be mocked
     DeductedVersion deductVersion(Logger log) {
         String spec = VersionSpec.readVersionSpec(versionFile);
         if (!spec.endsWith("*")) {
@@ -57,15 +61,15 @@ DeductedVersion deductVersion(Logger log) {
         }
     }
 
+    //Exposed for testing so that 'log' can be mocked and we can pass arbitrary spec
     DeductedVersion deductVersion(String spec, Logger log) {
         String gitOutput = runner.run("git", "tag");
         String[] tags = gitOutput.split("\\R");
         Optional<Version> nearest = new NearestTagFinder().findTag(asList(tags), spec);
         if (!nearest.isPresent()) {
             //if there is no nearest matching tag (same major, same minor) we can just use '0' for the wildcard
             String version = spec.replace("*", "0");
-            //TODO add information 'found no tags matching ...'
-            explainVersion(log, version, "found no tags");
+            explainVersion(log, version, "found no tags matching version spec: '" + spec + "'");
             return new DeductedVersion(version, null);
         }
 

src/main/java/org/shipkit/auto/version/CommitCounter.java

@@ -4,6 +4,9 @@
 
 import static java.lang.Integer.max;
 
+/**
+ * Counts commits based on the output from 'git log'
+ */
 class CommitCounter {
 
     /**

src/main/java/org/shipkit/auto/version/DeductedVersion.java

@@ -1,6 +1,7 @@
 package org.shipkit.auto.version;
 
 import javax.annotation.Nullable;
+import java.util.Objects;
 
 /**
  * Value object representing the version deducted by the auto plugin.
@@ -10,21 +11,27 @@ class DeductedVersion {
     private final String version;
     private final String previousVersion;
 
-    public DeductedVersion(String version, String previousVersion) {
+    DeductedVersion(String version, String previousVersion) {
+        Objects.requireNonNull(version, "version cannot be null");
         this.version = version;
         this.previousVersion = previousVersion;
     }
 
-    public String getVersion() {
+    /**
+     * Deducted version, never null.
+     */
+    String getVersion() {
         return version;
     }
 
     /**
+     * Previous version.
      * The returned value can be null when there is no previous tag
      * or when it was not possible to identify the previous tag.
+     * Not using Optional because it makes the client logic simpler.
      */
     @Nullable
-    public String getPreviousVersion() {
+    String getPreviousVersion() {
         return previousVersion;
     }
 }

src/main/java/org/shipkit/auto/version/NearestTagFinder.java

@@ -6,20 +6,40 @@
 import java.util.TreeSet;
 import java.util.regex.Pattern;
 
+import static org.shipkit.auto.version.VersionSpec.isWildcardSpec;
+
+/**
+ * Identifies the nearest tag based on the current tags and the version spec.
+ */
 class NearestTagFinder {
 
+    /**
+     * Finds the latest tag that matches the spec.
+     * For example, given tags "v1.0.0" and "v1.0.1" and spec "1.0.*", the result is "1.0.1".
+     * This method throws exception if the spec does not match the standard "major.minor.*",
+     * examples of invalid specs: "1.*.0", "1.0.0-beta.*".
+     *
+     * @param tags existing tag name, usually retrieved by running 'git tag'
+     *             Only tag names starting with 'v' (standard GitHub convention) are supported at the moment.
+     * @param spec the spec mask we are trying to find the matching tag, example: '1.0.*'.
+     *             Expected to have single '*' in place of the patch version.
+     * @return version corresponding to a tag or an empty optional if matching tag cannot be found.
+     */
     Optional<Version> findTag(Iterable<String> tags, String spec) {
-        //TODO validate spec, must have 1 wildcard at the end of the spec
-        Pattern pattern = Pattern.compile(spec.replaceAll("\\*", "\\\\d+"));
-        TreeSet<Version> candidates = new TreeSet<>();
+        if (!isWildcardSpec(spec)) {
+            throw new IllegalArgumentException("Invalid spec: '" + spec + "'. Correct examples: '1.0.*', '2.30.*'");
+        }
+        spec = spec.replace("*", "\\d+");
+        spec = spec.replaceAll("\\.", "\\\\.");
+        Pattern pattern = Pattern.compile(spec);
+        TreeSet<Version> candidates = new TreeSet<>(); //takes care of sorting
         for (String tag : tags) {
             if (tag.startsWith("v")) {
                 tag = tag.substring(1);
             } else {
-                continue; //only support tags with 'v' prefix
+                continue; //currently we only support tags with 'v' prefix
             }
             if (pattern.matcher(tag).matches()) {
-                //TODO: error handling when version is bad
                 candidates.add(Version.valueOf(tag));
             }
         }

src/main/java/org/shipkit/auto/version/ProcessRunner.java

@@ -6,6 +6,9 @@
 import java.io.InputStreamReader;
 import java.util.Arrays;
 
+/**
+ * Runs command in provided working dir.
+ */
 class ProcessRunner {
 
     private final File workDir;

src/main/java/org/shipkit/auto/version/VersionSpec.java

@@ -14,6 +14,14 @@ class VersionSpec {
 
     private static String ERROR = "Problems deducting the version automatically.";
 
+    /**
+     * Reads the version spec from provided file.
+     * Throws exception message with actionable message when version file does not exist
+     * or does not contain correctly formatted version spec.
+     *
+     * @param versionFile file that has the version spec
+     * @return validated version spec
+     */
     static String readVersionSpec(File versionFile) {
         Properties p = new Properties();
         try {
@@ -29,7 +37,7 @@ static String readVersionSpec(File versionFile) {
         }
         String versionSpec = (String) v;
 
-        if (versionSpec.matches("\\d+.\\d+.(\\*)")) {
+        if (isWildcardSpec(versionSpec)) {
             return versionSpec;
         }
 
@@ -42,6 +50,15 @@ static String readVersionSpec(File versionFile) {
         return versionSpec;
     }
 
+    /**
+     * Returns true when the version spec is valid and uses '*' wildcard.
+     *
+     * @param versionSpec version spec
+     */
+    static boolean isWildcardSpec(String versionSpec) {
+        return versionSpec.matches("\\d+\\.\\d+\\.\\*");
+    }
+
     private static String exceptionMessage(File versionFile) {
         return "Problems deducting the version automatically. Expected correct 'version' property in file: " + versionFile + "\n" +
                 "Correct examples: 'version=1.0.*', 'version=2.10.100'";

src/test/groovy/org/shipkit/auto/version/AutoVersionTest.groovy

@@ -41,7 +41,7 @@ v1.0.1
         v.version == "1.1.0"
         v.previousVersion == null
         1 * log.lifecycle("Building version '1.1.0'\n" +
-                "  - reason: shipkit-auto-version found no tags")
+                "  - reason: shipkit-auto-version found no tags matching version spec: '1.1.*'")
     }
 
     def "increments version"() {

src/test/groovy/org/shipkit/auto/version/NearestTagFinderTest.groovy

@@ -9,6 +9,7 @@ class NearestTagFinderTest extends Specification {
             "foo",
             "v1.0.0",
             "v1.0.1",
+            "v1,0,2",
             "x1.0.2",
             "v1.0.2x",
             "v1.0.1-rc.1",
@@ -23,8 +24,22 @@ class NearestTagFinderTest extends Specification {
 
         expect:
         new NearestTagFinder().findTag(tags, "1.0.*").get().toString() == "1.0.1"
-        new NearestTagFinder().findTag(tags, "2.0.0-beta.*").get().toString() == "2.0.0-beta.2"
         new NearestTagFinder().findTag(tags, "2.0.*").get().toString() == "2.0.0"
-        new NearestTagFinder().findTag(tags, "2.3.*").isPresent() == false
+
+        !new NearestTagFinder().findTag(tags, "2.3.*")
+        !new NearestTagFinder().findTag([], "2.3.*")
+    }
+
+    def "fails with clean error message"() {
+        when:
+        new NearestTagFinder().findTag(["v1.0.0"], badSpec)
+
+        then:
+        def ex = thrown(RuntimeException)
+        ex.message == "Invalid spec: '$badSpec'. Correct examples: '1.0.*', '2.30.*'"
+
+        where:
+        //in the future we may offer support for some of those
+        badSpec << ["", "1.0", "1.0.0", "1.*.0", "1.0.0-*.RC", "1.0.0-beta.*"]
     }
 }

src/test/groovy/org/shipkit/auto/version/VersionSpecTest.groovy

@@ -45,6 +45,6 @@ class VersionSpecTest extends TmpFolderSpecification {
         e.cause != null
 
         where:
-        spec << ["foo.version", "1.2", "1.2.**", "1.*.*", "1.0.0-beta.*"]
+        spec << ["foo.version", "1.2", "1.2.**", "1.*.*", "1.0.0-beta.*", "1.12*"]
     }
 }