From 1aea4653ae0fa2c526b330e6dc14e4a812b3784b Mon Sep 17 00:00:00 2001 From: Noah Rozelle Date: Sat, 16 Jul 2022 12:39:12 -0400 Subject: [PATCH 01/22] Commit up so I can work later on work computer --- pom.xml | 6 +++++ .../plugins/git/AbstractGitSCMSourceTest.java | 27 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/pom.xml b/pom.xml index 45fe6b8e5e..67a5bd1de8 100644 --- a/pom.xml +++ b/pom.xml @@ -269,6 +269,12 @@ jenkins-test-harness test + + org.jenkins-ci.plugins + git + 4.11.3 + compile + diff --git a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java index f05028284f..791b4f838e 100644 --- a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java +++ b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java @@ -1105,6 +1105,33 @@ public boolean isHead(@NonNull Probe probe, @NonNull TaskListener listener) thro sharedSampleRepo = null; } } + + @Test + public void shouldReturnExactMatchOverRelativeMatchTest() throws Exception { + // TODO: The idea is that this code should now make sure that there isn't an exact match before sending something back + sampleRepo.init(); + String masterHash = sampleRepo.head(); + sampleRepo.git("checkout", "-b", "dev"); + sampleRepo.write("file", "modified"); + sampleRepo.git("commit", "--all", "--message=dev"); + String v1Hash = sampleRepo.head(); + sampleRepo.write("file", "modified2"); + sampleRepo.git("commit", "--all", "--message=dev2"); + String v2Hash = sampleRepo.head(); + sampleRepo.write("file", "modified3"); + sampleRepo.git("commit", "--all", "--message=dev3"); + String devHash = sampleRepo.head(); + GitSCMSource source = new GitSCMSource(sampleRepo.toString()); + source.setTraits(Collections.singletonList(new BranchDiscoveryTrait())); + + TaskListener listener = StreamTaskListener.fromStderr(); + + listener.getLogger().println("\n=== fetch('master') ===\n"); + SCMRevision rev = source.fetch(masterHash, listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(rev.getHead().getName(), is("master")); + } + //Ugly but MockGitClient needs to be static and no good way to pass it on static GitSampleRepoRule sharedSampleRepo; From 6ef602d15cd04c321abf026e832c1f8b4e9b6616 Mon Sep 17 00:00:00 2001 From: Noah Rozelle Date: Mon, 18 Jul 2022 14:31:56 -0400 Subject: [PATCH 02/22] test needs some fine-tuning but should be at a good place --- pom.xml | 6 ++ .../plugins/git/AbstractGitSCMSource.java | 66 ++++++++++--------- .../plugins/git/AbstractGitSCMSourceTest.java | 59 +++++++++++++---- 3 files changed, 86 insertions(+), 45 deletions(-) diff --git a/pom.xml b/pom.xml index 67a5bd1de8..6bcd722a0d 100644 --- a/pom.xml +++ b/pom.xml @@ -275,6 +275,12 @@ 4.11.3 compile + + org.jenkins-ci.plugins + git + 4.11.3 + compile + diff --git a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java index a9e51aa5d3..53c1ad515e 100644 --- a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java +++ b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java @@ -922,16 +922,23 @@ protected SCMRevision retrieve(@NonNull final String revision, @NonNull final Ta } if (rev.toLowerCase(Locale.ENGLISH).startsWith(revision.toLowerCase(Locale.ENGLISH))) { shortNameMatches.add(name); + listener.getLogger().printf("Candidate partial match: %s revision %s%n", name, rev); if (shortHashMatch == null) { - listener.getLogger().printf("Candidate partial match: %s revision %s%n", name, rev); shortHashMatch = rev; } else { - listener.getLogger().printf("Candidate partial match: %s revision %s%n", name, rev); listener.getLogger().printf("Cannot resolve ambiguous short revision %s%n", revision); - return null; + if (fullTagMatches.isEmpty() && fullHashMatches.isEmpty() && fullHashMatch == null) { + // We haven't found any matches, and we have ambiguous matches, cannot determine + return null; + } } } } + if (fullHashMatch != null) { + // JENKINS-62592, for predictability, if an exact match is found, that should be returned + //since this would have been skipped if this was a head or a tag we can just return whatever + return new GitRefSCMRevision(new GitRefSCMHead(fullHashMatch, fullHashMatches.iterator().next()), fullHashMatch); + } if (!fullTagMatches.isEmpty()) { // we just want a tag so we can do a minimal fetch String name = StringUtils.removeStart(fullTagMatches.iterator().next(), Constants.R_TAGS); @@ -941,34 +948,6 @@ protected SCMRevision retrieve(@NonNull final String revision, @NonNull final Ta context.wantTags(true); context.withoutRefSpecs(); } - if (fullHashMatch != null) { - //since this would have been skipped if this was a head or a tag we can just return whatever - return new GitRefSCMRevision(new GitRefSCMHead(fullHashMatch, fullHashMatches.iterator().next()), fullHashMatch); - } - if (shortHashMatch != null) { - // woot this seems unambiguous - for (String name: shortNameMatches) { - if (name.startsWith(Constants.R_HEADS)) { - listener.getLogger().printf("Selected match: %s revision %s%n", name, shortHashMatch); - // WIN it's also a branch - return new GitBranchSCMRevision(new GitBranchSCMHead(StringUtils.removeStart(name, Constants.R_HEADS)), - shortHashMatch); - } else if (name.startsWith(Constants.R_TAGS)) { - tagName = StringUtils.removeStart(name, Constants.R_TAGS); - context.wantBranches(false); - context.wantTags(true); - context.withoutRefSpecs(); - } - } - if (tagName != null) { - listener.getLogger().printf("Selected match: %s revision %s%n", tagName, shortHashMatch); - } else { - return new GitRefSCMRevision(new GitRefSCMHead(shortHashMatch, shortNameMatches.iterator().next()), shortHashMatch); - } - } - if (candidateOtherRef != null) { - return candidateOtherRef; - } //if PruneStaleBranches it should take affect on the following retrievals boolean pruneRefs = context.pruneRefs(); if (tagName != null) { @@ -995,6 +974,30 @@ public SCMRevision run(GitClient client, String remoteName) throws IOException, context, listener, pruneRefs, retrieveContext); } + if (shortHashMatch != null) { + // woot this seems unambiguous + for (String name: shortNameMatches) { + if (name.startsWith(Constants.R_HEADS)) { + listener.getLogger().printf("Selected match: %s revision %s%n", name, shortHashMatch); + // WIN it's also a branch + return new GitBranchSCMRevision(new GitBranchSCMHead(StringUtils.removeStart(name, Constants.R_HEADS)), + shortHashMatch); + } else if (name.startsWith(Constants.R_TAGS)) { + tagName = StringUtils.removeStart(name, Constants.R_TAGS); + context.wantBranches(false); + context.wantTags(true); + context.withoutRefSpecs(); + } + } + if (tagName != null) { + listener.getLogger().printf("Selected match: %s revision %s%n", tagName, shortHashMatch); + } else { + return new GitRefSCMRevision(new GitRefSCMHead(shortHashMatch, shortNameMatches.iterator().next()), shortHashMatch); + } + } + if (candidateOtherRef != null) { + return candidateOtherRef; + } // Pokémon!... Got to catch them all listener.getLogger().printf("Could not find %s in remote references. " + "Pulling heads to local for deep search...%n", revision); @@ -1012,7 +1015,6 @@ public SCMRevision run(GitClient client, String remoteName) throws IOException, //just to be safe listener.error("Could not resolve %s", revision); return null; - } hash = objectId.name(); String candidatePrefix = Constants.R_REMOTES.substring(Constants.R_REFS.length()) diff --git a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java index 791b4f838e..dd93f6f4de 100644 --- a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java +++ b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java @@ -12,10 +12,7 @@ import hudson.EnvVars; import hudson.FilePath; import hudson.Launcher; -import hudson.model.Action; -import hudson.model.Actionable; -import hudson.model.Run; -import hudson.model.TaskListener; +import hudson.model.*; import hudson.plugins.git.GitException; import hudson.plugins.git.UserRemoteConfig; import hudson.plugins.git.extensions.impl.IgnoreNotifyCommit; @@ -35,11 +32,8 @@ import java.util.Set; import java.util.TreeMap; import java.util.UUID; -import jenkins.plugins.git.traits.BranchDiscoveryTrait; -import jenkins.plugins.git.traits.DiscoverOtherRefsTrait; -import jenkins.plugins.git.traits.IgnoreOnPushNotificationTrait; -import jenkins.plugins.git.traits.PruneStaleBranchTrait; -import jenkins.plugins.git.traits.TagDiscoveryTrait; + +import jenkins.plugins.git.traits.*; import jenkins.scm.api.SCMHead; import jenkins.scm.api.SCMHeadObserver; @@ -1106,30 +1100,69 @@ public boolean isHead(@NonNull Probe probe, @NonNull TaskListener listener) thro } } + @Issue("JENKINS-62592") @Test - public void shouldReturnExactMatchOverRelativeMatchTest() throws Exception { - // TODO: The idea is that this code should now make sure that there isn't an exact match before sending something back + public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception { sampleRepo.init(); String masterHash = sampleRepo.head(); sampleRepo.git("checkout", "-b", "dev"); sampleRepo.write("file", "modified"); sampleRepo.git("commit", "--all", "--message=dev"); + sampleRepo.git("tag", "v1"); String v1Hash = sampleRepo.head(); sampleRepo.write("file", "modified2"); sampleRepo.git("commit", "--all", "--message=dev2"); + sampleRepo.git("tag", "-a", "v2", "-m", "annotated"); String v2Hash = sampleRepo.head(); sampleRepo.write("file", "modified3"); sampleRepo.git("commit", "--all", "--message=dev3"); + // Grab the devHash, but lets try and generate a hash that we know will cause an issue in our test + ArrayList hashFirstLetter = new ArrayList<>(Arrays.asList(masterHash.substring(0,1), v1Hash.substring(0,1), v2Hash.substring(0,1))); String devHash = sampleRepo.head(); + int i = 4; // In order to name new files and create new commits + String newHash = null; + while (!hashFirstLetter.contains(devHash.substring(0,1))) { + // Generate a new commit and try again + sampleRepo.write("file", "modified" + i); + sampleRepo.git("commit", "--all", "--message=dev" + (i++)); + newHash = sampleRepo.head(); + hashFirstLetter.add(newHash.substring(0,1)); + } GitSCMSource source = new GitSCMSource(sampleRepo.toString()); - source.setTraits(Collections.singletonList(new BranchDiscoveryTrait())); + source.setTraits(new ArrayList<>()); TaskListener listener = StreamTaskListener.fromStderr(); listener.getLogger().println("\n=== fetch('master') ===\n"); - SCMRevision rev = source.fetch(masterHash, listener, null); + SCMRevision rev = source.fetch("master", listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(masterHash)); + listener.getLogger().println("\n=== fetch('dev') ===\n"); + rev = source.fetch("dev", listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(devHash)); + listener.getLogger().println("\n=== fetch('v1') ===\n"); + rev = source.fetch("v1", listener, null); + assertThat(rev, instanceOf(GitTagSCMRevision.class)); + assertThat(((GitTagSCMRevision)rev).getHash(), is(v1Hash)); + listener.getLogger().println("\n=== fetch('v2') ===\n"); + rev = source.fetch("v2", listener, null); + assertThat(rev, instanceOf(GitTagSCMRevision.class)); + assertThat(((GitTagSCMRevision)rev).getHash(), is(v2Hash)); + + listener.getLogger().printf("%n=== fetch('%s') ===%n%n", masterHash); + rev = source.fetch(masterHash, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(masterHash)); assertThat(rev.getHead().getName(), is("master")); + + if (null != newHash) { + listener.getLogger().printf("%n=== fetch('%s') ===%n%n", newHash); + rev = source.fetch(newHash, listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(newHash)); + assertThat(rev.getHead().getName(), is("master")); + } } //Ugly but MockGitClient needs to be static and no good way to pass it on From d3a8e5ecb0234ad9ad06a6bf2009e89f4a1793ef Mon Sep 17 00:00:00 2001 From: Noah Rozelle Date: Mon, 18 Jul 2022 16:34:05 -0400 Subject: [PATCH 03/22] tests are showing our problem now, need to ensure we are doing all we need for this feature --- .../plugins/git/AbstractGitSCMSourceTest.java | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java index dd93f6f4de..2cea8499f9 100644 --- a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java +++ b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java @@ -1118,29 +1118,28 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception sampleRepo.git("commit", "--all", "--message=dev3"); // Grab the devHash, but lets try and generate a hash that we know will cause an issue in our test ArrayList hashFirstLetter = new ArrayList<>(Arrays.asList(masterHash.substring(0,1), v1Hash.substring(0,1), v2Hash.substring(0,1))); - String devHash = sampleRepo.head(); + sampleRepo.git("tag", "devTag"); + String devTagHash = sampleRepo.head(); int i = 4; // In order to name new files and create new commits String newHash = null; - while (!hashFirstLetter.contains(devHash.substring(0,1))) { + while (!hashFirstLetter.contains(devTagHash.substring(0,1))) { // Generate a new commit and try again - sampleRepo.write("file", "modified" + i); - sampleRepo.git("commit", "--all", "--message=dev" + (i++)); newHash = sampleRepo.head(); hashFirstLetter.add(newHash.substring(0,1)); + sampleRepo.git("tag", "devTag"+i); + sampleRepo.write("file", "modified" + i); + sampleRepo.git("commit", "--all", "--message=dev" + (i++)); } GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(new ArrayList<>()); TaskListener listener = StreamTaskListener.fromStderr(); + listener.getLogger().printf("ArrayList of first hash chars: %s", hashFirstLetter); listener.getLogger().println("\n=== fetch('master') ===\n"); SCMRevision rev = source.fetch("master", listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(masterHash)); - listener.getLogger().println("\n=== fetch('dev') ===\n"); - rev = source.fetch("dev", listener, null); - assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(devHash)); listener.getLogger().println("\n=== fetch('v1') ===\n"); rev = source.fetch("v1", listener, null); assertThat(rev, instanceOf(GitTagSCMRevision.class)); @@ -1161,7 +1160,13 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception rev = source.fetch(newHash, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(newHash)); - assertThat(rev.getHead().getName(), is("master")); + assertThat(rev.getHead().getName(), is(newHash)); + + listener.getLogger().printf("%n=== fetch(devTag%d) ===%n%n", i - 1); + rev = source.fetch("devTag" + (i-1), listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(newHash)); + assertThat(rev.getHead().getName(), is("devTag" + (i - 1))); } } From 9c119661386ed9b289c2fe2cdffeea13efc37d94 Mon Sep 17 00:00:00 2001 From: Noah Rozelle Date: Tue, 19 Jul 2022 09:12:44 -0400 Subject: [PATCH 04/22] tests breaking, but looking into what issue could be, think issue has to do with my dynamic test generator (not grabbing the right tag head) --- .../plugins/git/AbstractGitSCMSource.java | 10 +++++++-- .../plugins/git/AbstractGitSCMSourceTest.java | 21 +++++++++++++++---- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java index 53c1ad515e..b5d8a17218 100644 --- a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java +++ b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java @@ -867,6 +867,7 @@ protected SCMRevision retrieve(@NonNull final String revision, @NonNull final Ta Set fullHashMatches = new TreeSet<>(); String fullHashMatch = null; GitRefSCMRevision candidateOtherRef = null; + Boolean shortRevisionAmbiguity = false; for (Map.Entry entry: remoteReferences.entrySet()) { String name = entry.getKey(); String rev = entry.getValue().name(); @@ -927,9 +928,10 @@ protected SCMRevision retrieve(@NonNull final String revision, @NonNull final Ta shortHashMatch = rev; } else { listener.getLogger().printf("Cannot resolve ambiguous short revision %s%n", revision); - if (fullTagMatches.isEmpty() && fullHashMatches.isEmpty() && fullHashMatch == null) { + if (fullTagMatches.isEmpty() && fullHashMatches.isEmpty() && fullHashMatch == null && candidateOtherRef == null) { // We haven't found any matches, and we have ambiguous matches, cannot determine - return null; + // TODO we gotta make a variable that determines if we have gone throught he entire list before returning null + shortRevisionAmbiguity = true; } } } @@ -998,6 +1000,10 @@ public SCMRevision run(GitClient client, String remoteName) throws IOException, if (candidateOtherRef != null) { return candidateOtherRef; } + if (shortRevisionAmbiguity) { + // Check if we have any other matches first, if not, return null as we cannot determine a match + return null; + } // Pokémon!... Got to catch them all listener.getLogger().printf("Could not find %s in remote references. " + "Pulling heads to local for deep search...%n", revision); diff --git a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java index 2cea8499f9..55f499ac4d 100644 --- a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java +++ b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java @@ -1121,14 +1121,14 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception sampleRepo.git("tag", "devTag"); String devTagHash = sampleRepo.head(); int i = 4; // In order to name new files and create new commits - String newHash = null; + String newHash = devTagHash; while (!hashFirstLetter.contains(devTagHash.substring(0,1))) { // Generate a new commit and try again - newHash = sampleRepo.head(); - hashFirstLetter.add(newHash.substring(0,1)); sampleRepo.git("tag", "devTag"+i); sampleRepo.write("file", "modified" + i); sampleRepo.git("commit", "--all", "--message=dev" + (i++)); + newHash = sampleRepo.head(); + hashFirstLetter.add(newHash.substring(0,1)); } GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(new ArrayList<>()); @@ -1160,13 +1160,26 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception rev = source.fetch(newHash, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(newHash)); - assertThat(rev.getHead().getName(), is(newHash)); + assertThat(rev.getHead().getName(), is("dev")); + + listener.getLogger().printf("%n=== fetch('%s') short hash ===%n%n", newHash.substring(0, 6)); + rev = source.fetch(newHash.substring(0, 6), listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(newHash)); + assertThat(rev.getHead().getName(), is("dev")); listener.getLogger().printf("%n=== fetch(devTag%d) ===%n%n", i - 1); rev = source.fetch("devTag" + (i-1), listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(newHash)); assertThat(rev.getHead().getName(), is("devTag" + (i - 1))); + + String ambiguousTag = hashFirstLetter.get(hashFirstLetter.size() - 1); + listener.getLogger().printf("%n=== fetch(%s) ===%n%n", ambiguousTag); + rev = source.fetch(ambiguousTag, listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(devTagHash)); + assertThat(rev.getHead().getName(), is("devTag" + (i - 1))); } } From a392480df2db20aa858fe64d795d0f6181253059 Mon Sep 17 00:00:00 2001 From: Noah Rozelle Date: Tue, 19 Jul 2022 09:48:32 -0400 Subject: [PATCH 05/22] yup, head wasn't correct, acting appropriately now, tests are passing with dynamic test creation --- .../plugins/git/AbstractGitSCMSourceTest.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java index 55f499ac4d..b5d9f51751 100644 --- a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java +++ b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java @@ -1121,12 +1121,14 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception sampleRepo.git("tag", "devTag"); String devTagHash = sampleRepo.head(); int i = 4; // In order to name new files and create new commits + String previousNewHash = null; String newHash = devTagHash; while (!hashFirstLetter.contains(devTagHash.substring(0,1))) { // Generate a new commit and try again sampleRepo.git("tag", "devTag"+i); sampleRepo.write("file", "modified" + i); sampleRepo.git("commit", "--all", "--message=dev" + (i++)); + previousNewHash = newHash; newHash = sampleRepo.head(); hashFirstLetter.add(newHash.substring(0,1)); } @@ -1155,7 +1157,7 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(masterHash)); assertThat(rev.getHead().getName(), is("master")); - if (null != newHash) { + if (!devTagHash.equals(newHash)) { listener.getLogger().printf("%n=== fetch('%s') ===%n%n", newHash); rev = source.fetch(newHash, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); @@ -1168,18 +1170,21 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(newHash)); assertThat(rev.getHead().getName(), is("dev")); - listener.getLogger().printf("%n=== fetch(devTag%d) ===%n%n", i - 1); - rev = source.fetch("devTag" + (i-1), listener, null); + + int lastDevTag = i == 4 ? 4 : i - 1; + String headHash = i == 4 ? devTagHash : previousNewHash; + listener.getLogger().printf("%n=== fetch(devTag%d) ===%n%n", lastDevTag); + rev = source.fetch("devTag" + lastDevTag, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(newHash)); - assertThat(rev.getHead().getName(), is("devTag" + (i - 1))); + assertThat(rev.getHead().getName(), is("devTag" + lastDevTag)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(headHash)); String ambiguousTag = hashFirstLetter.get(hashFirstLetter.size() - 1); listener.getLogger().printf("%n=== fetch(%s) ===%n%n", ambiguousTag); rev = source.fetch(ambiguousTag, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(rev.getHead().getName(), is("dev")); assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(devTagHash)); - assertThat(rev.getHead().getName(), is("devTag" + (i - 1))); } } From bb238bf71d1f125cf9e3056fedce92fd791e7563 Mon Sep 17 00:00:00 2001 From: Noah Rozelle Date: Tue, 19 Jul 2022 10:36:41 -0400 Subject: [PATCH 06/22] JENKINS-62592 cleaning up test which was impacted by change, fixed now! --- src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java index b5d8a17218..45bb554b6b 100644 --- a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java +++ b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java @@ -921,7 +921,7 @@ protected SCMRevision retrieve(@NonNull final String revision, @NonNull final Ta break; } } - if (rev.toLowerCase(Locale.ENGLISH).startsWith(revision.toLowerCase(Locale.ENGLISH))) { + if (!revision.isEmpty() && rev.toLowerCase(Locale.ENGLISH).startsWith(revision.toLowerCase(Locale.ENGLISH))) { shortNameMatches.add(name); listener.getLogger().printf("Candidate partial match: %s revision %s%n", name, rev); if (shortHashMatch == null) { From 77d3abbc6ebe7f150d4d6541eeea7d1a2e9fb06b Mon Sep 17 00:00:00 2001 From: Noah Rozelle Date: Tue, 19 Jul 2022 13:18:10 -0400 Subject: [PATCH 07/22] JENKINS-62592 reformat, renamed some bad test variable names --- .../plugins/git/AbstractGitSCMSourceTest.java | 109 +++++++++--------- 1 file changed, 56 insertions(+), 53 deletions(-) diff --git a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java index b5d9f51751..60f8330279 100644 --- a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java +++ b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java @@ -22,6 +22,7 @@ import hudson.plugins.git.extensions.impl.BuildChooserSetting; import hudson.plugins.git.extensions.impl.LocalBranch; import hudson.util.StreamTaskListener; + import java.io.File; import java.io.IOException; import java.util.ArrayList; @@ -202,18 +203,16 @@ public void retrieveHeadsSupportsTagDiscovery_findTagsWithTagDiscoveryTrait() th long fileTimeStampFuzz = isWindows() ? 2000L : 1000L; fileTimeStampFuzz = 12 * fileTimeStampFuzz / 10; // 20% grace for file system noise switch (scmHead.getName()) { - case "lightweight": - { - long timeStampDelta = afterLightweightTag - tagHead.getTimestamp(); - assertThat(timeStampDelta, is(both(greaterThanOrEqualTo(0L)).and(lessThanOrEqualTo(afterLightweightTag - beforeLightweightTag + fileTimeStampFuzz)))); - break; - } - case "annotated": - { - long timeStampDelta = afterAnnotatedTag - tagHead.getTimestamp(); - assertThat(timeStampDelta, is(both(greaterThanOrEqualTo(0L)).and(lessThanOrEqualTo(afterAnnotatedTag - beforeAnnotatedTag + fileTimeStampFuzz)))); - break; - } + case "lightweight": { + long timeStampDelta = afterLightweightTag - tagHead.getTimestamp(); + assertThat(timeStampDelta, is(both(greaterThanOrEqualTo(0L)).and(lessThanOrEqualTo(afterLightweightTag - beforeLightweightTag + fileTimeStampFuzz)))); + break; + } + case "annotated": { + long timeStampDelta = afterAnnotatedTag - tagHead.getTimestamp(); + assertThat(timeStampDelta, is(both(greaterThanOrEqualTo(0L)).and(lessThanOrEqualTo(afterAnnotatedTag - beforeAnnotatedTag + fileTimeStampFuzz)))); + break; + } default: fail("Unexpected tag head '" + scmHead.getName() + "'"); break; @@ -305,7 +304,7 @@ public void retrieveTags_folderScopedCredentials() throws Exception { assert folderStore != null; String fCredentialsId = "fcreds"; StandardCredentials fCredentials = new UsernamePasswordCredentialsImpl(CredentialsScope.GLOBAL, - fCredentialsId, "fcreds", "user", "password"); + fCredentialsId, "fcreds", "user", "password"); folderStore.addCredentials(Domain.global(), fCredentials); folderStore.save(); WorkflowJob p = f.createProject(WorkflowJob.class, "wjob"); @@ -361,19 +360,19 @@ public void retrieveByName() throws Exception { listener.getLogger().println("\n=== fetch('master') ===\n"); SCMRevision rev = source.fetch("master", listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(masterHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(masterHash)); listener.getLogger().println("\n=== fetch('dev') ===\n"); rev = source.fetch("dev", listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(devHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(devHash)); listener.getLogger().println("\n=== fetch('v1') ===\n"); rev = source.fetch("v1", listener, null); assertThat(rev, instanceOf(GitTagSCMRevision.class)); - assertThat(((GitTagSCMRevision)rev).getHash(), is(v1Hash)); + assertThat(((GitTagSCMRevision) rev).getHash(), is(v1Hash)); listener.getLogger().println("\n=== fetch('v2') ===\n"); rev = source.fetch("v2", listener, null); assertThat(rev, instanceOf(GitTagSCMRevision.class)); - assertThat(((GitTagSCMRevision)rev).getHash(), is(v2Hash)); + assertThat(((GitTagSCMRevision) rev).getHash(), is(v2Hash)); listener.getLogger().printf("%n=== fetch('%s') ===%n%n", masterHash); rev = source.fetch(masterHash, listener, null); @@ -471,7 +470,7 @@ private void retrievePrimaryHead(boolean duplicatePrimary) throws Exception { source.setOwner(owner); TaskListener listener = StreamTaskListener.fromStderr(); Map headByName = new TreeMap<>(); - for (SCMHead h: source.fetch(listener)) { + for (SCMHead h : source.fetch(listener)) { headByName.put(h.getName(), h); } if (duplicatePrimary) { @@ -481,7 +480,7 @@ private void retrievePrimaryHead(boolean duplicatePrimary) throws Exception { } List actions = source.fetchActions(null, listener); GitRemoteHeadRefAction refAction = null; - for (Action a: actions) { + for (Action a : actions) { if (a instanceof GitRemoteHeadRefAction) { refAction = (GitRemoteHeadRefAction) a; break; @@ -499,7 +498,7 @@ private void retrievePrimaryHead(boolean duplicatePrimary) throws Exception { } PrimaryInstanceMetadataAction primary = null; - for (Action a: actions) { + for (Action a : actions) { if (a instanceof PrimaryInstanceMetadataAction) { primary = (PrimaryInstanceMetadataAction) a; break; @@ -526,7 +525,7 @@ public void retrieveRevision() throws Exception { sampleRepo.write("file", "v3"); sampleRepo.git("commit", "--all", "--message=v3"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList(new BranchDiscoveryTrait(), new TagDiscoveryTrait())); StreamTaskListener listener = StreamTaskListener.fromStderr(); @@ -564,7 +563,7 @@ public void retrieveRevision_nonHead() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList(new BranchDiscoveryTrait(), new TagDiscoveryTrait())); StreamTaskListener listener = StreamTaskListener.fromStderr(); @@ -591,7 +590,7 @@ public void retrieveRevision_nonAdvertised() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList(new BranchDiscoveryTrait(), new TagDiscoveryTrait())); StreamTaskListener listener = StreamTaskListener.fromStderr(); @@ -620,7 +619,7 @@ public void retrieveRevision_customRef() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList( new BranchDiscoveryTrait(), @@ -655,7 +654,7 @@ public void retrieveRevision_customRef_descendant() throws Exception { sampleRepo.write("file", "v5"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList( new BranchDiscoveryTrait(), @@ -685,7 +684,7 @@ public void retrieveRevision_customRef_abbrev_sha1() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList( new BranchDiscoveryTrait(), @@ -715,7 +714,7 @@ public void retrieveRevision_pr_refspec() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList(new BranchDiscoveryTrait(), new TagDiscoveryTrait(), new DiscoverOtherRefsTrait("pull-requests/*/from"))); StreamTaskListener listener = StreamTaskListener.fromStderr(); @@ -742,7 +741,7 @@ public void retrieveRevision_pr_local_refspec() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); //new RefSpecsSCMSourceTrait("+refs/pull-requests/*/from:refs/remotes/@{remote}/pr/*") source.setTraits(Arrays.asList(new BranchDiscoveryTrait(), new TagDiscoveryTrait(), @@ -753,7 +752,8 @@ public void retrieveRevision_pr_local_refspec() throws Exception { } private int wsCount; - private String fileAt(String revision, Run run, SCMSource source, TaskListener listener) throws Exception { + + private String fileAt(String revision, Run run, SCMSource source, TaskListener listener) throws Exception { SCMRevision rev = source.fetch(revision, listener, null); if (rev == null) { return null; @@ -783,18 +783,18 @@ public void fetchOtherRef() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList(new BranchDiscoveryTrait(), new TagDiscoveryTrait(), new DiscoverOtherRefsTrait("custom/*"))); StreamTaskListener listener = StreamTaskListener.fromStderr(); final SCMHeadObserver.Collector collector = - source.fetch(new SCMSourceCriteria() { - @Override - public boolean isHead(@NonNull Probe probe, @NonNull TaskListener listener) throws IOException { - return true; - } - }, new SCMHeadObserver.Collector(), listener); + source.fetch(new SCMSourceCriteria() { + @Override + public boolean isHead(@NonNull Probe probe, @NonNull TaskListener listener) throws IOException { + return true; + } + }, new SCMHeadObserver.Collector(), listener); final Map result = collector.result(); assertThat(result.entrySet(), hasSize(4)); @@ -823,7 +823,7 @@ public void fetchOtherRevisions() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList(new BranchDiscoveryTrait(), new TagDiscoveryTrait(), new DiscoverOtherRefsTrait("custom/*"))); StreamTaskListener listener = StreamTaskListener.fromStderr(); @@ -982,7 +982,7 @@ public void refLockEncounteredIfPruneTraitNotPresentOnNotFoundRetrieval() throws try { source.fetch("v1.2", listener, null); - } catch (GitException e){ + } catch (GitException e) { assertFalse(e.getMessage().contains("--prune")); return; } @@ -1000,7 +1000,7 @@ public void refLockEncounteredIfPruneTraitNotPresentOnTagRetrieval() throws Exce try { source.fetch("v1.2", listener, null); - } catch (GitException e){ + } catch (GitException e) { assertFalse(e.getMessage().contains("--prune")); return; } @@ -1069,7 +1069,8 @@ private void createRefLockEnvironment(TaskListener listener, GitSCMSource source sampleRepo.git("push", source.getRemote(), "v1.2"); } - @Test @Issue("JENKINS-50394") + @Test + @Issue("JENKINS-50394") public void when_commits_added_during_discovery_we_do_not_crash() throws Exception { sampleRepo.init(); sampleRepo.git("checkout", "-b", "dev"); @@ -1092,7 +1093,7 @@ public boolean isHead(@NonNull Probe probe, @NonNull TaskListener listener) thro hasProperty("name", equalTo("master")), hasProperty("name", equalTo("dev")) )); - } catch(MissingObjectException me) { + } catch (MissingObjectException me) { fail("Not supposed to get MissingObjectException"); } finally { System.clearProperty(Git.class.getName() + ".mockClient"); @@ -1117,20 +1118,22 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception sampleRepo.write("file", "modified3"); sampleRepo.git("commit", "--all", "--message=dev3"); // Grab the devHash, but lets try and generate a hash that we know will cause an issue in our test - ArrayList hashFirstLetter = new ArrayList<>(Arrays.asList(masterHash.substring(0,1), v1Hash.substring(0,1), v2Hash.substring(0,1))); + ArrayList hashFirstLetter = new ArrayList<>( + Arrays.asList(masterHash.substring(0, 1), v1Hash.substring(0, 1), v2Hash.substring(0, 1)) + ); sampleRepo.git("tag", "devTag"); String devTagHash = sampleRepo.head(); - int i = 4; // In order to name new files and create new commits + int devTagIteration = 4; // In order to name new files and create new commits String previousNewHash = null; String newHash = devTagHash; - while (!hashFirstLetter.contains(devTagHash.substring(0,1))) { + while (!hashFirstLetter.contains(devTagHash.substring(0, 1))) { // Generate a new commit and try again - sampleRepo.git("tag", "devTag"+i); - sampleRepo.write("file", "modified" + i); - sampleRepo.git("commit", "--all", "--message=dev" + (i++)); + sampleRepo.git("tag", "devTag" + devTagIteration); + sampleRepo.write("file", "modified" + devTagIteration); + sampleRepo.git("commit", "--all", "--message=dev" + (devTagIteration++)); previousNewHash = newHash; newHash = sampleRepo.head(); - hashFirstLetter.add(newHash.substring(0,1)); + hashFirstLetter.add(newHash.substring(0, 1)); } GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(new ArrayList<>()); @@ -1141,15 +1144,15 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception listener.getLogger().println("\n=== fetch('master') ===\n"); SCMRevision rev = source.fetch("master", listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(masterHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(masterHash)); listener.getLogger().println("\n=== fetch('v1') ===\n"); rev = source.fetch("v1", listener, null); assertThat(rev, instanceOf(GitTagSCMRevision.class)); - assertThat(((GitTagSCMRevision)rev).getHash(), is(v1Hash)); + assertThat(((GitTagSCMRevision) rev).getHash(), is(v1Hash)); listener.getLogger().println("\n=== fetch('v2') ===\n"); rev = source.fetch("v2", listener, null); assertThat(rev, instanceOf(GitTagSCMRevision.class)); - assertThat(((GitTagSCMRevision)rev).getHash(), is(v2Hash)); + assertThat(((GitTagSCMRevision) rev).getHash(), is(v2Hash)); listener.getLogger().printf("%n=== fetch('%s') ===%n%n", masterHash); rev = source.fetch(masterHash, listener, null); @@ -1171,8 +1174,8 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception assertThat(rev.getHead().getName(), is("dev")); - int lastDevTag = i == 4 ? 4 : i - 1; - String headHash = i == 4 ? devTagHash : previousNewHash; + int lastDevTag = devTagIteration == 4 ? 4 : devTagIteration - 1; + String headHash = devTagIteration == 4 ? devTagHash : previousNewHash; listener.getLogger().printf("%n=== fetch(devTag%d) ===%n%n", lastDevTag); rev = source.fetch("devTag" + lastDevTag, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); From eb77163b2cf34b648e6dc2f8471a14f7c7b6fe4f Mon Sep 17 00:00:00 2001 From: Noah Rozelle Date: Tue, 19 Jul 2022 13:40:18 -0400 Subject: [PATCH 08/22] JENKINS-62592 adding a more "to-the-point" test regarding the issue we have seen --- .../jenkins/plugins/git/AbstractGitSCMSourceTest.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java index 60f8330279..465cbffaf7 100644 --- a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java +++ b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java @@ -1141,6 +1141,7 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception TaskListener listener = StreamTaskListener.fromStderr(); listener.getLogger().printf("ArrayList of first hash chars: %s", hashFirstLetter); + // Test existing functionality with additional revisions listener.getLogger().println("\n=== fetch('master') ===\n"); SCMRevision rev = source.fetch("master", listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); @@ -1160,6 +1161,7 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(masterHash)); assertThat(rev.getHead().getName(), is("master")); + // Test new functionality and verify that we are able to grab what we expect (full match versus hazy short hash) if (!devTagHash.equals(newHash)) { listener.getLogger().printf("%n=== fetch('%s') ===%n%n", newHash); rev = source.fetch(newHash, listener, null); @@ -1183,11 +1185,15 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(headHash)); String ambiguousTag = hashFirstLetter.get(hashFirstLetter.size() - 1); + String ambiguousHash = sampleRepo.head(); + sampleRepo.git("tag", ambiguousTag); + sampleRepo.write("file", "modified and ambiguous"); + sampleRepo.git("commit", "--all", "--message=ambiguousTagCommit"); listener.getLogger().printf("%n=== fetch(%s) ===%n%n", ambiguousTag); rev = source.fetch(ambiguousTag, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(rev.getHead().getName(), is("dev")); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(devTagHash)); + assertThat(rev.getHead().getName(), is(ambiguousTag)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(ambiguousHash)); } } From b22477616beaf2c8a4895f4e1fcdc5955bece5a1 Mon Sep 17 00:00:00 2001 From: Noah Rozelle Date: Thu, 21 Jul 2022 11:18:39 -0400 Subject: [PATCH 09/22] JENKINS-62592 adding a test to show branch will resolve first over hazy short hash --- .../jenkins/plugins/git/AbstractGitSCMSource.java | 1 - .../plugins/git/AbstractGitSCMSourceTest.java | 12 ++++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java index 45bb554b6b..c66680f416 100644 --- a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java +++ b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java @@ -930,7 +930,6 @@ protected SCMRevision retrieve(@NonNull final String revision, @NonNull final Ta listener.getLogger().printf("Cannot resolve ambiguous short revision %s%n", revision); if (fullTagMatches.isEmpty() && fullHashMatches.isEmpty() && fullHashMatch == null && candidateOtherRef == null) { // We haven't found any matches, and we have ambiguous matches, cannot determine - // TODO we gotta make a variable that determines if we have gone throught he entire list before returning null shortRevisionAmbiguity = true; } } diff --git a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java index 465cbffaf7..fad2f612d0 100644 --- a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java +++ b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java @@ -1194,6 +1194,18 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); assertThat(rev.getHead().getName(), is(ambiguousTag)); assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(ambiguousHash)); + + // Remove ambiguous tag from the repo and thne create a branch with the ambiguous search + sampleRepo.git("tag", "-d", ambiguousTag); + sampleRepo.git("checkout", "-b", ambiguousTag); + sampleRepo.write("file", "modified and ambiguous but a branch this time!"); + sampleRepo.git("commit", "--all", "--message=ambiguousBranchCommit"); + listener.getLogger().printf("%n=== fetch(%s) branch ===%n%n", ambiguousTag); + String ambiguousBranchHash = sampleRepo.head(); + rev = source.fetch(ambiguousTag, listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(rev.getHead().getName(), is(ambiguousTag)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(ambiguousBranchHash)); } } From 858c9177169018ca2bedc3baea7e4e529b1b6b96 Mon Sep 17 00:00:00 2001 From: Noah Rozelle Date: Sat, 16 Jul 2022 12:39:12 -0400 Subject: [PATCH 10/22] Commit up so I can work later on work computer --- pom.xml | 6 +++++ .../plugins/git/AbstractGitSCMSourceTest.java | 27 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/pom.xml b/pom.xml index 4ed778131c..e47acb582e 100644 --- a/pom.xml +++ b/pom.xml @@ -248,6 +248,12 @@ jenkins-test-harness test + + org.jenkins-ci.plugins + git + 4.11.3 + compile + diff --git a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java index 054713ce07..a56bd27d07 100644 --- a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java +++ b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java @@ -1081,6 +1081,33 @@ public void when_commits_added_during_discovery_we_do_not_crash() throws Excepti sharedSampleRepo = null; } } + + @Test + public void shouldReturnExactMatchOverRelativeMatchTest() throws Exception { + // TODO: The idea is that this code should now make sure that there isn't an exact match before sending something back + sampleRepo.init(); + String masterHash = sampleRepo.head(); + sampleRepo.git("checkout", "-b", "dev"); + sampleRepo.write("file", "modified"); + sampleRepo.git("commit", "--all", "--message=dev"); + String v1Hash = sampleRepo.head(); + sampleRepo.write("file", "modified2"); + sampleRepo.git("commit", "--all", "--message=dev2"); + String v2Hash = sampleRepo.head(); + sampleRepo.write("file", "modified3"); + sampleRepo.git("commit", "--all", "--message=dev3"); + String devHash = sampleRepo.head(); + GitSCMSource source = new GitSCMSource(sampleRepo.toString()); + source.setTraits(Collections.singletonList(new BranchDiscoveryTrait())); + + TaskListener listener = StreamTaskListener.fromStderr(); + + listener.getLogger().println("\n=== fetch('master') ===\n"); + SCMRevision rev = source.fetch(masterHash, listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(rev.getHead().getName(), is("master")); + } + //Ugly but MockGitClient needs to be static and no good way to pass it on static GitSampleRepoRule sharedSampleRepo; From 726e4400057ed6c2ed20af1c18f37dbf3fdde217 Mon Sep 17 00:00:00 2001 From: Noah Rozelle Date: Mon, 18 Jul 2022 14:31:56 -0400 Subject: [PATCH 11/22] test needs some fine-tuning but should be at a good place --- pom.xml | 6 ++ .../plugins/git/AbstractGitSCMSource.java | 66 ++++++++++--------- .../plugins/git/AbstractGitSCMSourceTest.java | 59 +++++++++++++---- 3 files changed, 86 insertions(+), 45 deletions(-) diff --git a/pom.xml b/pom.xml index e47acb582e..ff48aa3000 100644 --- a/pom.xml +++ b/pom.xml @@ -254,6 +254,12 @@ 4.11.3 compile + + org.jenkins-ci.plugins + git + 4.11.3 + compile + diff --git a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java index 5799331205..644b5d0498 100644 --- a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java +++ b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java @@ -924,16 +924,23 @@ protected SCMRevision retrieve(@NonNull final String revision, @NonNull final Ta } if (rev.toLowerCase(Locale.ENGLISH).startsWith(revision.toLowerCase(Locale.ENGLISH))) { shortNameMatches.add(name); + listener.getLogger().printf("Candidate partial match: %s revision %s%n", name, rev); if (shortHashMatch == null) { - listener.getLogger().printf("Candidate partial match: %s revision %s%n", name, rev); shortHashMatch = rev; } else { - listener.getLogger().printf("Candidate partial match: %s revision %s%n", name, rev); listener.getLogger().printf("Cannot resolve ambiguous short revision %s%n", revision); - return null; + if (fullTagMatches.isEmpty() && fullHashMatches.isEmpty() && fullHashMatch == null) { + // We haven't found any matches, and we have ambiguous matches, cannot determine + return null; + } } } } + if (fullHashMatch != null) { + // JENKINS-62592, for predictability, if an exact match is found, that should be returned + //since this would have been skipped if this was a head or a tag we can just return whatever + return new GitRefSCMRevision(new GitRefSCMHead(fullHashMatch, fullHashMatches.iterator().next()), fullHashMatch); + } if (!fullTagMatches.isEmpty()) { // we just want a tag so we can do a minimal fetch String name = StringUtils.removeStart(fullTagMatches.iterator().next(), Constants.R_TAGS); @@ -943,34 +950,6 @@ protected SCMRevision retrieve(@NonNull final String revision, @NonNull final Ta context.wantTags(true); context.withoutRefSpecs(); } - if (fullHashMatch != null) { - //since this would have been skipped if this was a head or a tag we can just return whatever - return new GitRefSCMRevision(new GitRefSCMHead(fullHashMatch, fullHashMatches.iterator().next()), fullHashMatch); - } - if (shortHashMatch != null) { - // woot this seems unambiguous - for (String name: shortNameMatches) { - if (name.startsWith(Constants.R_HEADS)) { - listener.getLogger().printf("Selected match: %s revision %s%n", name, shortHashMatch); - // WIN it's also a branch - return new GitBranchSCMRevision(new GitBranchSCMHead(StringUtils.removeStart(name, Constants.R_HEADS)), - shortHashMatch); - } else if (name.startsWith(Constants.R_TAGS)) { - tagName = StringUtils.removeStart(name, Constants.R_TAGS); - context.wantBranches(false); - context.wantTags(true); - context.withoutRefSpecs(); - } - } - if (tagName != null) { - listener.getLogger().printf("Selected match: %s revision %s%n", tagName, shortHashMatch); - } else { - return new GitRefSCMRevision(new GitRefSCMHead(shortHashMatch, shortNameMatches.iterator().next()), shortHashMatch); - } - } - if (candidateOtherRef != null) { - return candidateOtherRef; - } //if PruneStaleBranches it should take affect on the following retrievals boolean pruneRefs = context.pruneRefs(); if (tagName != null) { @@ -997,6 +976,30 @@ public SCMRevision run(GitClient client, String remoteName) throws IOException, context, listener, pruneRefs, retrieveContext); } + if (shortHashMatch != null) { + // woot this seems unambiguous + for (String name: shortNameMatches) { + if (name.startsWith(Constants.R_HEADS)) { + listener.getLogger().printf("Selected match: %s revision %s%n", name, shortHashMatch); + // WIN it's also a branch + return new GitBranchSCMRevision(new GitBranchSCMHead(StringUtils.removeStart(name, Constants.R_HEADS)), + shortHashMatch); + } else if (name.startsWith(Constants.R_TAGS)) { + tagName = StringUtils.removeStart(name, Constants.R_TAGS); + context.wantBranches(false); + context.wantTags(true); + context.withoutRefSpecs(); + } + } + if (tagName != null) { + listener.getLogger().printf("Selected match: %s revision %s%n", tagName, shortHashMatch); + } else { + return new GitRefSCMRevision(new GitRefSCMHead(shortHashMatch, shortNameMatches.iterator().next()), shortHashMatch); + } + } + if (candidateOtherRef != null) { + return candidateOtherRef; + } // Pokémon!... Got to catch them all listener.getLogger().printf("Could not find %s in remote references. " + "Pulling heads to local for deep search...%n", revision); @@ -1014,7 +1017,6 @@ public SCMRevision run(GitClient client, String remoteName) throws IOException, //just to be safe listener.error("Could not resolve %s", revision); return null; - } hash = objectId.name(); String candidatePrefix = Constants.R_REMOTES.substring(Constants.R_REFS.length()) diff --git a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java index a56bd27d07..d8ba376db2 100644 --- a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java +++ b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java @@ -11,10 +11,7 @@ import hudson.EnvVars; import hudson.FilePath; import hudson.Launcher; -import hudson.model.Action; -import hudson.model.Actionable; -import hudson.model.Run; -import hudson.model.TaskListener; +import hudson.model.*; import hudson.plugins.git.GitException; import hudson.plugins.git.UserRemoteConfig; import hudson.plugins.git.extensions.impl.IgnoreNotifyCommit; @@ -33,11 +30,8 @@ import java.util.Set; import java.util.TreeMap; import java.util.UUID; -import jenkins.plugins.git.traits.BranchDiscoveryTrait; -import jenkins.plugins.git.traits.DiscoverOtherRefsTrait; -import jenkins.plugins.git.traits.IgnoreOnPushNotificationTrait; -import jenkins.plugins.git.traits.PruneStaleBranchTrait; -import jenkins.plugins.git.traits.TagDiscoveryTrait; + +import jenkins.plugins.git.traits.*; import jenkins.scm.api.SCMHead; import jenkins.scm.api.SCMHeadObserver; @@ -1082,30 +1076,69 @@ public void when_commits_added_during_discovery_we_do_not_crash() throws Excepti } } + @Issue("JENKINS-62592") @Test - public void shouldReturnExactMatchOverRelativeMatchTest() throws Exception { - // TODO: The idea is that this code should now make sure that there isn't an exact match before sending something back + public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception { sampleRepo.init(); String masterHash = sampleRepo.head(); sampleRepo.git("checkout", "-b", "dev"); sampleRepo.write("file", "modified"); sampleRepo.git("commit", "--all", "--message=dev"); + sampleRepo.git("tag", "v1"); String v1Hash = sampleRepo.head(); sampleRepo.write("file", "modified2"); sampleRepo.git("commit", "--all", "--message=dev2"); + sampleRepo.git("tag", "-a", "v2", "-m", "annotated"); String v2Hash = sampleRepo.head(); sampleRepo.write("file", "modified3"); sampleRepo.git("commit", "--all", "--message=dev3"); + // Grab the devHash, but lets try and generate a hash that we know will cause an issue in our test + ArrayList hashFirstLetter = new ArrayList<>(Arrays.asList(masterHash.substring(0,1), v1Hash.substring(0,1), v2Hash.substring(0,1))); String devHash = sampleRepo.head(); + int i = 4; // In order to name new files and create new commits + String newHash = null; + while (!hashFirstLetter.contains(devHash.substring(0,1))) { + // Generate a new commit and try again + sampleRepo.write("file", "modified" + i); + sampleRepo.git("commit", "--all", "--message=dev" + (i++)); + newHash = sampleRepo.head(); + hashFirstLetter.add(newHash.substring(0,1)); + } GitSCMSource source = new GitSCMSource(sampleRepo.toString()); - source.setTraits(Collections.singletonList(new BranchDiscoveryTrait())); + source.setTraits(new ArrayList<>()); TaskListener listener = StreamTaskListener.fromStderr(); listener.getLogger().println("\n=== fetch('master') ===\n"); - SCMRevision rev = source.fetch(masterHash, listener, null); + SCMRevision rev = source.fetch("master", listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(masterHash)); + listener.getLogger().println("\n=== fetch('dev') ===\n"); + rev = source.fetch("dev", listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(devHash)); + listener.getLogger().println("\n=== fetch('v1') ===\n"); + rev = source.fetch("v1", listener, null); + assertThat(rev, instanceOf(GitTagSCMRevision.class)); + assertThat(((GitTagSCMRevision)rev).getHash(), is(v1Hash)); + listener.getLogger().println("\n=== fetch('v2') ===\n"); + rev = source.fetch("v2", listener, null); + assertThat(rev, instanceOf(GitTagSCMRevision.class)); + assertThat(((GitTagSCMRevision)rev).getHash(), is(v2Hash)); + + listener.getLogger().printf("%n=== fetch('%s') ===%n%n", masterHash); + rev = source.fetch(masterHash, listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(masterHash)); assertThat(rev.getHead().getName(), is("master")); + + if (null != newHash) { + listener.getLogger().printf("%n=== fetch('%s') ===%n%n", newHash); + rev = source.fetch(newHash, listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(newHash)); + assertThat(rev.getHead().getName(), is("master")); + } } //Ugly but MockGitClient needs to be static and no good way to pass it on From 3f796fa5c3cfaada07db4a1d1ac8599461c869bf Mon Sep 17 00:00:00 2001 From: Noah Rozelle Date: Mon, 18 Jul 2022 16:34:05 -0400 Subject: [PATCH 12/22] tests are showing our problem now, need to ensure we are doing all we need for this feature --- .../plugins/git/AbstractGitSCMSourceTest.java | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java index d8ba376db2..d1ac705ddd 100644 --- a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java +++ b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java @@ -1094,29 +1094,28 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception sampleRepo.git("commit", "--all", "--message=dev3"); // Grab the devHash, but lets try and generate a hash that we know will cause an issue in our test ArrayList hashFirstLetter = new ArrayList<>(Arrays.asList(masterHash.substring(0,1), v1Hash.substring(0,1), v2Hash.substring(0,1))); - String devHash = sampleRepo.head(); + sampleRepo.git("tag", "devTag"); + String devTagHash = sampleRepo.head(); int i = 4; // In order to name new files and create new commits String newHash = null; - while (!hashFirstLetter.contains(devHash.substring(0,1))) { + while (!hashFirstLetter.contains(devTagHash.substring(0,1))) { // Generate a new commit and try again - sampleRepo.write("file", "modified" + i); - sampleRepo.git("commit", "--all", "--message=dev" + (i++)); newHash = sampleRepo.head(); hashFirstLetter.add(newHash.substring(0,1)); + sampleRepo.git("tag", "devTag"+i); + sampleRepo.write("file", "modified" + i); + sampleRepo.git("commit", "--all", "--message=dev" + (i++)); } GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(new ArrayList<>()); TaskListener listener = StreamTaskListener.fromStderr(); + listener.getLogger().printf("ArrayList of first hash chars: %s", hashFirstLetter); listener.getLogger().println("\n=== fetch('master') ===\n"); SCMRevision rev = source.fetch("master", listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(masterHash)); - listener.getLogger().println("\n=== fetch('dev') ===\n"); - rev = source.fetch("dev", listener, null); - assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(devHash)); listener.getLogger().println("\n=== fetch('v1') ===\n"); rev = source.fetch("v1", listener, null); assertThat(rev, instanceOf(GitTagSCMRevision.class)); @@ -1137,7 +1136,13 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception rev = source.fetch(newHash, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(newHash)); - assertThat(rev.getHead().getName(), is("master")); + assertThat(rev.getHead().getName(), is(newHash)); + + listener.getLogger().printf("%n=== fetch(devTag%d) ===%n%n", i - 1); + rev = source.fetch("devTag" + (i-1), listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(newHash)); + assertThat(rev.getHead().getName(), is("devTag" + (i - 1))); } } From a25cdc32540c160d3a605c3e7e13f940c58e1d99 Mon Sep 17 00:00:00 2001 From: Noah Rozelle Date: Tue, 19 Jul 2022 09:12:44 -0400 Subject: [PATCH 13/22] tests breaking, but looking into what issue could be, think issue has to do with my dynamic test generator (not grabbing the right tag head) --- .../plugins/git/AbstractGitSCMSource.java | 10 +++++++-- .../plugins/git/AbstractGitSCMSourceTest.java | 21 +++++++++++++++---- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java index 644b5d0498..84855c38f9 100644 --- a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java +++ b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java @@ -869,6 +869,7 @@ protected SCMRevision retrieve(@NonNull final String revision, @NonNull final Ta Set fullHashMatches = new TreeSet<>(); String fullHashMatch = null; GitRefSCMRevision candidateOtherRef = null; + Boolean shortRevisionAmbiguity = false; for (Map.Entry entry: remoteReferences.entrySet()) { String name = entry.getKey(); String rev = entry.getValue().name(); @@ -929,9 +930,10 @@ protected SCMRevision retrieve(@NonNull final String revision, @NonNull final Ta shortHashMatch = rev; } else { listener.getLogger().printf("Cannot resolve ambiguous short revision %s%n", revision); - if (fullTagMatches.isEmpty() && fullHashMatches.isEmpty() && fullHashMatch == null) { + if (fullTagMatches.isEmpty() && fullHashMatches.isEmpty() && fullHashMatch == null && candidateOtherRef == null) { // We haven't found any matches, and we have ambiguous matches, cannot determine - return null; + // TODO we gotta make a variable that determines if we have gone throught he entire list before returning null + shortRevisionAmbiguity = true; } } } @@ -1000,6 +1002,10 @@ public SCMRevision run(GitClient client, String remoteName) throws IOException, if (candidateOtherRef != null) { return candidateOtherRef; } + if (shortRevisionAmbiguity) { + // Check if we have any other matches first, if not, return null as we cannot determine a match + return null; + } // Pokémon!... Got to catch them all listener.getLogger().printf("Could not find %s in remote references. " + "Pulling heads to local for deep search...%n", revision); diff --git a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java index d1ac705ddd..316c7b22f2 100644 --- a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java +++ b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java @@ -1097,14 +1097,14 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception sampleRepo.git("tag", "devTag"); String devTagHash = sampleRepo.head(); int i = 4; // In order to name new files and create new commits - String newHash = null; + String newHash = devTagHash; while (!hashFirstLetter.contains(devTagHash.substring(0,1))) { // Generate a new commit and try again - newHash = sampleRepo.head(); - hashFirstLetter.add(newHash.substring(0,1)); sampleRepo.git("tag", "devTag"+i); sampleRepo.write("file", "modified" + i); sampleRepo.git("commit", "--all", "--message=dev" + (i++)); + newHash = sampleRepo.head(); + hashFirstLetter.add(newHash.substring(0,1)); } GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(new ArrayList<>()); @@ -1136,13 +1136,26 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception rev = source.fetch(newHash, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(newHash)); - assertThat(rev.getHead().getName(), is(newHash)); + assertThat(rev.getHead().getName(), is("dev")); + + listener.getLogger().printf("%n=== fetch('%s') short hash ===%n%n", newHash.substring(0, 6)); + rev = source.fetch(newHash.substring(0, 6), listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(newHash)); + assertThat(rev.getHead().getName(), is("dev")); listener.getLogger().printf("%n=== fetch(devTag%d) ===%n%n", i - 1); rev = source.fetch("devTag" + (i-1), listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(newHash)); assertThat(rev.getHead().getName(), is("devTag" + (i - 1))); + + String ambiguousTag = hashFirstLetter.get(hashFirstLetter.size() - 1); + listener.getLogger().printf("%n=== fetch(%s) ===%n%n", ambiguousTag); + rev = source.fetch(ambiguousTag, listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(devTagHash)); + assertThat(rev.getHead().getName(), is("devTag" + (i - 1))); } } From a78a5038c72063336078fba2c15f803af745a5af Mon Sep 17 00:00:00 2001 From: Noah Rozelle Date: Tue, 19 Jul 2022 09:48:32 -0400 Subject: [PATCH 14/22] yup, head wasn't correct, acting appropriately now, tests are passing with dynamic test creation --- .../plugins/git/AbstractGitSCMSourceTest.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java index 316c7b22f2..8164d94a49 100644 --- a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java +++ b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java @@ -1097,12 +1097,14 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception sampleRepo.git("tag", "devTag"); String devTagHash = sampleRepo.head(); int i = 4; // In order to name new files and create new commits + String previousNewHash = null; String newHash = devTagHash; while (!hashFirstLetter.contains(devTagHash.substring(0,1))) { // Generate a new commit and try again sampleRepo.git("tag", "devTag"+i); sampleRepo.write("file", "modified" + i); sampleRepo.git("commit", "--all", "--message=dev" + (i++)); + previousNewHash = newHash; newHash = sampleRepo.head(); hashFirstLetter.add(newHash.substring(0,1)); } @@ -1131,7 +1133,7 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(masterHash)); assertThat(rev.getHead().getName(), is("master")); - if (null != newHash) { + if (!devTagHash.equals(newHash)) { listener.getLogger().printf("%n=== fetch('%s') ===%n%n", newHash); rev = source.fetch(newHash, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); @@ -1144,18 +1146,21 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(newHash)); assertThat(rev.getHead().getName(), is("dev")); - listener.getLogger().printf("%n=== fetch(devTag%d) ===%n%n", i - 1); - rev = source.fetch("devTag" + (i-1), listener, null); + + int lastDevTag = i == 4 ? 4 : i - 1; + String headHash = i == 4 ? devTagHash : previousNewHash; + listener.getLogger().printf("%n=== fetch(devTag%d) ===%n%n", lastDevTag); + rev = source.fetch("devTag" + lastDevTag, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(newHash)); - assertThat(rev.getHead().getName(), is("devTag" + (i - 1))); + assertThat(rev.getHead().getName(), is("devTag" + lastDevTag)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(headHash)); String ambiguousTag = hashFirstLetter.get(hashFirstLetter.size() - 1); listener.getLogger().printf("%n=== fetch(%s) ===%n%n", ambiguousTag); rev = source.fetch(ambiguousTag, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(rev.getHead().getName(), is("dev")); assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(devTagHash)); - assertThat(rev.getHead().getName(), is("devTag" + (i - 1))); } } From 817cfaa72de834dfc26b8a13fdaabe91cd8a3028 Mon Sep 17 00:00:00 2001 From: Noah Rozelle Date: Tue, 19 Jul 2022 10:36:41 -0400 Subject: [PATCH 15/22] JENKINS-62592 cleaning up test which was impacted by change, fixed now! --- src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java index 84855c38f9..b7a3a6d3db 100644 --- a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java +++ b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java @@ -923,7 +923,7 @@ protected SCMRevision retrieve(@NonNull final String revision, @NonNull final Ta break; } } - if (rev.toLowerCase(Locale.ENGLISH).startsWith(revision.toLowerCase(Locale.ENGLISH))) { + if (!revision.isEmpty() && rev.toLowerCase(Locale.ENGLISH).startsWith(revision.toLowerCase(Locale.ENGLISH))) { shortNameMatches.add(name); listener.getLogger().printf("Candidate partial match: %s revision %s%n", name, rev); if (shortHashMatch == null) { From a54838ea066b33fc4968f1ae57a626508293a183 Mon Sep 17 00:00:00 2001 From: Noah Rozelle Date: Tue, 19 Jul 2022 13:18:10 -0400 Subject: [PATCH 16/22] JENKINS-62592 reformat, renamed some bad test variable names --- .../plugins/git/AbstractGitSCMSourceTest.java | 59 ++++++++++--------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java index 8164d94a49..3c0955684a 100644 --- a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java +++ b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java @@ -21,6 +21,7 @@ import hudson.plugins.git.extensions.impl.BuildChooserSetting; import hudson.plugins.git.extensions.impl.LocalBranch; import hudson.util.StreamTaskListener; + import java.io.File; import java.util.ArrayList; import java.util.Arrays; @@ -469,7 +470,7 @@ private void retrievePrimaryHead(boolean duplicatePrimary) throws Exception { source.setOwner(owner); TaskListener listener = StreamTaskListener.fromStderr(); Map headByName = new TreeMap<>(); - for (SCMHead h: source.fetch(listener)) { + for (SCMHead h : source.fetch(listener)) { headByName.put(h.getName(), h); } if (duplicatePrimary) { @@ -479,7 +480,7 @@ private void retrievePrimaryHead(boolean duplicatePrimary) throws Exception { } List actions = source.fetchActions(null, listener); GitRemoteHeadRefAction refAction = null; - for (Action a: actions) { + for (Action a : actions) { if (a instanceof GitRemoteHeadRefAction) { refAction = (GitRemoteHeadRefAction) a; break; @@ -497,7 +498,7 @@ private void retrievePrimaryHead(boolean duplicatePrimary) throws Exception { } PrimaryInstanceMetadataAction primary = null; - for (Action a: actions) { + for (Action a : actions) { if (a instanceof PrimaryInstanceMetadataAction) { primary = (PrimaryInstanceMetadataAction) a; break; @@ -524,7 +525,7 @@ public void retrieveRevision() throws Exception { sampleRepo.write("file", "v3"); sampleRepo.git("commit", "--all", "--message=v3"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList(new BranchDiscoveryTrait(), new TagDiscoveryTrait())); StreamTaskListener listener = StreamTaskListener.fromStderr(); @@ -589,7 +590,7 @@ public void retrieveRevision_nonAdvertised() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList(new BranchDiscoveryTrait(), new TagDiscoveryTrait())); StreamTaskListener listener = StreamTaskListener.fromStderr(); @@ -618,7 +619,7 @@ public void retrieveRevision_customRef() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList( new BranchDiscoveryTrait(), @@ -653,7 +654,7 @@ public void retrieveRevision_customRef_descendant() throws Exception { sampleRepo.write("file", "v5"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList( new BranchDiscoveryTrait(), @@ -683,7 +684,7 @@ public void retrieveRevision_customRef_abbrev_sha1() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList( new BranchDiscoveryTrait(), @@ -713,7 +714,7 @@ public void retrieveRevision_pr_refspec() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList(new BranchDiscoveryTrait(), new TagDiscoveryTrait(), new DiscoverOtherRefsTrait("pull-requests/*/from"))); StreamTaskListener listener = StreamTaskListener.fromStderr(); @@ -740,7 +741,7 @@ public void retrieveRevision_pr_local_refspec() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); //new RefSpecsSCMSourceTrait("+refs/pull-requests/*/from:refs/remotes/@{remote}/pr/*") source.setTraits(Arrays.asList(new BranchDiscoveryTrait(), new TagDiscoveryTrait(), @@ -751,7 +752,8 @@ public void retrieveRevision_pr_local_refspec() throws Exception { } private int wsCount; - private String fileAt(String revision, Run run, SCMSource source, TaskListener listener) throws Exception { + + private String fileAt(String revision, Run run, SCMSource source, TaskListener listener) throws Exception { SCMRevision rev = source.fetch(revision, listener, null); if (rev == null) { return null; @@ -781,7 +783,7 @@ public void fetchOtherRef() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList(new BranchDiscoveryTrait(), new TagDiscoveryTrait(), new DiscoverOtherRefsTrait("custom/*"))); StreamTaskListener listener = StreamTaskListener.fromStderr(); @@ -816,7 +818,7 @@ public void fetchOtherRevisions() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList(new BranchDiscoveryTrait(), new TagDiscoveryTrait(), new DiscoverOtherRefsTrait("custom/*"))); StreamTaskListener listener = StreamTaskListener.fromStderr(); @@ -1050,7 +1052,8 @@ private void createRefLockEnvironment(TaskListener listener, GitSCMSource source sampleRepo.git("push", source.getRemote(), "v1.2"); } - @Test @Issue("JENKINS-50394") + @Test + @Issue("JENKINS-50394") public void when_commits_added_during_discovery_we_do_not_crash() throws Exception { sampleRepo.init(); sampleRepo.git("checkout", "-b", "dev"); @@ -1068,7 +1071,7 @@ public void when_commits_added_during_discovery_we_do_not_crash() throws Excepti hasProperty("name", equalTo("master")), hasProperty("name", equalTo("dev")) )); - } catch(MissingObjectException me) { + } catch (MissingObjectException me) { fail("Not supposed to get MissingObjectException"); } finally { System.clearProperty(Git.class.getName() + ".mockClient"); @@ -1093,20 +1096,22 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception sampleRepo.write("file", "modified3"); sampleRepo.git("commit", "--all", "--message=dev3"); // Grab the devHash, but lets try and generate a hash that we know will cause an issue in our test - ArrayList hashFirstLetter = new ArrayList<>(Arrays.asList(masterHash.substring(0,1), v1Hash.substring(0,1), v2Hash.substring(0,1))); + ArrayList hashFirstLetter = new ArrayList<>( + Arrays.asList(masterHash.substring(0, 1), v1Hash.substring(0, 1), v2Hash.substring(0, 1)) + ); sampleRepo.git("tag", "devTag"); String devTagHash = sampleRepo.head(); - int i = 4; // In order to name new files and create new commits + int devTagIteration = 4; // In order to name new files and create new commits String previousNewHash = null; String newHash = devTagHash; - while (!hashFirstLetter.contains(devTagHash.substring(0,1))) { + while (!hashFirstLetter.contains(devTagHash.substring(0, 1))) { // Generate a new commit and try again - sampleRepo.git("tag", "devTag"+i); - sampleRepo.write("file", "modified" + i); - sampleRepo.git("commit", "--all", "--message=dev" + (i++)); + sampleRepo.git("tag", "devTag" + devTagIteration); + sampleRepo.write("file", "modified" + devTagIteration); + sampleRepo.git("commit", "--all", "--message=dev" + (devTagIteration++)); previousNewHash = newHash; newHash = sampleRepo.head(); - hashFirstLetter.add(newHash.substring(0,1)); + hashFirstLetter.add(newHash.substring(0, 1)); } GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(new ArrayList<>()); @@ -1117,15 +1122,15 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception listener.getLogger().println("\n=== fetch('master') ===\n"); SCMRevision rev = source.fetch("master", listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(masterHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(masterHash)); listener.getLogger().println("\n=== fetch('v1') ===\n"); rev = source.fetch("v1", listener, null); assertThat(rev, instanceOf(GitTagSCMRevision.class)); - assertThat(((GitTagSCMRevision)rev).getHash(), is(v1Hash)); + assertThat(((GitTagSCMRevision) rev).getHash(), is(v1Hash)); listener.getLogger().println("\n=== fetch('v2') ===\n"); rev = source.fetch("v2", listener, null); assertThat(rev, instanceOf(GitTagSCMRevision.class)); - assertThat(((GitTagSCMRevision)rev).getHash(), is(v2Hash)); + assertThat(((GitTagSCMRevision) rev).getHash(), is(v2Hash)); listener.getLogger().printf("%n=== fetch('%s') ===%n%n", masterHash); rev = source.fetch(masterHash, listener, null); @@ -1147,8 +1152,8 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception assertThat(rev.getHead().getName(), is("dev")); - int lastDevTag = i == 4 ? 4 : i - 1; - String headHash = i == 4 ? devTagHash : previousNewHash; + int lastDevTag = devTagIteration == 4 ? 4 : devTagIteration - 1; + String headHash = devTagIteration == 4 ? devTagHash : previousNewHash; listener.getLogger().printf("%n=== fetch(devTag%d) ===%n%n", lastDevTag); rev = source.fetch("devTag" + lastDevTag, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); From bbd15584fad0ff05ff04b6ccc0e3141d0f52992c Mon Sep 17 00:00:00 2001 From: Noah Rozelle Date: Tue, 19 Jul 2022 13:40:18 -0400 Subject: [PATCH 17/22] JENKINS-62592 adding a more "to-the-point" test regarding the issue we have seen --- .../jenkins/plugins/git/AbstractGitSCMSourceTest.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java index 3c0955684a..ace89a72e6 100644 --- a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java +++ b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java @@ -1119,6 +1119,7 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception TaskListener listener = StreamTaskListener.fromStderr(); listener.getLogger().printf("ArrayList of first hash chars: %s", hashFirstLetter); + // Test existing functionality with additional revisions listener.getLogger().println("\n=== fetch('master') ===\n"); SCMRevision rev = source.fetch("master", listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); @@ -1138,6 +1139,7 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(masterHash)); assertThat(rev.getHead().getName(), is("master")); + // Test new functionality and verify that we are able to grab what we expect (full match versus hazy short hash) if (!devTagHash.equals(newHash)) { listener.getLogger().printf("%n=== fetch('%s') ===%n%n", newHash); rev = source.fetch(newHash, listener, null); @@ -1161,11 +1163,15 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(headHash)); String ambiguousTag = hashFirstLetter.get(hashFirstLetter.size() - 1); + String ambiguousHash = sampleRepo.head(); + sampleRepo.git("tag", ambiguousTag); + sampleRepo.write("file", "modified and ambiguous"); + sampleRepo.git("commit", "--all", "--message=ambiguousTagCommit"); listener.getLogger().printf("%n=== fetch(%s) ===%n%n", ambiguousTag); rev = source.fetch(ambiguousTag, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(rev.getHead().getName(), is("dev")); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(devTagHash)); + assertThat(rev.getHead().getName(), is(ambiguousTag)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(ambiguousHash)); } } From 94500f863fb85a1788c7a4d1110ae9646787933c Mon Sep 17 00:00:00 2001 From: Noah Rozelle Date: Thu, 21 Jul 2022 11:18:39 -0400 Subject: [PATCH 18/22] JENKINS-62592 adding a test to show branch will resolve first over hazy short hash --- .../jenkins/plugins/git/AbstractGitSCMSource.java | 1 - .../plugins/git/AbstractGitSCMSourceTest.java | 12 ++++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java index b7a3a6d3db..d16652661d 100644 --- a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java +++ b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java @@ -932,7 +932,6 @@ protected SCMRevision retrieve(@NonNull final String revision, @NonNull final Ta listener.getLogger().printf("Cannot resolve ambiguous short revision %s%n", revision); if (fullTagMatches.isEmpty() && fullHashMatches.isEmpty() && fullHashMatch == null && candidateOtherRef == null) { // We haven't found any matches, and we have ambiguous matches, cannot determine - // TODO we gotta make a variable that determines if we have gone throught he entire list before returning null shortRevisionAmbiguity = true; } } diff --git a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java index ace89a72e6..43200d4634 100644 --- a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java +++ b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java @@ -1172,6 +1172,18 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); assertThat(rev.getHead().getName(), is(ambiguousTag)); assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(ambiguousHash)); + + // Remove ambiguous tag from the repo and thne create a branch with the ambiguous search + sampleRepo.git("tag", "-d", ambiguousTag); + sampleRepo.git("checkout", "-b", ambiguousTag); + sampleRepo.write("file", "modified and ambiguous but a branch this time!"); + sampleRepo.git("commit", "--all", "--message=ambiguousBranchCommit"); + listener.getLogger().printf("%n=== fetch(%s) branch ===%n%n", ambiguousTag); + String ambiguousBranchHash = sampleRepo.head(); + rev = source.fetch(ambiguousTag, listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(rev.getHead().getName(), is(ambiguousTag)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(ambiguousBranchHash)); } } From d1bcc8013834ec7175e78c49ad0d689cf58be165 Mon Sep 17 00:00:00 2001 From: Noah Rozelle Date: Tue, 16 Aug 2022 08:47:34 -0400 Subject: [PATCH 19/22] JENKINS-62592 adding a test to show branch will resolve first over hazy short hash --- .../plugins/git/AbstractGitSCMSource.java | 1 - .../plugins/git/AbstractGitSCMSourceTest.java | 27 ++++++++++++++----- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java index b7a3a6d3db..d16652661d 100644 --- a/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java +++ b/src/main/java/jenkins/plugins/git/AbstractGitSCMSource.java @@ -932,7 +932,6 @@ protected SCMRevision retrieve(@NonNull final String revision, @NonNull final Ta listener.getLogger().printf("Cannot resolve ambiguous short revision %s%n", revision); if (fullTagMatches.isEmpty() && fullHashMatches.isEmpty() && fullHashMatch == null && candidateOtherRef == null) { // We haven't found any matches, and we have ambiguous matches, cannot determine - // TODO we gotta make a variable that determines if we have gone throught he entire list before returning null shortRevisionAmbiguity = true; } } diff --git a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java index ace89a72e6..55965ce625 100644 --- a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java +++ b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java @@ -8,6 +8,7 @@ import com.cloudbees.plugins.credentials.common.StandardCredentials; import com.cloudbees.plugins.credentials.domains.Domain; import com.cloudbees.plugins.credentials.impl.UsernamePasswordCredentialsImpl; +import edu.umd.cs.findbugs.annotations.NonNull; import hudson.EnvVars; import hudson.FilePath; import hudson.Launcher; @@ -23,6 +24,7 @@ import hudson.util.StreamTaskListener; import java.io.File; +import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -47,6 +49,7 @@ import jenkins.scm.api.SCMSourceCriteria; import jenkins.scm.api.SCMSourceOwner; import jenkins.scm.api.metadata.PrimaryInstanceMetadataAction; +import jenkins.scm.api.trait.SCMSourceTrait; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.transport.RefSpec; @@ -360,19 +363,19 @@ public void retrieveByName() throws Exception { listener.getLogger().println("\n=== fetch('master') ===\n"); SCMRevision rev = source.fetch("master", listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(masterHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(masterHash)); listener.getLogger().println("\n=== fetch('dev') ===\n"); rev = source.fetch("dev", listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(devHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(devHash)); listener.getLogger().println("\n=== fetch('v1') ===\n"); rev = source.fetch("v1", listener, null); assertThat(rev, instanceOf(GitTagSCMRevision.class)); - assertThat(((GitTagSCMRevision)rev).getHash(), is(v1Hash)); + assertThat(((GitTagSCMRevision) rev).getHash(), is(v1Hash)); listener.getLogger().println("\n=== fetch('v2') ===\n"); rev = source.fetch("v2", listener, null); assertThat(rev, instanceOf(GitTagSCMRevision.class)); - assertThat(((GitTagSCMRevision)rev).getHash(), is(v2Hash)); + assertThat(((GitTagSCMRevision) rev).getHash(), is(v2Hash)); listener.getLogger().printf("%n=== fetch('%s') ===%n%n", masterHash); rev = source.fetch(masterHash, listener, null); @@ -563,7 +566,7 @@ public void retrieveRevision_nonHead() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList(new BranchDiscoveryTrait(), new TagDiscoveryTrait())); StreamTaskListener listener = StreamTaskListener.fromStderr(); @@ -789,7 +792,7 @@ public void fetchOtherRef() throws Exception { StreamTaskListener listener = StreamTaskListener.fromStderr(); final SCMHeadObserver.Collector collector = - source.fetch((SCMSourceCriteria) (probe, listener1) -> true, new SCMHeadObserver.Collector(), listener); + source.fetch((SCMSourceCriteria) (probe, listener1) -> true, new SCMHeadObserver.Collector(), listener); final Map result = collector.result(); assertThat(result.entrySet(), hasSize(4)); @@ -1172,6 +1175,18 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); assertThat(rev.getHead().getName(), is(ambiguousTag)); assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(ambiguousHash)); + + // Remove ambiguous tag from the repo and thne create a branch with the ambiguous search + sampleRepo.git("tag", "-d", ambiguousTag); + sampleRepo.git("checkout", "-b", ambiguousTag); + sampleRepo.write("file", "modified and ambiguous but a branch this time!"); + sampleRepo.git("commit", "--all", "--message=ambiguousBranchCommit"); + listener.getLogger().printf("%n=== fetch(%s) branch ===%n%n", ambiguousTag); + String ambiguousBranchHash = sampleRepo.head(); + rev = source.fetch(ambiguousTag, listener, null); + assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); + assertThat(rev.getHead().getName(), is(ambiguousTag)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(ambiguousBranchHash)); } } From 84660807d229229dfb46ec418c784eb755d6417b Mon Sep 17 00:00:00 2001 From: Noah Rozelle Date: Mon, 26 Sep 2022 14:34:37 -0400 Subject: [PATCH 20/22] JENKINS-62592 address wildcard import and unnecessary whitespace changes --- .../plugins/git/AbstractGitSCMSourceTest.java | 83 +++++++++---------- 1 file changed, 41 insertions(+), 42 deletions(-) diff --git a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java index 55965ce625..7789ef0dfb 100644 --- a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java +++ b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java @@ -12,7 +12,10 @@ import hudson.EnvVars; import hudson.FilePath; import hudson.Launcher; -import hudson.model.*; +import hudson.model.Action; +import hudson.model.Actionable; +import hudson.model.Run; +import hudson.model.TaskListener; import hudson.plugins.git.GitException; import hudson.plugins.git.UserRemoteConfig; import hudson.plugins.git.extensions.impl.IgnoreNotifyCommit; @@ -22,7 +25,6 @@ import hudson.plugins.git.extensions.impl.BuildChooserSetting; import hudson.plugins.git.extensions.impl.LocalBranch; import hudson.util.StreamTaskListener; - import java.io.File; import java.io.IOException; import java.util.ArrayList; @@ -33,7 +35,6 @@ import java.util.Set; import java.util.TreeMap; import java.util.UUID; - import jenkins.plugins.git.traits.*; import jenkins.scm.api.SCMHead; @@ -363,69 +364,69 @@ public void retrieveByName() throws Exception { listener.getLogger().println("\n=== fetch('master') ===\n"); SCMRevision rev = source.fetch("master", listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(masterHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(masterHash)); listener.getLogger().println("\n=== fetch('dev') ===\n"); rev = source.fetch("dev", listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(devHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(devHash)); listener.getLogger().println("\n=== fetch('v1') ===\n"); rev = source.fetch("v1", listener, null); assertThat(rev, instanceOf(GitTagSCMRevision.class)); - assertThat(((GitTagSCMRevision) rev).getHash(), is(v1Hash)); + assertThat(((GitTagSCMRevision)rev).getHash(), is(v1Hash)); listener.getLogger().println("\n=== fetch('v2') ===\n"); rev = source.fetch("v2", listener, null); assertThat(rev, instanceOf(GitTagSCMRevision.class)); - assertThat(((GitTagSCMRevision) rev).getHash(), is(v2Hash)); + assertThat(((GitTagSCMRevision)rev).getHash(), is(v2Hash)); listener.getLogger().printf("%n=== fetch('%s') ===%n%n", masterHash); rev = source.fetch(masterHash, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(masterHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(masterHash)); assertThat(rev.getHead().getName(), is("master")); listener.getLogger().printf("%n=== fetch('%s') ===%n%n", masterHash.substring(0, 10)); rev = source.fetch(masterHash.substring(0, 10), listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(masterHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(masterHash)); assertThat(rev.getHead().getName(), is("master")); listener.getLogger().printf("%n=== fetch('%s') ===%n%n", devHash); rev = source.fetch(devHash, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(devHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(devHash)); assertThat(rev.getHead().getName(), is("dev")); listener.getLogger().printf("%n=== fetch('%s') ===%n%n", devHash.substring(0, 10)); rev = source.fetch(devHash.substring(0, 10), listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(devHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(devHash)); assertThat(rev.getHead().getName(), is("dev")); listener.getLogger().printf("%n=== fetch('%s') ===%n%n", v1Hash); rev = source.fetch(v1Hash, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(v1Hash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(v1Hash)); listener.getLogger().printf("%n=== fetch('%s') ===%n%n", v1Hash.substring(0, 10)); rev = source.fetch(v1Hash.substring(0, 10), listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(v1Hash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(v1Hash)); listener.getLogger().printf("%n=== fetch('%s') ===%n%n", v2Hash); rev = source.fetch(v2Hash, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(v2Hash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(v2Hash)); listener.getLogger().printf("%n=== fetch('%s') ===%n%n", v2Hash.substring(0, 10)); rev = source.fetch(v2Hash.substring(0, 10), listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(v2Hash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(v2Hash)); String v2Tag = "refs/tags/v2"; listener.getLogger().printf("%n=== fetch('%s') ===%n%n", v2Tag); rev = source.fetch(v2Tag, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(v2Hash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(v2Hash)); } @@ -473,7 +474,7 @@ private void retrievePrimaryHead(boolean duplicatePrimary) throws Exception { source.setOwner(owner); TaskListener listener = StreamTaskListener.fromStderr(); Map headByName = new TreeMap<>(); - for (SCMHead h : source.fetch(listener)) { + for (SCMHead h: source.fetch(listener)) { headByName.put(h.getName(), h); } if (duplicatePrimary) { @@ -483,7 +484,7 @@ private void retrievePrimaryHead(boolean duplicatePrimary) throws Exception { } List actions = source.fetchActions(null, listener); GitRemoteHeadRefAction refAction = null; - for (Action a : actions) { + for (Action a: actions) { if (a instanceof GitRemoteHeadRefAction) { refAction = (GitRemoteHeadRefAction) a; break; @@ -528,7 +529,7 @@ public void retrieveRevision() throws Exception { sampleRepo.write("file", "v3"); sampleRepo.git("commit", "--all", "--message=v3"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList(new BranchDiscoveryTrait(), new TagDiscoveryTrait())); StreamTaskListener listener = StreamTaskListener.fromStderr(); @@ -566,7 +567,7 @@ public void retrieveRevision_nonHead() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList(new BranchDiscoveryTrait(), new TagDiscoveryTrait())); StreamTaskListener listener = StreamTaskListener.fromStderr(); @@ -593,7 +594,7 @@ public void retrieveRevision_nonAdvertised() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList(new BranchDiscoveryTrait(), new TagDiscoveryTrait())); StreamTaskListener listener = StreamTaskListener.fromStderr(); @@ -622,7 +623,7 @@ public void retrieveRevision_customRef() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList( new BranchDiscoveryTrait(), @@ -657,7 +658,7 @@ public void retrieveRevision_customRef_descendant() throws Exception { sampleRepo.write("file", "v5"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList( new BranchDiscoveryTrait(), @@ -687,7 +688,7 @@ public void retrieveRevision_customRef_abbrev_sha1() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList( new BranchDiscoveryTrait(), @@ -717,7 +718,7 @@ public void retrieveRevision_pr_refspec() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList(new BranchDiscoveryTrait(), new TagDiscoveryTrait(), new DiscoverOtherRefsTrait("pull-requests/*/from"))); StreamTaskListener listener = StreamTaskListener.fromStderr(); @@ -744,7 +745,7 @@ public void retrieveRevision_pr_local_refspec() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); //new RefSpecsSCMSourceTrait("+refs/pull-requests/*/from:refs/remotes/@{remote}/pr/*") source.setTraits(Arrays.asList(new BranchDiscoveryTrait(), new TagDiscoveryTrait(), @@ -755,8 +756,7 @@ public void retrieveRevision_pr_local_refspec() throws Exception { } private int wsCount; - - private String fileAt(String revision, Run run, SCMSource source, TaskListener listener) throws Exception { + private String fileAt(String revision, Run run, SCMSource source, TaskListener listener) throws Exception { SCMRevision rev = source.fetch(revision, listener, null); if (rev == null) { return null; @@ -786,7 +786,7 @@ public void fetchOtherRef() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList(new BranchDiscoveryTrait(), new TagDiscoveryTrait(), new DiscoverOtherRefsTrait("custom/*"))); StreamTaskListener listener = StreamTaskListener.fromStderr(); @@ -821,7 +821,7 @@ public void fetchOtherRevisions() throws Exception { sampleRepo.write("file", "v4"); sampleRepo.git("commit", "--all", "--message=v4"); // dev // SCM.checkout does not permit a null build argument, unfortunately. - Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); + Run run = r.buildAndAssertSuccess(r.createFreeStyleProject()); GitSCMSource source = new GitSCMSource(sampleRepo.toString()); source.setTraits(Arrays.asList(new BranchDiscoveryTrait(), new TagDiscoveryTrait(), new DiscoverOtherRefsTrait("custom/*"))); StreamTaskListener listener = StreamTaskListener.fromStderr(); @@ -1055,8 +1055,7 @@ private void createRefLockEnvironment(TaskListener listener, GitSCMSource source sampleRepo.git("push", source.getRemote(), "v1.2"); } - @Test - @Issue("JENKINS-50394") + @Test @Issue("JENKINS-50394") public void when_commits_added_during_discovery_we_do_not_crash() throws Exception { sampleRepo.init(); sampleRepo.git("checkout", "-b", "dev"); @@ -1074,7 +1073,7 @@ public void when_commits_added_during_discovery_we_do_not_crash() throws Excepti hasProperty("name", equalTo("master")), hasProperty("name", equalTo("dev")) )); - } catch (MissingObjectException me) { + } catch(MissingObjectException me) { fail("Not supposed to get MissingObjectException"); } finally { System.clearProperty(Git.class.getName() + ".mockClient"); @@ -1126,20 +1125,20 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception listener.getLogger().println("\n=== fetch('master') ===\n"); SCMRevision rev = source.fetch("master", listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(masterHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(masterHash)); listener.getLogger().println("\n=== fetch('v1') ===\n"); rev = source.fetch("v1", listener, null); assertThat(rev, instanceOf(GitTagSCMRevision.class)); - assertThat(((GitTagSCMRevision) rev).getHash(), is(v1Hash)); + assertThat(((GitTagSCMRevision)rev).getHash(), is(v1Hash)); listener.getLogger().println("\n=== fetch('v2') ===\n"); rev = source.fetch("v2", listener, null); assertThat(rev, instanceOf(GitTagSCMRevision.class)); - assertThat(((GitTagSCMRevision) rev).getHash(), is(v2Hash)); + assertThat(((GitTagSCMRevision)rev).getHash(), is(v2Hash)); listener.getLogger().printf("%n=== fetch('%s') ===%n%n", masterHash); rev = source.fetch(masterHash, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(masterHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(masterHash)); assertThat(rev.getHead().getName(), is("master")); // Test new functionality and verify that we are able to grab what we expect (full match versus hazy short hash) @@ -1147,13 +1146,13 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception listener.getLogger().printf("%n=== fetch('%s') ===%n%n", newHash); rev = source.fetch(newHash, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(newHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(newHash)); assertThat(rev.getHead().getName(), is("dev")); listener.getLogger().printf("%n=== fetch('%s') short hash ===%n%n", newHash.substring(0, 6)); rev = source.fetch(newHash.substring(0, 6), listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(newHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(newHash)); assertThat(rev.getHead().getName(), is("dev")); @@ -1163,7 +1162,7 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception rev = source.fetch("devTag" + lastDevTag, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); assertThat(rev.getHead().getName(), is("devTag" + lastDevTag)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(headHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(headHash)); String ambiguousTag = hashFirstLetter.get(hashFirstLetter.size() - 1); String ambiguousHash = sampleRepo.head(); @@ -1174,7 +1173,7 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception rev = source.fetch(ambiguousTag, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); assertThat(rev.getHead().getName(), is(ambiguousTag)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(ambiguousHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(ambiguousHash)); // Remove ambiguous tag from the repo and thne create a branch with the ambiguous search sampleRepo.git("tag", "-d", ambiguousTag); @@ -1186,7 +1185,7 @@ public void exactBranchMatchShouldSupersedePartialBranchMatch() throws Exception rev = source.fetch(ambiguousTag, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); assertThat(rev.getHead().getName(), is(ambiguousTag)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(ambiguousBranchHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(ambiguousBranchHash)); } } From fa93af18eb193cef3449c3d303aabc85c385d21a Mon Sep 17 00:00:00 2001 From: Noah Rozelle Date: Mon, 26 Sep 2022 14:37:44 -0400 Subject: [PATCH 21/22] JENKINS-62592 more whitespace changes --- .../plugins/git/AbstractGitSCMSourceTest.java | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java index 7789ef0dfb..697ffa1b7f 100644 --- a/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java +++ b/src/test/java/jenkins/plugins/git/AbstractGitSCMSourceTest.java @@ -35,8 +35,11 @@ import java.util.Set; import java.util.TreeMap; import java.util.UUID; -import jenkins.plugins.git.traits.*; - +import jenkins.plugins.git.traits.BranchDiscoveryTrait; +import jenkins.plugins.git.traits.DiscoverOtherRefsTrait; +import jenkins.plugins.git.traits.IgnoreOnPushNotificationTrait; +import jenkins.plugins.git.traits.PruneStaleBranchTrait; +import jenkins.plugins.git.traits.TagDiscoveryTrait; import jenkins.scm.api.SCMHead; import jenkins.scm.api.SCMHeadObserver; import jenkins.scm.api.SCMRevision; @@ -381,13 +384,13 @@ public void retrieveByName() throws Exception { listener.getLogger().printf("%n=== fetch('%s') ===%n%n", masterHash); rev = source.fetch(masterHash, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(masterHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(masterHash)); assertThat(rev.getHead().getName(), is("master")); listener.getLogger().printf("%n=== fetch('%s') ===%n%n", masterHash.substring(0, 10)); rev = source.fetch(masterHash.substring(0, 10), listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(masterHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(masterHash)); assertThat(rev.getHead().getName(), is("master")); listener.getLogger().printf("%n=== fetch('%s') ===%n%n", devHash); @@ -399,34 +402,34 @@ public void retrieveByName() throws Exception { listener.getLogger().printf("%n=== fetch('%s') ===%n%n", devHash.substring(0, 10)); rev = source.fetch(devHash.substring(0, 10), listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(devHash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(devHash)); assertThat(rev.getHead().getName(), is("dev")); listener.getLogger().printf("%n=== fetch('%s') ===%n%n", v1Hash); rev = source.fetch(v1Hash, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(v1Hash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(v1Hash)); listener.getLogger().printf("%n=== fetch('%s') ===%n%n", v1Hash.substring(0, 10)); rev = source.fetch(v1Hash.substring(0, 10), listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(v1Hash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(v1Hash)); listener.getLogger().printf("%n=== fetch('%s') ===%n%n", v2Hash); rev = source.fetch(v2Hash, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(v2Hash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(v2Hash)); listener.getLogger().printf("%n=== fetch('%s') ===%n%n", v2Hash.substring(0, 10)); rev = source.fetch(v2Hash.substring(0, 10), listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(v2Hash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(v2Hash)); String v2Tag = "refs/tags/v2"; listener.getLogger().printf("%n=== fetch('%s') ===%n%n", v2Tag); rev = source.fetch(v2Tag, listener, null); assertThat(rev, instanceOf(AbstractGitSCMSource.SCMRevisionImpl.class)); - assertThat(((AbstractGitSCMSource.SCMRevisionImpl)rev).getHash(), is(v2Hash)); + assertThat(((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(), is(v2Hash)); } From 5673ebcc09212b268064ec787908067d8cba8c67 Mon Sep 17 00:00:00 2001 From: Mark Waite Date: Mon, 19 Jun 2023 11:10:29 -0600 Subject: [PATCH 22/22] Remove unnecessary change from pom.xml --- pom.xml | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/pom.xml b/pom.xml index f12b103d91..12fd71b806 100644 --- a/pom.xml +++ b/pom.xml @@ -257,18 +257,6 @@ jenkins-test-harness test - - org.jenkins-ci.plugins - git - 4.11.3 - compile - - - org.jenkins-ci.plugins - git - 4.11.3 - compile -