Merge pull request #704 from git-commit-id/701

#701: add testcases to for an issue with submodules

Author: TheSnoozer

src/main/java/pl/project13/maven/git/GitDirLocator.java

@@ -135,22 +135,28 @@ private File processGitDirFile(@Nonnull File file) {
       }
 
       // All seems ok so return the "gitdir" value read from the file.
-      File gitDir = resolveWorktree(new File(parts[1]));
+      String extractFromConfig = parts[1];
+      File gitDir = resolveWorktree(new File(extractFromConfig));
       if (gitDir.isAbsolute()) {
         // gitdir value is an absolute path. Return as-is
         return gitDir;
       } else {
         // gitdir value is relative.
-        return new File(file.getParentFile(), parts[1]);
+        return new File(file.getParentFile(), extractFromConfig);
       }
     } catch (IOException e) {
       return null;
     }
   }
 
   /**
-   * If the file looks like the location of a worktree, return the .git folder of the git repository
-   * of the worktree. If not, return the file as is.
+   * Attempts to resolve the actual location of the .git folder for a given
+   * worktree.
+   * For example for a worktree like {@code a/.git/worktrees/X} structure would
+   * return {@code a/.git}.
+   *
+   * If the conditions for a git worktree like file structure are met simply return the provided
+   * argument as is.
    */
   static File resolveWorktree(File fileLocation) {
     Path parent = fileLocation.toPath().getParent();

src/test/java/pl/project13/maven/git/AvailableGitTestRepo.java

@@ -54,7 +54,42 @@ public enum AvailableGitTestRepo {
    */
   WITH_COMMIT_THAT_HAS_TWO_TAGS("src/test/resources/_git_with_commit_that_has_two_tags"),
   ON_A_TAG_DIRTY("src/test/resources/_git_on_a_tag_dirty"),
+
+  /**
+   * <pre>
+   * * 01ed93c - (11 years ago) any commit, just a readme - Konrad Malawski (HEAD -> master)
+   * * 4ce26eb - (11 years ago) my submodules, yay - Konrad Malawski
+   * </pre>
+   * <pre>
+   * $ git submodule status
+   * -9fd4b69a5ca09b60884d4f8f49ce16ea071077be module1
+   * -9fd4b69a5ca09b60884d4f8f49ce16ea071077be module2
+   * -9fd4b69a5ca09b60884d4f8f49ce16ea071077be module3
+   * -9fd4b69a5ca09b60884d4f8f49ce16ea071077be module4
+   *
+   * $ git config --file .gitmodules --get-regexp '\.url$'
+   * submodule.module1.url /tmp/module1
+   * submodule.module2.url /tmp/module1
+   * submodule.module3.url /tmp/module1
+   * submodule.module4.url /tmp/module1
+   * </pre>
+   */
   WITH_SUBMODULES("src/test/resources/_git_with_submodules"),
+
+  /**
+   * <pre>
+   * 6455ccd - (3 minutes ago) init (HEAD -> master)
+   * </pre>
+   * <pre>
+   * $ git submodule status
+   * 945bfe60e8a3eff168e915c7ba5bac37c9d0165b remote-module (heads/empty-branch)
+   *
+   * $ git submodule foreach --recursive git remote get-url origin
+   * Entering 'remote-module'
+   * git@github.com:git-commit-id/git-test-resources.git
+   * </pre>
+   */
+  WITH_REMOTE_SUBMODULES("src/test/resources/_git_with_remote_submodules"),
   /**
    *
    *

src/test/java/pl/project13/maven/git/BigDiffTest.java

@@ -86,23 +86,6 @@ public void bigDiff() throws Exception {
     assertGitPropertiesPresentInProject(targetProject.getProperties());
   }
 
-  private void assertGitPropertiesPresentInProject(final Properties properties) {
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.build.time"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.build.host"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.branch"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.id.full"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.id.abbrev"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.id.describe"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.build.user.name"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.build.user.email"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.user.name"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.user.email"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.message.full"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.message.short"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.time"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.remote.origin.url"));
-  }
-
   private GitDescribeConfig createGitDescribeConfig(
       final boolean forceLongFormat, final int abbrev) {
     final GitDescribeConfig gitDescribeConfig = new GitDescribeConfig();

src/test/java/pl/project13/maven/git/GitCommitIdMojoIntegrationTest.java

@@ -18,15 +18,13 @@
 
 package pl.project13.maven.git;
 
-import static java.util.Arrays.asList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.entry;
 import static org.mockito.Mockito.when;
 
 import java.io.File;
 import java.text.SimpleDateFormat;
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
@@ -49,17 +47,6 @@
 
 @RunWith(JUnitParamsRunner.class)
 public class GitCommitIdMojoIntegrationTest extends GitIntegrationTest {
-  private static final boolean UseJGit = false;
-  private static final boolean UseNativeGit = true;
-
-  public static Collection<?> useNativeGit() {
-    return asList(UseJGit, UseNativeGit);
-  }
-
-  public static Collection<?> useDirty() {
-    return asList(true, false);
-  }
-
   @Test
   @Parameters(method = "useNativeGit")
   public void shouldIncludeExpectedProperties(boolean useNativeGit) throws Exception {
@@ -78,19 +65,7 @@ public void shouldIncludeExpectedProperties(boolean useNativeGit) throws Excepti
 
     // then
     Properties properties = targetProject.getProperties();
-
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.branch"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.dirty"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.id.full"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.id.abbrev"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.build.user.name"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.build.user.email"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.user.name"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.user.email"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.message.full"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.message.short"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.time"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.remote.origin.url"));
+    assertGitPropertiesPresentInProject(properties);
   }
 
   @Test
@@ -1731,28 +1706,4 @@ private GitDescribeConfig createGitDescribeConfig(boolean forceLongFormat, int a
     gitDescribeConfig.setDirty("");
     return gitDescribeConfig;
   }
-
-  private void assertPropertyPresentAndEqual(Properties properties, String key, String expected) {
-    assertThat(properties.stringPropertyNames()).contains(key);
-    assertThat(properties.getProperty(key)).isEqualTo(expected);
-  }
-
-  private void assertGitPropertiesPresentInProject(Properties properties) {
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.build.time"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.build.host"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.branch"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.id.full"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.id.abbrev"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.id.describe"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.build.user.name"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.build.user.email"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.user.name"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.user.email"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.message.full"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.message.short"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.time"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.remote.origin.url"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.closest.tag.name"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.closest.tag.commit.count"));
-  }
 }

src/test/java/pl/project13/maven/git/GitDirLocatorTest.java

@@ -25,7 +25,9 @@
 import java.util.Collections;
 import java.util.List;
 import org.apache.maven.project.MavenProject;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
@@ -37,18 +39,20 @@ public class GitDirLocatorTest {
 
   List<MavenProject> reactorProjects = Collections.emptyList();
 
+  @Rule
+  public TemporaryFolder folder = new TemporaryFolder();
+
   @Test
   public void shouldUseTheManuallySpecifiedDirectory() throws Exception {
     // given
-    File dotGitDir = Files.createTempDirectory("temp").toFile();
+    File dotGitDir = folder.newFolder("temp");
     try {
-
       // when
       GitDirLocator locator = new GitDirLocator(project, reactorProjects);
       File foundDirectory = locator.lookupGitDirectory(dotGitDir);
 
       // then
-      assert foundDirectory != null;
+      assertThat(foundDirectory).isNotNull();
       assertThat(foundDirectory.getAbsolutePath()).isEqualTo(dotGitDir.getAbsolutePath());
     } finally {
       if (!dotGitDir.delete()) {
@@ -57,12 +61,66 @@ public void shouldUseTheManuallySpecifiedDirectory() throws Exception {
     }
   }
 
+  @Test
+  public void shouldResolveRelativeSubmodule() throws Exception {
+    // given
+    folder.newFolder("main-project");
+    folder.newFolder("main-project", ".git", "modules", "sub-module");
+    folder.newFolder("main-project", "sub-module");
+
+    // and a .git dir in submodule that points to the main's project .git/modules/submodule
+    File dotGitDir = folder.getRoot().toPath()
+        .resolve("main-project")
+        .resolve("sub-module")
+        .resolve(".git")
+        .toFile();
+    Files.write(
+        dotGitDir.toPath(),
+        "gitdir: ../.git/modules/sub-module".getBytes()
+    );
+
+    try {
+      // when
+      GitDirLocator locator = new GitDirLocator(project, reactorProjects);
+      File foundDirectory = locator.lookupGitDirectory(dotGitDir);
+
+      // then
+      assertThat(foundDirectory).isNotNull();
+      assertThat(
+          foundDirectory.getCanonicalFile()
+      ).isEqualTo(
+          folder.getRoot().toPath()
+          .resolve("main-project")
+          .resolve(".git")
+          .resolve("modules")
+          .resolve("sub-module")
+          .toFile()
+      );
+    } finally {
+      if (!dotGitDir.delete()) {
+        dotGitDir.deleteOnExit();
+      }
+    }
+  }
+
   @Test
   public void testWorktreeResolution() {
-    String[] noopCases = {"", "a", "a/b", ".git/worktrees", ".git/worktrees/", "a.git/worktrees/b"};
+    // tests to ensure we do not try to modify things that should not be modified
+    String[] noopCases = {
+      "",
+      "a",
+      "a/b",
+      ".git/worktrees",
+      ".git/worktrees/",
+      "a.git/worktrees/b",
+      ".git/modules",
+      ".git/modules/",
+      "a.git/modules/b",
+    };
     for (String path : noopCases) {
       assertThat(GitDirLocator.resolveWorktree(new File(path))).isEqualTo(new File(path));
     }
+    // tests that worktree resolution works
     assertThat(GitDirLocator.resolveWorktree(new File("a/.git/worktrees/b")))
         .isEqualTo(new File("a/.git"));
     assertThat(GitDirLocator.resolveWorktree(new File("/a/.git/worktrees/b")))

src/test/java/pl/project13/maven/git/GitIntegrationTest.java

@@ -18,13 +18,16 @@
 
 package pl.project13.maven.git;
 
+import static java.util.Arrays.asList;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.when;
 
 import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 import java.util.Optional;
 import java.util.Properties;
@@ -47,6 +50,13 @@ public abstract class GitIntegrationTest {
   private static final String SANDBOX_DIR = "target" + File.separator + "sandbox" + File.separator;
   protected static final String evaluateOnCommit = "HEAD";
 
+  private static final boolean UseJGit = false;
+  private static final boolean UseNativeGit = true;
+
+  public static Collection<?> useNativeGit() {
+    return asList(UseJGit, UseNativeGit);
+  }
+
   /** Sandbox directory with unique name for current test. */
   private String currSandbox;
 
@@ -156,4 +166,47 @@ private static List<MavenProject> getReactorProjects(@Nonnull MavenProject proje
     }
     return reactorProjects;
   }
+
+  public static void assertPropertyPresentAndEqual(
+      Properties properties, String key, String expected) {
+    assertThat(properties.stringPropertyNames()).contains(key);
+    assertThat(properties.getProperty(key)).isEqualTo(expected);
+  }
+
+  /**
+   * Ensures that the provided properties contain the properties the plugin can generate.
+   * See also {@link pl.project13.core.GitCommitPropertyConstant}
+   *
+   * @param properties The properties that should be verified
+   */
+  public static void assertGitPropertiesPresentInProject(Properties properties) {
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.branch"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.local.branch.ahead"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.local.branch.behind"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.dirty"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.id.full"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.id.abbrev"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.id.describe"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.id.describe-short"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.build.user.name"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.build.user.email"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.build.time"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.build.version"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.build.host"));
+    // assertThat(properties).satisfies(new ContainsKeyCondition("git.build.number"));
+    // assertThat(properties).satisfies(new ContainsKeyCondition("git.build.number.unique"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.user.name"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.user.email"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.message.full"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.message.short"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.time"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.author.time"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.committer.time"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.remote.origin.url"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.tags"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.closest.tag.name"));
+    // assertThat(properties).satisfies(new ContainsKeyCondition("git.tag"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.closest.tag.commit.count"));
+    assertThat(properties).satisfies(new ContainsKeyCondition("git.total.commit.count"));
+  }
 }

src/test/java/pl/project13/maven/git/GitPropertiesFileTest.java

@@ -108,25 +108,4 @@ public void shouldConformPropertiesFileWhenSpecialCharactersInValueString(boolea
       }
     }
   }
-
-  private void assertGitPropertiesPresentInProject(Properties properties) {
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.build.time"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.build.host"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.branch"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.id.full"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.id.abbrev"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.id.describe"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.build.user.name"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.build.user.email"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.user.name"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.user.email"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.message.full"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.message.short"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.time"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.committer.time"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.commit.author.time"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.remote.origin.url"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.closest.tag.name"));
-    assertThat(properties).satisfies(new ContainsKeyCondition("git.closest.tag.commit.count"));
-  }
 }