From c71412003a22e18650e2dedd394ef79dcb01d1cf Mon Sep 17 00:00:00 2001 From: TheSnoozer Date: Mon, 16 Oct 2017 00:19:01 -0400 Subject: [PATCH 1/5] https://github.com/ktoso/maven-git-commit-id-plugin/issues/221: use gitDescribeConfig to determine the git.closest.tag.name and git.closest.tag.count --- .../pl/project13/jgit/DescribeCommand.java | 26 +----- .../java/pl/project13/jgit/JGitCommon.java | 91 ++++++++++--------- .../pl/project13/maven/git/JGitProvider.java | 4 +- .../maven/git/NativeGitProvider.java | 14 ++- .../git/GitCommitIdMojoIntegrationTest.java | 74 +++++++++++++++ 5 files changed, 141 insertions(+), 68 deletions(-) diff --git a/src/main/java/pl/project13/jgit/DescribeCommand.java b/src/main/java/pl/project13/jgit/DescribeCommand.java index 58fe59d3..c7ae76f4 100644 --- a/src/main/java/pl/project13/jgit/DescribeCommand.java +++ b/src/main/java/pl/project13/jgit/DescribeCommand.java @@ -255,7 +255,8 @@ public DescribeResult call() throws GitAPIException { ObjectReader objectReader = repo.newObjectReader(); // get tags - Map> tagObjectIdToName = findTagObjectIds(repo, tagsFlag); + String matchPattern = createMatchPattern(); + Map> tagObjectIdToName = jGitCommon.findTagObjectIds(repo, tagsFlag, matchPattern); // get current commit RevCommit headCommit = findHeadObjectId(repo); @@ -344,28 +345,7 @@ static boolean hasTags(ObjectId headCommit, @NotNull Map> } RevCommit findHeadObjectId(@NotNull Repository repo) throws RuntimeException { - try { - ObjectId headId = repo.resolve("HEAD"); - - RevWalk walk = new RevWalk(repo); - RevCommit headCommit = walk.lookupCommit(headId); - walk.dispose(); - - log.info("HEAD is [{}]", headCommit.getName()); - return headCommit; - } catch (IOException ex) { - throw new RuntimeException("Unable to obtain HEAD commit!", ex); - } - } - - // git commit id -> its tag (or tags) - private Map> findTagObjectIds(@NotNull Repository repo, boolean tagsFlag) { - String matchPattern = createMatchPattern(); - Map> commitIdsToTags = jGitCommon.getCommitIdsToTags(repo, tagsFlag, matchPattern); - Map> commitIdsToTagNames = jGitCommon.transformRevTagsMapToDateSortedTagNames(commitIdsToTags); - log.info("Created map: [{}]", commitIdsToTagNames); - - return commitIdsToTagNames; + return jGitCommon.findHeadObjectId(repo); } private String createMatchPattern() { diff --git a/src/main/java/pl/project13/jgit/JGitCommon.java b/src/main/java/pl/project13/jgit/JGitCommon.java index 22c92104..409006bc 100644 --- a/src/main/java/pl/project13/jgit/JGitCommon.java +++ b/src/main/java/pl/project13/jgit/JGitCommon.java @@ -25,6 +25,7 @@ import org.eclipse.jgit.api.Status; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.errors.IncorrectObjectTypeException; +import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; @@ -40,7 +41,9 @@ import com.google.common.base.Predicate; import com.google.common.collect.Collections2; import com.google.common.collect.Lists; +import pl.project13.maven.git.GitDescribeConfig; import pl.project13.maven.git.log.LoggerBridge; +import pl.project13.maven.git.util.Pair; public class JGitCommon { @@ -81,61 +84,65 @@ private Collection getTags(final Git git, final ObjectId headId, final R return tags; } - public String getClosestTagName(@NotNull Repository repo) { - Map> map = getClosestTagAsMap(repo); - for (Map.Entry> entry : map.entrySet()) { - return trimFullTagName(entry.getValue().get(0).tagName); - } - return ""; + public String getClosestTagName(@NotNull Repository repo, GitDescribeConfig gitDescribe) { + // TODO: Why does some tests fail when it gets headCommit from JGitprovider? + RevCommit headCommit = findHeadObjectId(repo); + Pair pair = getClosestRevCommit(repo, headCommit, gitDescribe); + return pair.second; } - public String getClosestTagCommitCount(@NotNull Repository repo, RevCommit headCommit) { - HashMap> map = transformRevTagsMapToDateSortedTagNames(getClosestTagAsMap(repo)); - ObjectId obj = (ObjectId) map.keySet().toArray()[0]; - - try (RevWalk walk = new RevWalk(repo)) { - RevCommit commit = walk.lookupCommit(obj); - walk.dispose(); - - int distance = distanceBetween(repo, headCommit, commit); - return String.valueOf(distance); - } + public String getClosestTagCommitCount(@NotNull Repository repo, GitDescribeConfig gitDescribe) { + // TODO: Why does some tests fail when it gets headCommit from JGitprovider? + RevCommit headCommit = findHeadObjectId(repo); + Pair pair = getClosestRevCommit(repo, headCommit, gitDescribe); + RevCommit revCommit = pair.first; + int distance = distanceBetween(repo, headCommit, revCommit); + return String.valueOf(distance); } - private Map> getClosestTagAsMap(@NotNull Repository repo) { - Map> mapWithClosestTagOnly = new HashMap<>(); + private Pair getClosestRevCommit(@NotNull Repository repo, RevCommit headCommit, GitDescribeConfig gitDescribe) { + boolean includeLightweightTags = false; String matchPattern = ".*"; - Map> commitIdsToTags = getCommitIdsToTags(repo, true, matchPattern); - LinkedHashMap> sortedCommitIdsToTags = sortByDatedRevTag(commitIdsToTags); - - for (Map.Entry> entry: sortedCommitIdsToTags.entrySet()) { - mapWithClosestTagOnly.put(entry.getKey(), entry.getValue()); - break; + if (gitDescribe != null) { + includeLightweightTags = gitDescribe.getTags(); + if (!"*".equals(gitDescribe.getMatch())) { + matchPattern = gitDescribe.getMatch(); + } } + Map> tagObjectIdToName = findTagObjectIds(repo, includeLightweightTags, matchPattern); + if (tagObjectIdToName.containsKey(headCommit)) { + String tagName = tagObjectIdToName.get(headCommit).iterator().next(); + return Pair.of(headCommit, tagName); + } + List commits = findCommitsUntilSomeTag(repo, headCommit, tagObjectIdToName); + RevCommit revCommit = commits.get(0); + String tagName = tagObjectIdToName.get(revCommit).iterator().next(); - return mapWithClosestTagOnly; + return Pair.of(revCommit, tagName); } - private LinkedHashMap> sortByDatedRevTag(Map> map) { - List>> list = new ArrayList<>(map.entrySet()); + protected Map> findTagObjectIds(@NotNull Repository repo, boolean includeLightweightTags, String matchPattern) { + Map> commitIdsToTags = getCommitIdsToTags(repo, includeLightweightTags, matchPattern); + Map> commitIdsToTagNames = transformRevTagsMapToDateSortedTagNames(commitIdsToTags); + log.info("Created map: [{}]", commitIdsToTagNames); - Collections.sort(list, new Comparator>>() { - public int compare(Map.Entry> m1, Map.Entry> m2) { - // we need to sort the DatedRevTags to a commit first, otherwise we may get problems when we have two tags for the same commit - Collections.sort(m1.getValue(), datedRevTagComparator()); - Collections.sort(m2.getValue(), datedRevTagComparator()); + return commitIdsToTagNames; + } - DatedRevTag datedRevTag1 = m1.getValue().get(0); - DatedRevTag datedRevTag2 = m2.getValue().get(0); - return datedRevTagComparator().compare(datedRevTag1,datedRevTag2); - } - }); + protected RevCommit findHeadObjectId(@NotNull Repository repo) throws RuntimeException { + try { + ObjectId headId = repo.resolve(Constants.HEAD); - LinkedHashMap> result = new LinkedHashMap<>(); - for (Map.Entry> entry : list) { - result.put(entry.getKey(), entry.getValue()); + try (RevWalk walk = new RevWalk(repo)) { + RevCommit headCommit = walk.lookupCommit(headId); + walk.dispose(); + + log.info("HEAD is [{}]", headCommit.getName()); + return headCommit; + } + } catch (IOException ex) { + throw new RuntimeException("Unable to obtain HEAD commit!", ex); } - return result; } protected Map> getCommitIdsToTags(@NotNull Repository repo, boolean includeLightweightTags, String matchPattern) { diff --git a/src/main/java/pl/project13/maven/git/JGitProvider.java b/src/main/java/pl/project13/maven/git/JGitProvider.java index 8d5289d7..c9e302d0 100644 --- a/src/main/java/pl/project13/maven/git/JGitProvider.java +++ b/src/main/java/pl/project13/maven/git/JGitProvider.java @@ -187,7 +187,7 @@ public String getTags() throws GitCommitIdExecutionException { public String getClosestTagName() throws GitCommitIdExecutionException { Repository repo = getGitRepository(); try { - return jGitCommon.getClosestTagName(repo); + return jGitCommon.getClosestTagName(repo, gitDescribe); } catch (Throwable t) { // could not find any tags to describe } @@ -198,7 +198,7 @@ public String getClosestTagName() throws GitCommitIdExecutionException { public String getClosestTagCommitCount() throws GitCommitIdExecutionException { Repository repo = getGitRepository(); try { - return jGitCommon.getClosestTagCommitCount(repo, headCommit); + return jGitCommon.getClosestTagCommitCount(repo, gitDescribe); } catch (Throwable t) { // could not find any tags to describe } diff --git a/src/main/java/pl/project13/maven/git/NativeGitProvider.java b/src/main/java/pl/project13/maven/git/NativeGitProvider.java index 4a10d2ae..ab959d50 100644 --- a/src/main/java/pl/project13/maven/git/NativeGitProvider.java +++ b/src/main/java/pl/project13/maven/git/NativeGitProvider.java @@ -212,7 +212,19 @@ public String getRemoteOriginUrl() throws GitCommitIdExecutionException { @Override public String getClosestTagName() throws GitCommitIdExecutionException { try { - return runGitCommand(canonical, "describe --abbrev=0 --tags"); + StringBuilder argumentsForGitDescribe = new StringBuilder(); + argumentsForGitDescribe.append("describe --abbrev=0"); + if (gitDescribe != null) { + if (gitDescribe.getTags()) { + argumentsForGitDescribe.append(" --tags"); + } + + final String matchOption = gitDescribe.getMatch(); + if (matchOption != null && !matchOption.isEmpty()) { + argumentsForGitDescribe.append(" --match=").append(matchOption); + } + } + return runGitCommand(canonical, argumentsForGitDescribe.toString()); } catch (NativeCommandException ignore) { // could not find any tags to describe } diff --git a/src/test/java/pl/project13/maven/git/GitCommitIdMojoIntegrationTest.java b/src/test/java/pl/project13/maven/git/GitCommitIdMojoIntegrationTest.java index 1b9654e5..71cc89d1 100644 --- a/src/test/java/pl/project13/maven/git/GitCommitIdMojoIntegrationTest.java +++ b/src/test/java/pl/project13/maven/git/GitCommitIdMojoIntegrationTest.java @@ -869,6 +869,80 @@ public void testDetectDirtyWorkingDirectory(boolean useNativeGit) throws Excepti assertThat(properties).includes(entry("git.commit.id.describe", "0b0181b" + dirtySuffix)); // assert dirtySuffix at the end! } + @Test + @Parameters(method = "useNativeGit") + public void shouldGenerateClosestTagInformationWithExcludeLightweightTagsForClosestTag(boolean useNativeGit) throws Exception { + // given + mavenSandbox + .withParentProject("my-jar-project", "jar") + .withNoChildProject() + .withGitRepoInParent(AvailableGitTestRepo.WITH_LIGHTWEIGHT_TAG_BEFORE_ANNOTATED_TAG) + .create(); + + MavenProject targetProject = mavenSandbox.getParentProject(); + setProjectToExecuteMojoIn(targetProject); + + GitDescribeConfig gitDescribe = createGitDescribeConfig(true, 9); + gitDescribe.setDirty("-customDirtyMark"); + gitDescribe.setTags(false); // exclude lightweight tags + + alterMojoSettings("gitDescribe", gitDescribe); + alterMojoSettings("useNativeGit", useNativeGit); + + // when + mojo.execute(); + + // then + assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.commit.id.abbrev"); + assertThat(targetProject.getProperties().getProperty("git.commit.id.abbrev")).isEqualTo("b6a73ed"); + + assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.commit.id.describe"); + assertThat(targetProject.getProperties().getProperty("git.commit.id.describe")).isEqualTo("annotated-tag-2-gb6a73ed74-customDirtyMark"); + + assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.closest.tag.name"); + assertThat(targetProject.getProperties().getProperty("git.closest.tag.name")).isEqualTo("annotated-tag"); + + assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.closest.tag.commit.count"); + assertThat(targetProject.getProperties().getProperty("git.closest.tag.commit.count")).isEqualTo("2"); + } + + @Test + @Parameters(method = "useNativeGit") + public void shouldGenerateClosestTagInformationWithIncludeLightweightTagsForClosestTag(boolean useNativeGit) throws Exception { + // given + mavenSandbox + .withParentProject("my-jar-project", "jar") + .withNoChildProject() + .withGitRepoInParent(AvailableGitTestRepo.WITH_LIGHTWEIGHT_TAG_BEFORE_ANNOTATED_TAG) + .create(); + + MavenProject targetProject = mavenSandbox.getParentProject(); + setProjectToExecuteMojoIn(targetProject); + + GitDescribeConfig gitDescribe = createGitDescribeConfig(true, 9); + gitDescribe.setDirty("-customDirtyMark"); + gitDescribe.setTags(true); // include lightweight tags + + alterMojoSettings("gitDescribe", gitDescribe); + alterMojoSettings("useNativeGit", useNativeGit); + + // when + mojo.execute(); + + // then + assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.commit.id.abbrev"); + assertThat(targetProject.getProperties().getProperty("git.commit.id.abbrev")).isEqualTo("b6a73ed"); + + assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.commit.id.describe"); + assertThat(targetProject.getProperties().getProperty("git.commit.id.describe")).isEqualTo("lightweight-tag-1-gb6a73ed74-customDirtyMark"); + + assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.closest.tag.name"); + assertThat(targetProject.getProperties().getProperty("git.closest.tag.name")).isEqualTo("lightweight-tag"); + + assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.closest.tag.commit.count"); + assertThat(targetProject.getProperties().getProperty("git.closest.tag.commit.count")).isEqualTo("1"); + } + private GitDescribeConfig createGitDescribeConfig(boolean forceLongFormat, int abbrev) { GitDescribeConfig gitDescribeConfig = new GitDescribeConfig(); gitDescribeConfig.setTags(true); From 311e7ceb9b972a975dff0dd3ab14b798803d12f2 Mon Sep 17 00:00:00 2001 From: TheSnoozer Date: Mon, 16 Oct 2017 23:31:00 -0400 Subject: [PATCH 2/5] cleanup test and introduce a common method that verifies if a key is present inside the properties and matches a specific value --- .../git/GitCommitIdMojoIntegrationTest.java | 81 +++++++------------ 1 file changed, 29 insertions(+), 52 deletions(-) diff --git a/src/test/java/pl/project13/maven/git/GitCommitIdMojoIntegrationTest.java b/src/test/java/pl/project13/maven/git/GitCommitIdMojoIntegrationTest.java index 71cc89d1..13e4e1a3 100644 --- a/src/test/java/pl/project13/maven/git/GitCommitIdMojoIntegrationTest.java +++ b/src/test/java/pl/project13/maven/git/GitCommitIdMojoIntegrationTest.java @@ -300,8 +300,7 @@ public void shouldGenerateDescribeWithTagOnlyWhenForceLongFormatIsFalse(boolean mojo.execute(); // then - assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.commit.id.describe"); - assertThat(targetProject.getProperties().getProperty("git.commit.id.describe")).isEqualTo("v1.0.0-dirty"); + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.commit.id.describe", "v1.0.0-dirty"); } @Test @@ -324,13 +323,9 @@ public void shouldGenerateDescribeWithTagOnlyWhenForceLongFormatIsFalseAndAbbrev mojo.execute(); // then - Set propNames = targetProject.getProperties().stringPropertyNames(); + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.commit.id.describe", "v1.0.0"); - assertThat(propNames).contains("git.commit.id.describe"); - assertThat(targetProject.getProperties().getProperty("git.commit.id.describe")).isEqualTo("v1.0.0"); - - assertThat(propNames).contains("git.commit.id.describe-short"); - assertThat(targetProject.getProperties().getProperty("git.commit.id.describe-short")).isEqualTo("v1.0.0"); + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.commit.id.describe-short", "v1.0.0"); } @Test @@ -353,8 +348,7 @@ public void shouldGenerateDescribeWithTagAndZeroAndCommitIdWhenForceLongFormatIs mojo.execute(); // then - assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.commit.id.describe"); - assertThat(targetProject.getProperties().getProperty("git.commit.id.describe")).isEqualTo("v1.0.0-0-gde4db35"); + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.commit.id.describe", "v1.0.0-0-gde4db35"); } @Test @@ -377,13 +371,9 @@ public void shouldGenerateDescribeWithTagAndZeroAndCommitIdWhenForceLongFormatIs mojo.execute(); // then - Set propNames = targetProject.getProperties().stringPropertyNames(); - - assertThat(propNames).contains("git.commit.id.describe"); - assertThat(targetProject.getProperties().getProperty("git.commit.id.describe")).isEqualTo("v1.0.0-0-gde4db35917"); + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.commit.id.describe", "v1.0.0-0-gde4db35917"); - assertThat(propNames).contains("git.commit.id.describe-short"); - assertThat(targetProject.getProperties().getProperty("git.commit.id.describe-short")).isEqualTo("v1.0.0-0"); + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.commit.id.describe-short", "v1.0.0-0"); } @Test @@ -405,8 +395,7 @@ public void shouldGenerateCommitIdAbbrevWithDefaultLength(boolean useNativeGit) mojo.execute(); // then - assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.commit.id.abbrev"); - assertThat(targetProject.getProperties().getProperty("git.commit.id.abbrev")).isEqualTo("de4db35"); + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.commit.id.abbrev", "de4db35"); } @Test @@ -527,8 +516,7 @@ public void shouldAlwaysPrintGitDescribe(boolean useNativeGit) throws Exception mojo.execute(); // then - assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.commit.id.describe"); - assertThat(targetProject.getProperties().getProperty("git.commit.id.describe")).isEqualTo("0b0181b"); + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.commit.id.describe", "0b0181b"); } @Test @@ -674,11 +662,9 @@ public void shouldGenerateClosestTagInformationWhenOnATag(boolean useNativeGit) mojo.execute(); // then - assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.closest.tag.name"); - assertThat(targetProject.getProperties().getProperty("git.closest.tag.name")).isEqualTo("v1.0.0"); + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.closest.tag.name", "v1.0.0"); - assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.closest.tag.commit.count"); - assertThat(targetProject.getProperties().getProperty("git.closest.tag.commit.count")).isEqualTo("0"); + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.closest.tag.commit.count", "0"); } @Test @@ -703,11 +689,9 @@ public void shouldGenerateClosestTagInformationWhenOnATagAndDirty(boolean useNat mojo.execute(); // then - assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.closest.tag.name"); - assertThat(targetProject.getProperties().getProperty("git.closest.tag.name")).isEqualTo("v1.0.0"); + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.closest.tag.name", "v1.0.0"); - assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.closest.tag.commit.count"); - assertThat(targetProject.getProperties().getProperty("git.closest.tag.commit.count")).isEqualTo("0"); + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.closest.tag.commit.count", "0"); } @@ -736,11 +720,9 @@ public void shouldGenerateClosestTagInformationWhenCommitHasTwoTags(boolean useN // then // AvailableGitTestRepo.WITH_COMMIT_THAT_HAS_TWO_TAGS ==> Where the newest-tag was created latest - assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.closest.tag.name"); - assertThat(targetProject.getProperties().getProperty("git.closest.tag.name")).isEqualTo("newest-tag"); + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.closest.tag.name", "newest-tag"); - assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.closest.tag.commit.count"); - assertThat(targetProject.getProperties().getProperty("git.closest.tag.commit.count")).isEqualTo("0"); + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.closest.tag.commit.count", "0"); } @Test @@ -777,11 +759,9 @@ public void shouldUseDateFormatTimeZone(boolean useNativeGit) throws Exception { // then Properties properties = targetProject.getProperties(); - assertThat(properties.stringPropertyNames()).contains("git.commit.time"); - assertThat(properties.getProperty("git.commit.time")).isEqualTo(expectedTimeZoneOffset); + assertPropertyPresentAndEqual(properties, "git.commit.time", expectedTimeZoneOffset); - assertThat(properties.stringPropertyNames()).contains("git.build.time"); - assertThat(properties.getProperty("git.build.time")).isEqualTo(expectedTimeZoneOffset); + assertPropertyPresentAndEqual(properties, "git.build.time", expectedTimeZoneOffset); // set the timezone back TimeZone.setDefault(currentDefaultTimeZone); @@ -893,17 +873,13 @@ public void shouldGenerateClosestTagInformationWithExcludeLightweightTagsForClos mojo.execute(); // then - assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.commit.id.abbrev"); - assertThat(targetProject.getProperties().getProperty("git.commit.id.abbrev")).isEqualTo("b6a73ed"); + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.commit.id.abbrev", "b6a73ed"); - assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.commit.id.describe"); - assertThat(targetProject.getProperties().getProperty("git.commit.id.describe")).isEqualTo("annotated-tag-2-gb6a73ed74-customDirtyMark"); + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.commit.id.describe", "annotated-tag-2-gb6a73ed74-customDirtyMark"); - assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.closest.tag.name"); - assertThat(targetProject.getProperties().getProperty("git.closest.tag.name")).isEqualTo("annotated-tag"); + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.closest.tag.name", "annotated-tag"); - assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.closest.tag.commit.count"); - assertThat(targetProject.getProperties().getProperty("git.closest.tag.commit.count")).isEqualTo("2"); + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.closest.tag.commit.count", "2"); } @Test @@ -930,17 +906,13 @@ public void shouldGenerateClosestTagInformationWithIncludeLightweightTagsForClos mojo.execute(); // then - assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.commit.id.abbrev"); - assertThat(targetProject.getProperties().getProperty("git.commit.id.abbrev")).isEqualTo("b6a73ed"); + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.commit.id.abbrev", "b6a73ed"); - assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.commit.id.describe"); - assertThat(targetProject.getProperties().getProperty("git.commit.id.describe")).isEqualTo("lightweight-tag-1-gb6a73ed74-customDirtyMark"); + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.commit.id.describe", "lightweight-tag-1-gb6a73ed74-customDirtyMark"); - assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.closest.tag.name"); - assertThat(targetProject.getProperties().getProperty("git.closest.tag.name")).isEqualTo("lightweight-tag"); + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.closest.tag.name", "lightweight-tag"); - assertThat(targetProject.getProperties().stringPropertyNames()).contains("git.closest.tag.commit.count"); - assertThat(targetProject.getProperties().getProperty("git.closest.tag.commit.count")).isEqualTo("1"); + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.closest.tag.commit.count", "1"); } private GitDescribeConfig createGitDescribeConfig(boolean forceLongFormat, int abbrev) { @@ -956,6 +928,11 @@ private void alterMojoSettings(String parameterName, Object parameterValue) { setInternalState(mojo, parameterName, parameterValue); } + 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")); From a984c2bde9d69c697b1e901ac0252aebfeb7e239 Mon Sep 17 00:00:00 2001 From: TheSnoozer Date: Mon, 16 Oct 2017 23:38:53 -0400 Subject: [PATCH 3/5] replace alterMojoSettings with proper method calls and remove the org/mockito/internal/util/reflection/Whitebox --- .../project13/maven/git/GitCommitIdMojo.java | 34 +++- .../internal/util/reflection/Whitebox.java | 40 ----- .../git/GitCommitIdMojoIntegrationTest.java | 151 +++++++++--------- .../maven/git/GitCommitIdMojoTest.java | 2 +- .../maven/git/GitIntegrationTest.java | 32 ++-- .../maven/git/GitPropertiesFileTest.java | 11 +- .../maven/git/GitSubmodulesTest.java | 5 +- .../maven/git/NaivePerformanceTest.java | 9 +- .../maven/git/NativeAndJGitProviderTest.java | 13 +- .../checks/checkstyle-suppressions.xml | 2 +- 10 files changed, 131 insertions(+), 168 deletions(-) delete mode 100644 src/test/java/org/mockito/internal/util/reflection/Whitebox.java diff --git a/src/main/java/pl/project13/maven/git/GitCommitIdMojo.java b/src/main/java/pl/project13/maven/git/GitCommitIdMojo.java index 4fe12983..a551eb8d 100644 --- a/src/main/java/pl/project13/maven/git/GitCommitIdMojo.java +++ b/src/main/java/pl/project13/maven/git/GitCommitIdMojo.java @@ -716,6 +716,10 @@ public void setVerbose(boolean verbose) { this.verbose = verbose; } + public void setProject(MavenProject project) { + this.project = project; + } + public void setDotGitDirectory(File dotGitDirectory) { this.dotGitDirectory = dotGitDirectory; } @@ -748,11 +752,39 @@ public void setIncludeOnlyProperties(List includeOnlyProperties) { this.includeOnlyProperties = includeOnlyProperties; } - public void useNativeGit(boolean useNativeGit) { + public void setUseNativeGit(boolean useNativeGit) { this.useNativeGit = useNativeGit; } public void setCommitIdGenerationMode(String commitIdGenerationMode) { this.commitIdGenerationMode = commitIdGenerationMode; } + + public void setSkip(boolean skip) { + this.skip = skip; + } + + public void setSkipPoms(boolean skipPoms) { + this.skipPoms = skipPoms; + } + + public void setGenerateGitPropertiesFile(boolean generateGitPropertiesFile) { + this.generateGitPropertiesFile = generateGitPropertiesFile; + } + + public void setGenerateGitPropertiesFilename(String generateGitPropertiesFilename) { + this.generateGitPropertiesFilename = generateGitPropertiesFilename; + } + + public void setDateFormatTimeZone(String dateFormatTimeZone) { + this.dateFormatTimeZone = dateFormatTimeZone; + } + + public void setFailOnNoGitDirectory(boolean failOnNoGitDirectory) { + this.failOnNoGitDirectory = failOnNoGitDirectory; + } + + public void setFailOnUnableToExtractRepoInfo(boolean failOnUnableToExtractRepoInfo) { + this.failOnUnableToExtractRepoInfo = failOnUnableToExtractRepoInfo; + } } diff --git a/src/test/java/org/mockito/internal/util/reflection/Whitebox.java b/src/test/java/org/mockito/internal/util/reflection/Whitebox.java deleted file mode 100644 index b1b98135..00000000 --- a/src/test/java/org/mockito/internal/util/reflection/Whitebox.java +++ /dev/null @@ -1,40 +0,0 @@ -package org.mockito.internal.util.reflection; - -import java.lang.reflect.Field; - -@Deprecated -public class Whitebox { - public static void setInternalState(Object target, String field, Object value) { - Class c = target.getClass(); - try { - Field f = getFieldFromHierarchy(c, field); - f.setAccessible(true); - f.set(target, value); - } catch (Exception e) { - throw new RuntimeException("Unable to set internal state on a private field. Please report to mockito mailing list.", e); - } - } - - private static Field getFieldFromHierarchy(Class clazz, String field) { - Field f = getField(clazz, field); - while (f == null && clazz != Object.class) { - clazz = clazz.getSuperclass(); - f = getField(clazz, field); - } - if (f == null) { - throw new RuntimeException( - "You want me to get this field: '" + field + - "' on this class: '" + clazz.getSimpleName() + - "' but this field is not declared withing hierarchy of this class!"); - } - return f; - } - - private static Field getField(Class clazz, String field) { - try { - return clazz.getDeclaredField(field); - } catch (NoSuchFieldException e) { - return null; - } - } -} diff --git a/src/test/java/pl/project13/maven/git/GitCommitIdMojoIntegrationTest.java b/src/test/java/pl/project13/maven/git/GitCommitIdMojoIntegrationTest.java index 13e4e1a3..a2fe0b2d 100644 --- a/src/test/java/pl/project13/maven/git/GitCommitIdMojoIntegrationTest.java +++ b/src/test/java/pl/project13/maven/git/GitCommitIdMojoIntegrationTest.java @@ -38,7 +38,6 @@ import static java.util.Arrays.*; import static org.fest.assertions.Assertions.assertThat; import static org.fest.assertions.MapAssert.entry; -import static org.mockito.internal.util.reflection.Whitebox.setInternalState; @RunWith(JUnitParamsRunner.class) public class GitCommitIdMojoIntegrationTest extends GitIntegrationTest { @@ -61,7 +60,7 @@ public void shouldResolvePropertiesOnDefaultSettingsForNonPomProject(boolean use mavenSandbox.withParentProject("my-jar-project", "jar").withNoChildProject().withGitRepoInParent(AvailableGitTestRepo.WITH_ONE_COMMIT).create(); MavenProject targetProject = mavenSandbox.getParentProject(); setProjectToExecuteMojoIn(targetProject); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -77,8 +76,8 @@ public void shouldNotRunWhenSkipIsSet(boolean useNativeGit) throws Exception { mavenSandbox.withParentProject("my-skip-project", "jar").withNoChildProject().withGitRepoInParent(AvailableGitTestRepo.WITH_ONE_COMMIT).create(); MavenProject targetProject = mavenSandbox.getParentProject(); setProjectToExecuteMojoIn(targetProject); - alterMojoSettings("skip", true); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setSkip(true); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -94,7 +93,7 @@ public void shouldNotRunWhenPackagingPomAndDefaultSettingsApply(boolean useNativ mavenSandbox.withParentProject("my-pom-project", "pom").withNoChildProject().withGitRepoInParent(AvailableGitTestRepo.WITH_ONE_COMMIT).create(); MavenProject targetProject = mavenSandbox.getParentProject(); setProjectToExecuteMojoIn(targetProject); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -110,8 +109,8 @@ public void shouldRunWhenPackagingPomAndSkipPomsFalse(boolean useNativeGit) thro mavenSandbox.withParentProject("my-pom-project", "pom").withNoChildProject().withGitRepoInParent(AvailableGitTestRepo.WITH_ONE_COMMIT).create(); MavenProject targetProject = mavenSandbox.getParentProject(); setProjectToExecuteMojoIn(targetProject); - alterMojoSettings("skipPoms", false); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setSkipPoms(false); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -127,8 +126,8 @@ public void shouldUseParentProjectRepoWhenInvokedFromChild(boolean useNativeGit) mavenSandbox.withParentProject("my-pom-project", "pom").withChildProject("my-jar-module", "jar").withGitRepoInParent(AvailableGitTestRepo.WITH_ONE_COMMIT).create(); MavenProject targetProject = mavenSandbox.getChildProject(); setProjectToExecuteMojoIn(targetProject); - alterMojoSettings("skipPoms", false); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setSkipPoms(false); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -144,8 +143,8 @@ public void shouldUseChildProjectRepoIfInvokedFromChild(boolean useNativeGit) th mavenSandbox.withParentProject("my-pom-project", "pom").withChildProject("my-jar-module", "jar").withGitRepoInChild(AvailableGitTestRepo.WITH_ONE_COMMIT).create(); MavenProject targetProject = mavenSandbox.getChildProject(); setProjectToExecuteMojoIn(targetProject); - alterMojoSettings("skipPoms", false); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setSkipPoms(false); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -165,8 +164,8 @@ public void shouldFailWithExceptionWhenNoGitRepoFound(boolean useNativeGit) thro MavenProject targetProject = mavenSandbox.getChildProject(); setProjectToExecuteMojoIn(targetProject); - alterMojoSettings("skipPoms", false); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setSkipPoms(false); + mojo.setUseNativeGit(useNativeGit); mojo.execute(); } @@ -186,9 +185,9 @@ public void shouldGenerateCustomPropertiesFileProperties(boolean useNativeGit) t File expectedFile = new File(targetProject.getBasedir(), targetFilePath); setProjectToExecuteMojoIn(targetProject); - alterMojoSettings("generateGitPropertiesFile", true); - alterMojoSettings("generateGitPropertiesFilename", targetFilePath); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setGenerateGitPropertiesFile(true); + mojo.setGenerateGitPropertiesFilename(targetFilePath); + mojo.setUseNativeGit(useNativeGit); // when try { @@ -216,10 +215,10 @@ public void shouldGenerateCustomPropertiesFileJson(boolean useNativeGit) throws File expectedFile = new File(targetProject.getBasedir(), targetFilePath); setProjectToExecuteMojoIn(targetProject); - alterMojoSettings("generateGitPropertiesFile", true); - alterMojoSettings("generateGitPropertiesFilename", targetFilePath); - alterMojoSettings("format", "json"); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setGenerateGitPropertiesFile(true); + mojo.setGenerateGitPropertiesFilename(targetFilePath); + mojo.setFormat("json"); + mojo.setUseNativeGit(useNativeGit); // when try { mojo.execute(); @@ -247,8 +246,8 @@ public void shouldSkipWithoutFailOnNoGitDirectoryWhenNoGitRepoFound(boolean useN MavenProject targetProject = mavenSandbox.getParentProject(); setProjectToExecuteMojoIn(targetProject); - alterMojoSettings("failOnNoGitDirectory", false); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setFailOnNoGitDirectory(false); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -268,8 +267,8 @@ public void shouldNotSkipWithoutFailOnNoGitDirectoryWhenNoGitRepoIsPresent(boole MavenProject targetProject = mavenSandbox.getParentProject(); setProjectToExecuteMojoIn(targetProject); - alterMojoSettings("failOnNoGitDirectory", false); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setFailOnNoGitDirectory(false); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -293,8 +292,8 @@ public void shouldGenerateDescribeWithTagOnlyWhenForceLongFormatIsFalse(boolean GitDescribeConfig gitDescribeConfig = createGitDescribeConfig(false, 7); gitDescribeConfig.setDirty("-dirty"); // checking if dirty works as expected - alterMojoSettings("gitDescribe", gitDescribeConfig); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setGitDescribe(gitDescribeConfig); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -316,8 +315,8 @@ public void shouldGenerateDescribeWithTagOnlyWhenForceLongFormatIsFalseAndAbbrev setProjectToExecuteMojoIn(targetProject); GitDescribeConfig gitDescribeConfig = createGitDescribeConfig(false, 10); - alterMojoSettings("gitDescribe", gitDescribeConfig); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setGitDescribe(gitDescribeConfig); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -341,8 +340,8 @@ public void shouldGenerateDescribeWithTagAndZeroAndCommitIdWhenForceLongFormatIs setProjectToExecuteMojoIn(targetProject); GitDescribeConfig gitDescribeConfig = createGitDescribeConfig(true, 7); - alterMojoSettings("gitDescribe", gitDescribeConfig); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setGitDescribe(gitDescribeConfig); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -364,8 +363,8 @@ public void shouldGenerateDescribeWithTagAndZeroAndCommitIdWhenForceLongFormatIs setProjectToExecuteMojoIn(targetProject); GitDescribeConfig gitDescribeConfig = createGitDescribeConfig(true, 10); - alterMojoSettings("gitDescribe", gitDescribeConfig); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setGitDescribe(gitDescribeConfig); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -388,8 +387,8 @@ public void shouldGenerateCommitIdAbbrevWithDefaultLength(boolean useNativeGit) MavenProject targetProject = mavenSandbox.getChildProject(); setProjectToExecuteMojoIn(targetProject); - alterMojoSettings("abbrevLength", 7); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setAbbrevLength(7); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -410,8 +409,8 @@ public void shouldGenerateCommitIdAbbrevWithNonDefaultLength(boolean useNativeGi MavenProject targetProject = mavenSandbox.getChildProject(); setProjectToExecuteMojoIn(targetProject); - alterMojoSettings("abbrevLength", 10); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setAbbrevLength(10); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -432,8 +431,8 @@ public void shouldFormatDate(boolean useNativeGit) throws Exception { setProjectToExecuteMojoIn(targetProject); String dateFormat = "MM/dd/yyyy"; - alterMojoSettings("dateFormat", dateFormat); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setDateFormat(dateFormat); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -460,8 +459,8 @@ public void shouldSkipGitDescribe(boolean useNativeGit) throws Exception { GitDescribeConfig gitDescribeConfig = createGitDescribeConfig(true, 7); gitDescribeConfig.setSkip(true); - alterMojoSettings("gitDescribe", gitDescribeConfig); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setGitDescribe(gitDescribeConfig); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -485,8 +484,8 @@ public void shouldMarkGitDescribeAsDirty(boolean useNativeGit) throws Exception GitDescribeConfig gitDescribeConfig = createGitDescribeConfig(true, 7); String dirtySuffix = "-dirtyTest"; gitDescribeConfig.setDirty(dirtySuffix); - alterMojoSettings("gitDescribe", gitDescribeConfig); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setGitDescribe(gitDescribeConfig); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -509,8 +508,8 @@ public void shouldAlwaysPrintGitDescribe(boolean useNativeGit) throws Exception GitDescribeConfig gitDescribeConfig = createGitDescribeConfig(true, 7); gitDescribeConfig.setAlways(true); - alterMojoSettings("gitDescribe", gitDescribeConfig); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setGitDescribe(gitDescribeConfig); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -532,8 +531,8 @@ public void shouldWorkWithEmptyGitDescribe(boolean useNativeGit) throws Exceptio setProjectToExecuteMojoIn(targetProject); GitDescribeConfig gitDescribeConfig = new GitDescribeConfig(); - alterMojoSettings("gitDescribe", gitDescribeConfig); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setGitDescribe(gitDescribeConfig); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -554,8 +553,8 @@ public void shouldWorkWithNullGitDescribe(boolean useNativeGit) throws Exception setProjectToExecuteMojoIn(targetProject); - alterMojoSettings("gitDescribe", null); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setGitDescribe(null); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -582,8 +581,8 @@ public void shouldExtractTagsOnGivenCommit(boolean useNativeGit) throws Exceptio MavenProject targetProject = mavenSandbox.getParentProject(); setProjectToExecuteMojoIn(targetProject); - alterMojoSettings("gitDescribe", null); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setGitDescribe(null); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -624,8 +623,8 @@ public void runGitDescribeWithMatchOption(boolean useNativeGit) throws Exception gitDescribeConfig.setMatch(gitDescribeMatchNeedle); gitDescribeConfig.setAlways(false); - alterMojoSettings("gitDescribe", gitDescribeConfig); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setGitDescribe(gitDescribeConfig); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -655,8 +654,8 @@ public void shouldGenerateClosestTagInformationWhenOnATag(boolean useNativeGit) GitDescribeConfig gitDescribeConfig = createGitDescribeConfig(false, 7); gitDescribeConfig.setDirty("-dirty"); // checking if dirty works as expected - alterMojoSettings("gitDescribe", gitDescribeConfig); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setGitDescribe(gitDescribeConfig); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -682,8 +681,8 @@ public void shouldGenerateClosestTagInformationWhenOnATagAndDirty(boolean useNat GitDescribeConfig gitDescribeConfig = createGitDescribeConfig(true, 7); String dirtySuffix = "-dirtyTest"; gitDescribeConfig.setDirty(dirtySuffix); - alterMojoSettings("gitDescribe", gitDescribeConfig); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setGitDescribe(gitDescribeConfig); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -712,8 +711,8 @@ public void shouldGenerateClosestTagInformationWhenCommitHasTwoTags(boolean useN MavenProject targetProject = mavenSandbox.getParentProject(); setProjectToExecuteMojoIn(targetProject); - alterMojoSettings("gitDescribe", null); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setGitDescribe(null); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -745,10 +744,10 @@ public void shouldUseDateFormatTimeZone(boolean useNativeGit) throws Exception { TimeZone executionTimeZone = TimeZone.getTimeZone("GMT" + executionTimeZoneOffset); GitDescribeConfig gitDescribeConfig = createGitDescribeConfig(true, 7); - alterMojoSettings("gitDescribe", gitDescribeConfig); - alterMojoSettings("useNativeGit", useNativeGit); - alterMojoSettings("dateFormat", dateFormat); - alterMojoSettings("dateFormatTimeZone", expectedTimeZone.getID()); + mojo.setGitDescribe(gitDescribeConfig); + mojo.setUseNativeGit(useNativeGit); + mojo.setDateFormat(dateFormat); + mojo.setDateFormatTimeZone(expectedTimeZone.getID()); // override the default timezone for execution and testing TimeZone currentDefaultTimeZone = TimeZone.getDefault(); @@ -779,8 +778,8 @@ public void shouldGenerateCommitIdOldFashioned(boolean useNativeGit) throws Exce setProjectToExecuteMojoIn(targetProject); - alterMojoSettings("useNativeGit", useNativeGit); - alterMojoSettings("commitIdGenerationMode", "flat"); + mojo.setUseNativeGit(useNativeGit); + mojo.setCommitIdGenerationMode("flat"); // when mojo.execute(); @@ -806,10 +805,10 @@ public void testDetectCleanWorkingDirectory(boolean useNativeGit) throws Excepti GitDescribeConfig gitDescribeConfig = createGitDescribeConfig(true, 7); String dirtySuffix = "-dirtyTest"; gitDescribeConfig.setDirty(dirtySuffix); - alterMojoSettings("gitDescribe", gitDescribeConfig); + mojo.setGitDescribe(gitDescribeConfig); - alterMojoSettings("useNativeGit", useNativeGit); - alterMojoSettings("commitIdGenerationMode", "flat"); + mojo.setUseNativeGit(useNativeGit); + mojo.setCommitIdGenerationMode("flat"); // when mojo.execute(); @@ -835,10 +834,10 @@ public void testDetectDirtyWorkingDirectory(boolean useNativeGit) throws Excepti GitDescribeConfig gitDescribeConfig = createGitDescribeConfig(true, 7); String dirtySuffix = "-dirtyTest"; gitDescribeConfig.setDirty(dirtySuffix); - alterMojoSettings("gitDescribe", gitDescribeConfig); + mojo.setGitDescribe(gitDescribeConfig); - alterMojoSettings("useNativeGit", useNativeGit); - alterMojoSettings("commitIdGenerationMode", "flat"); + mojo.setUseNativeGit(useNativeGit); + mojo.setCommitIdGenerationMode("flat"); // when mojo.execute(); @@ -866,8 +865,8 @@ public void shouldGenerateClosestTagInformationWithExcludeLightweightTagsForClos gitDescribe.setDirty("-customDirtyMark"); gitDescribe.setTags(false); // exclude lightweight tags - alterMojoSettings("gitDescribe", gitDescribe); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setGitDescribe(gitDescribe); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -899,8 +898,8 @@ public void shouldGenerateClosestTagInformationWithIncludeLightweightTagsForClos gitDescribe.setDirty("-customDirtyMark"); gitDescribe.setTags(true); // include lightweight tags - alterMojoSettings("gitDescribe", gitDescribe); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setGitDescribe(gitDescribe); + mojo.setUseNativeGit(useNativeGit); // when mojo.execute(); @@ -924,10 +923,6 @@ private GitDescribeConfig createGitDescribeConfig(boolean forceLongFormat, int a return gitDescribeConfig; } - private void alterMojoSettings(String parameterName, Object parameterValue) { - setInternalState(mojo, parameterName, parameterValue); - } - private void assertPropertyPresentAndEqual(Properties properties, String key, String expected) { assertThat(properties.stringPropertyNames()).contains(key); assertThat(properties.getProperty(key)).isEqualTo(expected); diff --git a/src/test/java/pl/project13/maven/git/GitCommitIdMojoTest.java b/src/test/java/pl/project13/maven/git/GitCommitIdMojoTest.java index d8c67bca..84b695e5 100644 --- a/src/test/java/pl/project13/maven/git/GitCommitIdMojoTest.java +++ b/src/test/java/pl/project13/maven/git/GitCommitIdMojoTest.java @@ -61,7 +61,7 @@ public void setUp() throws Exception { mojo.setAbbrevLength(abbrevLength); mojo.setDateFormat(dateFormat); mojo.setVerbose(true); - mojo.useNativeGit(false); + mojo.setUseNativeGit(false); mojo.setGitDescribe(gitDescribeConfig); mojo.setCommitIdGenerationMode("full"); diff --git a/src/test/java/pl/project13/maven/git/GitIntegrationTest.java b/src/test/java/pl/project13/maven/git/GitIntegrationTest.java index 52397524..0348d2d5 100644 --- a/src/test/java/pl/project13/maven/git/GitIntegrationTest.java +++ b/src/test/java/pl/project13/maven/git/GitIntegrationTest.java @@ -27,12 +27,8 @@ import java.io.File; import java.io.IOException; -import java.util.HashMap; -import java.util.Map; import java.util.concurrent.ThreadLocalRandom; -import static org.mockito.internal.util.reflection.Whitebox.setInternalState; - public abstract class GitIntegrationTest { private static final String SANDBOX_DIR = "target" + File.separator + "sandbox" + File.separator; @@ -99,25 +95,21 @@ protected File dotGitDir(@NotNull Optional projectDir) { } public static void initializeMojoWithDefaults(GitCommitIdMojo mojo) { - Map mojoDefaults = new HashMap<>(); - mojoDefaults.put("verbose", false); - mojoDefaults.put("skipPoms", true); - mojoDefaults.put("abbrevLength", 7); - mojoDefaults.put("generateGitPropertiesFile", false); - mojoDefaults.put("generateGitPropertiesFilename", "src/main/resources/git.properties"); - mojoDefaults.put("prefix", "git"); - mojoDefaults.put("dateFormat", "yyyy-MM-dd'T'HH:mm:ssZ"); - mojoDefaults.put("failOnNoGitDirectory", true); - mojoDefaults.put("useNativeGit", false); - mojoDefaults.put("commitIdGenerationMode", "full"); - for (Map.Entry entry : mojoDefaults.entrySet()) { - setInternalState(mojo, entry.getKey(), entry.getValue()); - } + mojo.setVerbose(false); + mojo.setSkipPoms(true); + mojo.setAbbrevLength(7); + mojo.setGenerateGitPropertiesFile(false); + mojo.setGenerateGitPropertiesFilename("src/main/resources/git.properties"); + mojo.setPrefix("git"); + mojo.setDateFormat("yyyy-MM-dd'T'HH:mm:ssZ"); + mojo.setFailOnNoGitDirectory(true); + mojo.setUseNativeGit(false); + mojo.setCommitIdGenerationMode("full"); } public void setProjectToExecuteMojoIn(@NotNull MavenProject project) { - setInternalState(mojo, "project", project); - setInternalState(mojo, "dotGitDirectory", new File(project.getBasedir(), ".git")); + mojo.setProject(project); + mojo.setDotGitDirectory(new File(project.getBasedir(), ".git")); } } diff --git a/src/test/java/pl/project13/maven/git/GitPropertiesFileTest.java b/src/test/java/pl/project13/maven/git/GitPropertiesFileTest.java index 022b41bc..d04ee004 100644 --- a/src/test/java/pl/project13/maven/git/GitPropertiesFileTest.java +++ b/src/test/java/pl/project13/maven/git/GitPropertiesFileTest.java @@ -30,7 +30,6 @@ import static java.util.Arrays.asList; import static org.fest.assertions.Assertions.assertThat; -import static org.mockito.internal.util.reflection.Whitebox.setInternalState; @RunWith(JUnitParamsRunner.class) public class GitPropertiesFileTest extends GitIntegrationTest { @@ -58,9 +57,9 @@ public void shouldConformPropertiesFileWhenSpecialCharactersInValueString(boolea File expectedFile = new File(targetProject.getBasedir(), targetFilePath); setProjectToExecuteMojoIn(targetProject); - alterMojoSettings("generateGitPropertiesFile", true); - alterMojoSettings("generateGitPropertiesFilename", targetFilePath); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setGenerateGitPropertiesFile(true); + mojo.setGenerateGitPropertiesFilename(targetFilePath); + mojo.setUseNativeGit(useNativeGit); // when try { @@ -95,10 +94,6 @@ public void shouldConformPropertiesFileWhenSpecialCharactersInValueString(boolea } } - private void alterMojoSettings(String parameterName, Object parameterValue) { - setInternalState(mojo, parameterName, parameterValue); - } - private void assertGitPropertiesPresentInProject(Properties properties) { assertThat(properties).satisfies(new ContainsKeyCondition("git.build.time")); assertThat(properties).satisfies(new ContainsKeyCondition("git.build.host")); diff --git a/src/test/java/pl/project13/maven/git/GitSubmodulesTest.java b/src/test/java/pl/project13/maven/git/GitSubmodulesTest.java index b008c4eb..8188c07e 100644 --- a/src/test/java/pl/project13/maven/git/GitSubmodulesTest.java +++ b/src/test/java/pl/project13/maven/git/GitSubmodulesTest.java @@ -25,7 +25,6 @@ import java.util.Properties; import static org.fest.assertions.Assertions.assertThat; -import static org.mockito.internal.util.reflection.Whitebox.setInternalState; public class GitSubmodulesTest extends GitIntegrationTest { @@ -48,8 +47,8 @@ public void shouldResolvePropertiesOnDefaultSettingsForNonPomProject() throws Ex } public void setProjectToExecuteMojoIn(@NotNull MavenProject project) { - setInternalState(mojo, "project", project); - setInternalState(mojo, "dotGitDirectory", new File(project.getBasedir(), ".git")); + mojo.setProject(project); + mojo.setDotGitDirectory(new File(project.getBasedir(), ".git")); } private void assertGitPropertiesPresentInProject(Properties properties) { diff --git a/src/test/java/pl/project13/maven/git/NaivePerformanceTest.java b/src/test/java/pl/project13/maven/git/NaivePerformanceTest.java index dd5fd5fc..2a8e4ba9 100644 --- a/src/test/java/pl/project13/maven/git/NaivePerformanceTest.java +++ b/src/test/java/pl/project13/maven/git/NaivePerformanceTest.java @@ -29,7 +29,6 @@ import java.util.Properties; import static org.fest.assertions.Assertions.assertThat; -import static org.mockito.internal.util.reflection.Whitebox.setInternalState; @RunWith(JUnitParamsRunner.class) public class NaivePerformanceTest extends GitIntegrationTest { @@ -61,8 +60,8 @@ public void performance(boolean useNativeGit, int iterations) throws Exception { GitDescribeConfig gitDescribeConfig = createGitDescribeConfig(true, 7); gitDescribeConfig.setAlways(true); - alterMojoSettings("gitDescribe", gitDescribeConfig); - alterMojoSettings("useNativeGit", useNativeGit); + mojo.setGitDescribe(gitDescribeConfig); + mojo.setUseNativeGit(useNativeGit); // when long startTime = System.currentTimeMillis(); @@ -85,10 +84,6 @@ private GitDescribeConfig createGitDescribeConfig(boolean forceLongFormat, int a return gitDescribeConfig; } - private void alterMojoSettings(String parameterName, Object parameterValue) { - setInternalState(mojo, parameterName, parameterValue); - } - private void assertGitPropertiesPresentInProject(Properties properties) { assertThat(properties).satisfies(new ContainsKeyCondition("git.build.time")); assertThat(properties).satisfies(new ContainsKeyCondition("git.build.host")); diff --git a/src/test/java/pl/project13/maven/git/NativeAndJGitProviderTest.java b/src/test/java/pl/project13/maven/git/NativeAndJGitProviderTest.java index 2f88332d..c1ece48a 100644 --- a/src/test/java/pl/project13/maven/git/NativeAndJGitProviderTest.java +++ b/src/test/java/pl/project13/maven/git/NativeAndJGitProviderTest.java @@ -19,7 +19,6 @@ import static org.fest.assertions.Assertions.assertThat; import static org.junit.Assert.assertEquals; -import static org.mockito.internal.util.reflection.Whitebox.setInternalState; import java.text.DateFormat; import java.text.SimpleDateFormat; @@ -101,16 +100,16 @@ public void testCompareISO8601Time() throws Exception { private void verifyNativeAndJGit(AvailableGitTestRepo repo, MavenProject targetProject, String formatString) throws Exception { setProjectToExecuteMojoIn(targetProject); - alterMojoSettings("skipPoms", false); - alterMojoSettings("dateFormat", formatString); + mojo.setSkipPoms(false); + mojo.setDateFormat(formatString); DateFormat format = new SimpleDateFormat(formatString); - alterMojoSettings("useNativeGit", false); + mojo.setUseNativeGit(false); mojo.execute(); Properties jgitProps = createCopy(targetProject.getProperties()); - alterMojoSettings("useNativeGit", true); + mojo.setUseNativeGit(true); mojo.execute(); Properties nativeProps = createCopy(targetProject.getProperties()); @@ -139,10 +138,6 @@ private void verifyNativeAndJGit(AvailableGitTestRepo repo, MavenProject targetP assertEquals("commit times parse to different time stamps", jGitCommitTimeInMs, nativeCommitTimeInMs); } - private void alterMojoSettings(String parameterName, Object parameterValue) { - setInternalState(mojo, parameterName, parameterValue); - } - private Properties createCopy(Properties orig) { Properties p = new Properties(); for (String key : orig.stringPropertyNames()) { diff --git a/src/test/resources/checks/checkstyle-suppressions.xml b/src/test/resources/checks/checkstyle-suppressions.xml index 98381106..f5342f05 100644 --- a/src/test/resources/checks/checkstyle-suppressions.xml +++ b/src/test/resources/checks/checkstyle-suppressions.xml @@ -23,6 +23,6 @@ - + \ No newline at end of file From f3db1d4d990e8c4520e4ce0aed2e0005aa6b9839 Mon Sep 17 00:00:00 2001 From: TheSnoozer Date: Tue, 17 Oct 2017 00:27:34 -0400 Subject: [PATCH 4/5] declare setters as @VisibleForTesting instead of just plain public --- .../project13/maven/git/GitCommitIdMojo.java | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/main/java/pl/project13/maven/git/GitCommitIdMojo.java b/src/main/java/pl/project13/maven/git/GitCommitIdMojo.java index a551eb8d..6e48de32 100644 --- a/src/main/java/pl/project13/maven/git/GitCommitIdMojo.java +++ b/src/main/java/pl/project13/maven/git/GitCommitIdMojo.java @@ -708,83 +708,83 @@ static class CannotReadFileException extends Exception { // SETTERS FOR TESTS ---------------------------------------------------- - public void setFormat(String format) { + @VisibleForTesting void setFormat(String format) { this.format = format; } - public void setVerbose(boolean verbose) { + @VisibleForTesting void setVerbose(boolean verbose) { this.verbose = verbose; } - public void setProject(MavenProject project) { + @VisibleForTesting void setProject(MavenProject project) { this.project = project; } - public void setDotGitDirectory(File dotGitDirectory) { + @VisibleForTesting void setDotGitDirectory(File dotGitDirectory) { this.dotGitDirectory = dotGitDirectory; } - public void setPrefix(String prefix) { + @VisibleForTesting void setPrefix(String prefix) { this.prefix = prefix; } - public void setDateFormat(String dateFormat) { + @VisibleForTesting void setDateFormat(String dateFormat) { this.dateFormat = dateFormat; } - public Properties getProperties() { + @VisibleForTesting Properties getProperties() { return properties; } - public void setGitDescribe(GitDescribeConfig gitDescribe) { + @VisibleForTesting void setGitDescribe(GitDescribeConfig gitDescribe) { this.gitDescribe = gitDescribe; } - public void setAbbrevLength(int abbrevLength) { + @VisibleForTesting void setAbbrevLength(int abbrevLength) { this.abbrevLength = abbrevLength; } - public void setExcludeProperties(List excludeProperties) { + @VisibleForTesting void setExcludeProperties(List excludeProperties) { this.excludeProperties = excludeProperties; } - public void setIncludeOnlyProperties(List includeOnlyProperties) { + @VisibleForTesting void setIncludeOnlyProperties(List includeOnlyProperties) { this.includeOnlyProperties = includeOnlyProperties; } - public void setUseNativeGit(boolean useNativeGit) { + @VisibleForTesting void setUseNativeGit(boolean useNativeGit) { this.useNativeGit = useNativeGit; } - public void setCommitIdGenerationMode(String commitIdGenerationMode) { + @VisibleForTesting void setCommitIdGenerationMode(String commitIdGenerationMode) { this.commitIdGenerationMode = commitIdGenerationMode; } - public void setSkip(boolean skip) { + @VisibleForTesting void setSkip(boolean skip) { this.skip = skip; } - public void setSkipPoms(boolean skipPoms) { + @VisibleForTesting void setSkipPoms(boolean skipPoms) { this.skipPoms = skipPoms; } - public void setGenerateGitPropertiesFile(boolean generateGitPropertiesFile) { + @VisibleForTesting void setGenerateGitPropertiesFile(boolean generateGitPropertiesFile) { this.generateGitPropertiesFile = generateGitPropertiesFile; } - public void setGenerateGitPropertiesFilename(String generateGitPropertiesFilename) { + @VisibleForTesting void setGenerateGitPropertiesFilename(String generateGitPropertiesFilename) { this.generateGitPropertiesFilename = generateGitPropertiesFilename; } - public void setDateFormatTimeZone(String dateFormatTimeZone) { + @VisibleForTesting void setDateFormatTimeZone(String dateFormatTimeZone) { this.dateFormatTimeZone = dateFormatTimeZone; } - public void setFailOnNoGitDirectory(boolean failOnNoGitDirectory) { + @VisibleForTesting void setFailOnNoGitDirectory(boolean failOnNoGitDirectory) { this.failOnNoGitDirectory = failOnNoGitDirectory; } - public void setFailOnUnableToExtractRepoInfo(boolean failOnUnableToExtractRepoInfo) { + @VisibleForTesting void setFailOnUnableToExtractRepoInfo(boolean failOnUnableToExtractRepoInfo) { this.failOnUnableToExtractRepoInfo = failOnUnableToExtractRepoInfo; } } From 3d12d1d3aa9aa7dbe267b431bb8b68aa4eea78ab Mon Sep 17 00:00:00 2001 From: TheSnoozer Date: Tue, 17 Oct 2017 19:13:43 -0400 Subject: [PATCH 5/5] https://github.com/ktoso/maven-git-commit-id-plugin/issues/221: ensure that git.closest.tag.name and git.closest.tag.count prefer annotated tags (like the discribe command) and also works with patterns like 'light*' --- .../pl/project13/jgit/DescribeCommand.java | 4 +- .../java/pl/project13/jgit/JGitCommon.java | 8 ++- .../git/GitCommitIdMojoIntegrationTest.java | 67 +++++++++++++++++++ 3 files changed, 75 insertions(+), 4 deletions(-) diff --git a/src/main/java/pl/project13/jgit/DescribeCommand.java b/src/main/java/pl/project13/jgit/DescribeCommand.java index c7ae76f4..4583dae6 100644 --- a/src/main/java/pl/project13/jgit/DescribeCommand.java +++ b/src/main/java/pl/project13/jgit/DescribeCommand.java @@ -353,8 +353,6 @@ private String createMatchPattern() { return ".*"; } - return "^refs/tags/\\Q" + - matchOption.get().replace("*", "\\E.*\\Q").replace("?", "\\E.\\Q") + - "\\E$"; + return jGitCommon.createMatchPattern(matchOption.get()); } } diff --git a/src/main/java/pl/project13/jgit/JGitCommon.java b/src/main/java/pl/project13/jgit/JGitCommon.java index 409006bc..fcbd9550 100644 --- a/src/main/java/pl/project13/jgit/JGitCommon.java +++ b/src/main/java/pl/project13/jgit/JGitCommon.java @@ -106,7 +106,7 @@ private Pair getClosestRevCommit(@NotNull Repository repo, Re if (gitDescribe != null) { includeLightweightTags = gitDescribe.getTags(); if (!"*".equals(gitDescribe.getMatch())) { - matchPattern = gitDescribe.getMatch(); + matchPattern = createMatchPattern(gitDescribe.getMatch()); } } Map> tagObjectIdToName = findTagObjectIds(repo, includeLightweightTags, matchPattern); @@ -121,6 +121,12 @@ private Pair getClosestRevCommit(@NotNull Repository repo, Re return Pair.of(revCommit, tagName); } + protected String createMatchPattern(String pattern) { + return "^refs/tags/\\Q" + + pattern.replace("*", "\\E.*\\Q").replace("?", "\\E.\\Q") + + "\\E$"; + } + protected Map> findTagObjectIds(@NotNull Repository repo, boolean includeLightweightTags, String matchPattern) { Map> commitIdsToTags = getCommitIdsToTags(repo, includeLightweightTags, matchPattern); Map> commitIdsToTagNames = transformRevTagsMapToDateSortedTagNames(commitIdsToTags); diff --git a/src/test/java/pl/project13/maven/git/GitCommitIdMojoIntegrationTest.java b/src/test/java/pl/project13/maven/git/GitCommitIdMojoIntegrationTest.java index a2fe0b2d..bf669de8 100644 --- a/src/test/java/pl/project13/maven/git/GitCommitIdMojoIntegrationTest.java +++ b/src/test/java/pl/project13/maven/git/GitCommitIdMojoIntegrationTest.java @@ -914,6 +914,73 @@ public void shouldGenerateClosestTagInformationWithIncludeLightweightTagsForClos assertPropertyPresentAndEqual(targetProject.getProperties(), "git.closest.tag.commit.count", "1"); } + @Test + @Parameters(method = "useNativeGit") + public void shouldGenerateClosestTagInformationWithIncludeLightweightTagsForClosestTagAndPreferAnnotatedTags(boolean useNativeGit) throws Exception { + // given + mavenSandbox + .withParentProject("my-jar-project", "jar") + .withNoChildProject() + .withGitRepoInParent(AvailableGitTestRepo.WITH_COMMIT_THAT_HAS_TWO_TAGS) + .create(); + + MavenProject targetProject = mavenSandbox.getParentProject(); + setProjectToExecuteMojoIn(targetProject); + + GitDescribeConfig gitDescribe = createGitDescribeConfig(true, 9); + gitDescribe.setDirty("-customDirtyMark"); + gitDescribe.setTags(true); // include lightweight tags + + mojo.setGitDescribe(gitDescribe); + mojo.setUseNativeGit(useNativeGit); + + // when + mojo.execute(); + + // then + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.commit.id.abbrev", "b6a73ed"); + + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.commit.id.describe", "newest-tag-1-gb6a73ed74-customDirtyMark"); + + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.closest.tag.name", "newest-tag"); + + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.closest.tag.commit.count", "1"); + } + + @Test + @Parameters(method = "useNativeGit") + public void shouldGenerateClosestTagInformationWithIncludeLightweightTagsForClosestTagAndFilter(boolean useNativeGit) throws Exception { + // given + mavenSandbox + .withParentProject("my-jar-project", "jar") + .withNoChildProject() + .withGitRepoInParent(AvailableGitTestRepo.WITH_COMMIT_THAT_HAS_TWO_TAGS) + .create(); + + MavenProject targetProject = mavenSandbox.getParentProject(); + setProjectToExecuteMojoIn(targetProject); + + GitDescribeConfig gitDescribe = createGitDescribeConfig(true, 9); + gitDescribe.setDirty("-customDirtyMark"); + gitDescribe.setTags(true); // include lightweight tags + gitDescribe.setMatch("light*"); + + mojo.setGitDescribe(gitDescribe); + mojo.setUseNativeGit(useNativeGit); + + // when + mojo.execute(); + + // then + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.commit.id.abbrev", "b6a73ed"); + + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.commit.id.describe", "lightweight-tag-1-gb6a73ed74-customDirtyMark"); + + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.closest.tag.name", "lightweight-tag"); + + assertPropertyPresentAndEqual(targetProject.getProperties(), "git.closest.tag.commit.count", "1"); + } + private GitDescribeConfig createGitDescribeConfig(boolean forceLongFormat, int abbrev) { GitDescribeConfig gitDescribeConfig = new GitDescribeConfig(); gitDescribeConfig.setTags(true);