Unit tests & reorganize classes (#93)

* Pull in junit & assertj for some unit tests

* Add validation to GitVersionArgs

* Move all logic to new VersionDetails implementation

* Cache the expensive git describe part

* Delete old classes

* Reduce methods in GitCli

* Move utilities closer to usage

* Move isRepoEmpty utility

* Move util closer to usage

* Reduce args

* Delete GitCli utilities

* Re-order slightly

* Unit test for VersionDetails

* Don't bother re-throwing IOExceptions

* Use java 7 compatible assertJ

* Convert to unit test

* Move one more unit test

Author: iamdanfox

build.gradle

@@ -21,6 +21,8 @@ dependencies {
     compile 'com.google.guava:guava:20.0'
 
     testCompile gradleTestKit()
+    testCompile 'junit:junit:4.12'
+    testCompile 'org.assertj:assertj-core:2.9.0'
     testCompile('org.spockframework:spock-core:1.0-groovy-2.4') {
         exclude module: 'groovy-all'
     }

src/main/java/com/palantir/gradle/gitversion/GitCli.java

@@ -4,7 +4,7 @@ interface GitDescribe {
 
     /**
      * Mimics behaviour of 'git describe --tags --always --first-parent --match=${prefix}*'
-     * Method returns null if repository is empty.
+     * Method can assume repo is not empty but should never throw.
      */
     String describe(String prefix);
 }

src/main/java/com/palantir/gradle/gitversion/GitDescribe.java

@@ -1,9 +1,5 @@
 package com.palantir.gradle.gitversion;
 
-import org.eclipse.jgit.api.Git;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.Ref;
-
 class GitUtils {
 
     static final int SHA_ABBR_LENGTH = 7;
@@ -13,24 +9,4 @@ private GitUtils() {}
     static String abbrevHash(String s) {
         return s.substring(0, SHA_ABBR_LENGTH);
     }
-
-    static boolean isRepoEmpty(Git git) {
-        // back-compat: the JGit "describe" command throws an exception in repositories with no commits, so call it
-        // first to preserve this behavior in cases where this call would fail but native "git" call does not.
-        try {
-            git.describe().call();
-            return true;
-        } catch (Exception ignored) {
-            return false;
-        }
-    }
-
-    // getPeeledObjectId returns:
-    // "if this ref is an annotated tag the id of the commit (or tree or blob) that the annotated tag refers to;
-    // null if this ref does not refer to an annotated tag."
-    // We use this to check if tag is annotated.
-    static boolean isAnnotatedTag(Ref ref) {
-        ObjectId peeledObjectId = ref.getPeeledObjectId();
-        return peeledObjectId != null;
-    }
 }

src/main/java/com/palantir/gradle/gitversion/GitUtils.java

@@ -1,15 +1,24 @@
 package com.palantir.gradle.gitversion;
 
+import com.google.common.base.Preconditions;
+
 import java.util.Map;
 
 class GitVersionArgs {
+    private static final String PREFIX_REGEX = "[/@]?([A-Za-z]+[/@-])+";
+
     private String prefix = "";
 
     public String getPrefix() {
         return prefix;
     }
 
     public void setPrefix(String prefix) {
+        Preconditions.checkNotNull(prefix, "prefix must not be null");
+
+        Preconditions.checkState(prefix.matches(PREFIX_REGEX),
+                "Specified prefix `" + prefix + "` does not match the allowed format regex `" + PREFIX_REGEX + "`.");
+
         this.prefix = prefix;
     }
 

src/main/java/com/palantir/gradle/gitversion/GitVersionArgs.java

@@ -2,11 +2,7 @@
 
 import groovy.lang.Closure;
 import org.eclipse.jgit.api.Git;
-import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.internal.storage.file.FileRepository;
-import org.eclipse.jgit.lib.Constants;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.Ref;
 import org.gradle.api.Action;
 import org.gradle.api.Plugin;
 import org.gradle.api.Project;
@@ -18,16 +14,18 @@
 public class GitVersionPlugin implements Plugin<Project> {
     public void apply(final Project project) {
 
+        final Git git = gitRepo(project);
+
         // intentionally not using .getExtension() here for back-compat
         project.getExtensions().getExtraProperties().set("gitVersion", new Closure<String>(this, this) {
             public String doCall(Object args) {
-                return versionDetails(project, GitVersionArgs.fromGroovyClosure(args)).getVersion();
+                return new VersionDetails(git, GitVersionArgs.fromGroovyClosure(args)).getVersion();
             }
         });
 
         project.getExtensions().getExtraProperties().set("versionDetails", new Closure<VersionDetails>(this, this) {
             public VersionDetails doCall(Object args) {
-                return versionDetails(project, GitVersionArgs.fromGroovyClosure(args));
+                return new VersionDetails(git, GitVersionArgs.fromGroovyClosure(args));
             }
         });
 
@@ -42,105 +40,37 @@ public void execute(Task task) {
         printVersionTask.setDescription("Prints the project's configured version to standard out");
     }
 
-    public static void verifyPrefix(final String prefix) {
-        assert prefix != null && (prefix.equals("") || prefix.matches(PREFIX_REGEX)) : "Specified prefix `" + prefix + "` does not match the allowed format regex `" + PREFIX_REGEX + "`.";
-    }
-
-    private static String stripPrefix(String description, final String prefix) {
-        return description == null ? description : description.replaceFirst("^" + prefix, "");
-    }
-
-    private VersionDetails versionDetails(Project project, GitVersionArgs args) {
-        verifyPrefix(args.getPrefix());
-        Git git = gitRepo(project);
-        String description = stripPrefix(gitDescribe(project, args.getPrefix()), args.getPrefix());
-        String hash = gitHash(git);
-        String fullHash = gitHashFull(git);
-        String branchName = gitBranchName(git);
-        boolean isClean = isClean(git);
-
-        return new VersionDetails(description, hash, fullHash, branchName, isClean);
-    }
-
     private Git gitRepo(Project project) {
         try {
-            File gitDir = GitCli.getRootGitDir(project.getProjectDir());
+            File gitDir = getRootGitDir(project.getProjectDir());
             return Git.wrap(new FileRepository(gitDir));
         } catch (IOException e) {
             throw new RuntimeException(e);
         }
     }
 
-    private String gitDescribe(Project project, String prefix) {
-        // This used to be implemented with JGit and replaced with shelling out to installed git (#46) because JGit
-        // didn't support required behavior. Using installed git doesn't work in some environments or
-        // with older versions of git client. We're switching back to implementation with JGit. To make sure we don't
-        // make breaking change, we're keeping both implementations. Plan is to get rid of installed git implementation.
-        // TODO(mbakovic): Use JGit only implementation #87
-
-        Git git = gitRepo(project);
-        String nativeGitDescribe = new NativeGitDescribe(project.getProjectDir(), git).describe(prefix);
-        String jgitDescribe = new JGitDescribe(git).describe(prefix);
-
-        // If native failed, return JGit one
-        if (nativeGitDescribe == null) {
-            return jgitDescribe;
-        }
-
-
-        // If native succeeded, make sure it's same as JGit one
-        if (!nativeGitDescribe.equals(jgitDescribe)) {
-            throw new IllegalStateException(String.format("Inconsistent git describe: native was %s and jgit was %s. " + "Please report this on github.com/palantir/gradle-git-version", nativeGitDescribe, jgitDescribe));
-        }
-
-
-        return jgitDescribe;
-    }
-
-    private String gitHash(Git git) {
-        String gitHashFull = gitHashFull(git);
-        if (gitHashFull == null) {
-            return null;
+    private static File getRootGitDir(File currentRoot) {
+        File gitDir = scanForRootGitDir(currentRoot);
+        if (!gitDir.exists()) {
+            throw new IllegalArgumentException("Cannot find '.git' directory");
         }
-
-        return gitHashFull.substring(0, VERSION_ABBR_LENGTH);
+        return gitDir;
     }
 
-    private String gitHashFull(Git git) {
-        try {
-            ObjectId objectId = git.getRepository().findRef(Constants.HEAD).getObjectId();
-            if (objectId == null) {
-                return null;
-            }
+    private static File scanForRootGitDir(File currentRoot) {
+        File gitDir = new File(currentRoot, ".git");
 
-            return objectId.name();
-        } catch (IOException e) {
-            throw new RuntimeException(e);
+        if (gitDir.exists()) {
+            return gitDir;
         }
-    }
 
-    private String gitBranchName(Git git) {
-        try {
-            Ref ref = git.getRepository().findRef(git.getRepository().getBranch());
-            if (ref == null) {
-                return null;
-            }
-
-            return ref.getName().substring(Constants.R_HEADS.length());
-        } catch (IOException e) {
-            throw new RuntimeException(e);
+        // stop at the root directory, return non-existing File object;
+        if (currentRoot.getParentFile() == null) {
+            return gitDir;
         }
-    }
 
-    private boolean isClean(Git git) {
-        try {
-            return git.status().call().isClean();
-        } catch (GitAPIException e) {
-            throw new RuntimeException(e);
-        }
+        // look in parent directory;
+        return scanForRootGitDir(currentRoot.getParentFile());
     }
-
-    private static final int VERSION_ABBR_LENGTH = 10;
-    private static final String PREFIX_REGEX = "[/@]?([A-Za-z]+[/@-])+";
 }
 

src/main/java/com/palantir/gradle/gitversion/GitVersionPlugin.java

@@ -30,11 +30,6 @@ class JGitDescribe implements GitDescribe {
 
     @Override
     public String describe(String prefix) {
-        if (!GitUtils.isRepoEmpty(git)) {
-            log.debug("Repository is empty");
-            return null;
-        }
-
         try {
             ObjectId headObjectId = git.getRepository().resolve(Constants.HEAD);