From f9214100fd7de835fe02dd9f2b2f945190df3882 Mon Sep 17 00:00:00 2001 From: Rishabh Budhouliya Date: Wed, 2 Sep 2020 00:01:11 +0530 Subject: [PATCH 01/12] Resolve JENKINS-63519 --- src/main/java/hudson/plugins/git/GitSCM.java | 12 ++- .../jenkins/plugins/git/GitToolChooser.java | 55 +++++++++-- .../plugins/git/GitToolChooserTest.java | 96 ++++++++++++++----- 3 files changed, 130 insertions(+), 33 deletions(-) diff --git a/src/main/java/hudson/plugins/git/GitSCM.java b/src/main/java/hudson/plugins/git/GitSCM.java index 49a435d845..c4b9e233f1 100644 --- a/src/main/java/hudson/plugins/git/GitSCM.java +++ b/src/main/java/hudson/plugins/git/GitSCM.java @@ -838,6 +838,8 @@ public GitClient createClient(TaskListener listener, EnvVars environment, Run>emptyList()); + + jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(tool, t); + + GitToolChooser r = new GitToolChooser(remote, context, credentialsId, t, true); + + System.out.println(r.getGitTool()); + } + /* In the event of having no cache but extension APIs in the ExtensionList, the estimator should recommend a tool instead of recommending no git implementation. @@ -82,9 +107,9 @@ public void testSizeEstimationWithNoGitCache() throws Exception { List list = jenkins.jenkins.getItems(); //Since no installation is provided, the gitExe will be git - String gitExe = "git"; + GitTool rTool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); - GitToolChooser repoSizeEstimator = new GitToolChooser(instance.getRemote(), list.get(0), "github", gitExe, true); + GitToolChooser repoSizeEstimator = new GitToolChooser(instance.getRemote(), list.get(0), "github", rTool, true); String tool = repoSizeEstimator.getGitTool(); // The class should make recommendation because of APIs implementation even though @@ -92,7 +117,7 @@ public void testSizeEstimationWithNoGitCache() throws Exception { assertThat(tool, is(not("NONE"))); // If size were reported as 0, should return NONE - assertThat(repoSizeEstimator.determineSwitchOnSize(0L, gitExe), is("NONE")); + assertThat(repoSizeEstimator.determineSwitchOnSize(0L, rTool), is("NONE")); } /* @@ -131,7 +156,7 @@ public void testSizeEstimationWithGitCache() throws Exception { List list = jenkins.jenkins.getItems(); - GitToolChooser repoSizeEstimator = new GitToolChooser(source.getRemote(), list.get(0), "github", tool.getGitExe(), true); + GitToolChooser repoSizeEstimator = new GitToolChooser(source.getRemote(), list.get(0), "github", tool, true); /* Since the size of repository is 21.785 KiBs, the estimator should suggest "jgit" as an implementation */ @@ -153,9 +178,10 @@ public void testSizeEstimationWithAPIForGit() throws Exception { List list = jenkins.jenkins.getItems(); // Assuming no tool is installed and git is present in the machine - String gitExe = "git"; + GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); + - GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", gitExe, true); + GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool, true); assertThat(sizeEstimator.getGitTool(), containsString("git")); } @@ -178,7 +204,7 @@ public void testSizeEstimationWithAPIForJGit() throws Exception { buildAProject(sampleRepo, false); List list = jenkins.jenkins.getItems(); - GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool.getGitExe(), true); + GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool, true); assertThat(sizeEstimator.getGitTool(), containsString("jgit")); } @@ -196,9 +222,9 @@ public void testSizeEstimationWithBitbucketAPIs() throws Exception { List list = jenkins.jenkins.getItems(); // Assuming no tool is installed by user and git is present in the machine - String gitExe = "git"; + GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); - GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", gitExe,true); + GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool,true); assertThat(sizeEstimator.getGitTool(), is("NONE")); } @@ -217,9 +243,9 @@ public void testSizeEstimationWithException() throws Exception { List list = jenkins.jenkins.getItems(); // Assuming no tool is installed by user and git is present in the machine - String gitExe = "git"; + GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); - GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", gitExe, true); + GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool, true); assertThat(sizeEstimator.getGitTool(), is("NONE")); } @@ -236,9 +262,9 @@ public void testSizeEstimationWithNoCredentials() throws Exception { List list = jenkins.jenkins.getItems(); // Assuming no tool is installed by user and git is present in the machine - String gitExe = "git"; + GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); - GitToolChooser sizeEstimator = new GitToolChooser(sampleRepo.toString(), list.get(0), null, gitExe, true); + GitToolChooser sizeEstimator = new GitToolChooser(sampleRepo.toString(), list.get(0), null, tool, true); assertThat(sizeEstimator.getGitTool(), is("NONE")); } @@ -257,11 +283,11 @@ public void testGitToolChooserWithCustomGitTool() throws Exception { GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(tool); - GitToolChooser gitToolChooser = new GitToolChooser(remote, context, credentialsId, tool.getGitExe(), true); + GitToolChooser gitToolChooser = new GitToolChooser(remote, context, credentialsId, tool, true); //According to size of repo, "jgit" should be recommended but it is not installed by the user - //Hence, in this case GitToolChooser resolve gitExe as the user configured `home` value - assertThat(gitToolChooser.getGitTool(), is(SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git")); + //Hence, in this case GitToolChooser should return a NONE + assertThat(gitToolChooser.getGitTool(), is("NONE")); } @@ -276,7 +302,7 @@ public void testGitToolChooserWithBothGitAndJGit() throws Exception { GitTool jgitTool = new JGitTool(Collections.>emptyList()); jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(tool, jgitTool); - GitToolChooser gitToolChooser = new GitToolChooser(remote, context, credentialsId, tool.getGitExe(), true); + GitToolChooser gitToolChooser = new GitToolChooser(remote, context, credentialsId, tool, true); assertThat(gitToolChooser.getGitTool(), is("jgit")); } @@ -295,7 +321,7 @@ public void testGitToolChooserWithAllTools() throws Exception { GitTool jGitApacheTool = new JGitApacheTool(Collections.>emptyList()); jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(tool, jgitTool, jGitApacheTool); - GitToolChooser gitToolChooser = new GitToolChooser(remote, context, credentialsId, tool.getGitExe(), true); + GitToolChooser gitToolChooser = new GitToolChooser(remote, context, credentialsId, tool, true); assertThat(gitToolChooser.getGitTool(), is("jgit")); } @@ -314,7 +340,7 @@ public void testGitToolChooserWithJGitApache() throws Exception { GitTool jGitApacheTool = new JGitApacheTool(Collections.>emptyList()); jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(tool, jGitApacheTool); - GitToolChooser gitToolChooser = new GitToolChooser(remote, context, credentialsId, jGitApacheTool.getGitExe(), true); + GitToolChooser gitToolChooser = new GitToolChooser(remote, context, credentialsId, jGitApacheTool, true); assertThat(gitToolChooser.getGitTool(), is("jgitapache")); } @@ -331,7 +357,7 @@ public void testGitToolChooserWithJGitApacheAndGit() throws Exception { GitTool jGitApacheTool = new JGitApacheTool(Collections.>emptyList()); jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(jGitApacheTool); - GitToolChooser gitToolChooser = new GitToolChooser(remote, context, credentialsId, jGitApacheTool.getGitExe(), true); + GitToolChooser gitToolChooser = new GitToolChooser(remote, context, credentialsId, jGitApacheTool, true); assertThat(gitToolChooser.getGitTool(), is("jgitapache")); } @@ -350,9 +376,9 @@ public void testGitToolChooserWithDefaultTool() throws Exception { List list = jenkins.jenkins.getItems(); // Assuming no tool is installed and git is present in the machine - String gitExe = "git"; + GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); - GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", gitExe, true); + GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool, true); assertThat(sizeEstimator.getGitTool(), containsString("git")); } @@ -374,7 +400,7 @@ public void testGitToolChooserWithOnlyJGit() throws Exception { // Assuming no tool is installed and git is present in the machine String gitExe = jGitTool.getGitExe(); - GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", gitExe, true); + GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", jGitTool, true); assertThat(sizeEstimator.getGitTool(), is("jgit")); } @@ -396,7 +422,7 @@ public void testGitToolChooserWithCustomGitTool_2() throws Exception { // Assuming no tool is installed and git is present in the machine String gitExe = tool.getGitExe(); - GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", gitExe, true); + GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool, true); assertThat(sizeEstimator.getGitTool(), is(SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git")); } @@ -420,7 +446,7 @@ public void testGitToolChooserWithAllTools_2() throws Exception { // Assuming no tool is installed and git is present in the machine String gitExe = tool.getGitExe(); - GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", gitExe, true); + GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool, true); assertThat(sizeEstimator.getGitTool(), is(SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git")); } @@ -502,6 +528,26 @@ public Long getSizeOfRepository(String remote, Item context, String credentialsI } } + private static class HelloToolInstaller extends CommandInstaller { + + private boolean invoked; + + public HelloToolInstaller(String label, String command, String toolHome) { + super(label, command, toolHome); + } + + public boolean isInvoked() { + return invoked; + } + + @Override + public FilePath performInstallation(ToolInstallation toolInstallation, Node node, TaskListener taskListener) throws IOException, InterruptedException { + taskListener.error("Hello, world!"); + invoked = true; + return super.performInstallation(toolInstallation, node, taskListener); + } + } + private void buildAProject(GitSampleRepoRule sampleRepo, boolean noCredentials) throws Exception { WorkflowJob p = jenkins.jenkins.createProject(WorkflowJob.class, "p"); From 719f28478bbcf8e538860159c90825dd03d92cfb Mon Sep 17 00:00:00 2001 From: Rishabh Budhouliya Date: Fri, 4 Sep 2020 10:47:06 +0530 Subject: [PATCH 02/12] Introducing recommendGitToolOnAgent In reference to the bug JENKINS-63519 which exposed an issue where the GitToolChooser is not able to determine the correct implementation if a node-specific installation is present in the system. This API is called when the system wants to recommend `cli git` and the user has chosen `jgit`. To find the right cli git, it iterates over the available installations, checks if one of them contains a property map, confirms with the node attributes (using GitUtils.resolveGitTool) and then finally suggests the right git implementation. With this change, we have added two new parameters: Node path and TaskListener. Both of them are needed for better estimation of the correct git tool present on a particular agent. --- src/main/java/hudson/plugins/git/GitSCM.java | 5 +- .../jenkins/plugins/git/GitToolChooser.java | 96 +++++++++---------- .../plugins/git/GitToolChooserTest.java | 49 +++++----- 3 files changed, 72 insertions(+), 78 deletions(-) diff --git a/src/main/java/hudson/plugins/git/GitSCM.java b/src/main/java/hudson/plugins/git/GitSCM.java index c4b9e233f1..3416191aa2 100644 --- a/src/main/java/hudson/plugins/git/GitSCM.java +++ b/src/main/java/hudson/plugins/git/GitSCM.java @@ -849,7 +849,7 @@ public GitClient createClient(TaskListener listener, EnvVars environment, Run preferredToolList = new ArrayList<>(); + GitTool correctTool = null; + String toolName = userChoice.getName(); + if (toolName.equals(JGitTool.MAGIC_EXENAME) || toolName.equals(JGitApacheTool.MAGIC_EXENAME)) { + GitTool[] toolList = Jenkins.get().getDescriptorByType(GitTool.DescriptorImpl.class).getInstallations(); + for (GitTool tool : toolList) { + if (!tool.getProperties().isEmpty()) { + preferredToolList.add(tool); + } + } + for (GitTool tool: preferredToolList) { + if (tool.getName().equals(getResolvedGitTool(tool.getName()).getName())) { + correctTool = getResolvedGitTool(tool.getName()); } } } - return null; + return correctTool; + } + + private GitTool getResolvedGitTool(String recommendation) { + if (currentNode == null) { + currentNode = Jenkins.get(); + } + return GitUtils.resolveGitTool(recommendation, currentNode, null, listener); + } + + /** + * Recommend git tool to be used by the git client + * @return git implementation recommendation in the form of a string + */ + public String getGitTool() { + return gitTool; } /** diff --git a/src/test/java/jenkins/plugins/git/GitToolChooserTest.java b/src/test/java/jenkins/plugins/git/GitToolChooserTest.java index 631296f9b3..29133626e4 100644 --- a/src/test/java/jenkins/plugins/git/GitToolChooserTest.java +++ b/src/test/java/jenkins/plugins/git/GitToolChooserTest.java @@ -32,7 +32,6 @@ import java.io.File; import java.io.IOException; -import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -82,12 +81,12 @@ public void testResolveGitTool() throws IOException, InterruptedException { new InstallSourceProperty(Collections.singletonList(inst)))); GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); + GitTool JTool = new JGitTool(Collections.>emptyList()); + jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(JTool, tool, t); - jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(tool, t); + GitToolChooser r = new GitToolChooser(remote, context, credentialsId, JTool, null, TaskListener.NULL,true); - GitToolChooser r = new GitToolChooser(remote, context, credentialsId, t, true); - - System.out.println(r.getGitTool()); + assertThat(r.getGitTool().contains("default/git/updated/git"), is(true)); } /* @@ -109,12 +108,12 @@ public void testSizeEstimationWithNoGitCache() throws Exception { //Since no installation is provided, the gitExe will be git GitTool rTool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); - GitToolChooser repoSizeEstimator = new GitToolChooser(instance.getRemote(), list.get(0), "github", rTool, true); + GitToolChooser repoSizeEstimator = new GitToolChooser(instance.getRemote(), list.get(0), "github", rTool, null, TaskListener.NULL, true); String tool = repoSizeEstimator.getGitTool(); - // The class should make recommendation because of APIs implementation even though - // it can't find a .git cached directory - assertThat(tool, is(not("NONE"))); + // The class get a size < 5M from APIs and wants to recommend `jgit` but will return NONE instead + // as `jgit` is not enabled by the user + assertThat(tool, is("NONE")); // If size were reported as 0, should return NONE assertThat(repoSizeEstimator.determineSwitchOnSize(0L, rTool), is("NONE")); @@ -156,7 +155,7 @@ public void testSizeEstimationWithGitCache() throws Exception { List list = jenkins.jenkins.getItems(); - GitToolChooser repoSizeEstimator = new GitToolChooser(source.getRemote(), list.get(0), "github", tool, true); + GitToolChooser repoSizeEstimator = new GitToolChooser(source.getRemote(), list.get(0), "github", tool, null,TaskListener.NULL,true); /* Since the size of repository is 21.785 KiBs, the estimator should suggest "jgit" as an implementation */ @@ -181,7 +180,7 @@ public void testSizeEstimationWithAPIForGit() throws Exception { GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); - GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool, true); + GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool, null, TaskListener.NULL,true); assertThat(sizeEstimator.getGitTool(), containsString("git")); } @@ -204,7 +203,7 @@ public void testSizeEstimationWithAPIForJGit() throws Exception { buildAProject(sampleRepo, false); List list = jenkins.jenkins.getItems(); - GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool, true); + GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool, null, TaskListener.NULL,true); assertThat(sizeEstimator.getGitTool(), containsString("jgit")); } @@ -224,7 +223,7 @@ public void testSizeEstimationWithBitbucketAPIs() throws Exception { // Assuming no tool is installed by user and git is present in the machine GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); - GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool,true); + GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool,null, TaskListener.NULL,true); assertThat(sizeEstimator.getGitTool(), is("NONE")); } @@ -245,7 +244,7 @@ public void testSizeEstimationWithException() throws Exception { // Assuming no tool is installed by user and git is present in the machine GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); - GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool, true); + GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool,null, TaskListener.NULL,true); assertThat(sizeEstimator.getGitTool(), is("NONE")); } @@ -264,7 +263,7 @@ public void testSizeEstimationWithNoCredentials() throws Exception { // Assuming no tool is installed by user and git is present in the machine GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); - GitToolChooser sizeEstimator = new GitToolChooser(sampleRepo.toString(), list.get(0), null, tool, true); + GitToolChooser sizeEstimator = new GitToolChooser(sampleRepo.toString(), list.get(0), null, tool, null, TaskListener.NULL,true); assertThat(sizeEstimator.getGitTool(), is("NONE")); } @@ -283,7 +282,7 @@ public void testGitToolChooserWithCustomGitTool() throws Exception { GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(tool); - GitToolChooser gitToolChooser = new GitToolChooser(remote, context, credentialsId, tool, true); + GitToolChooser gitToolChooser = new GitToolChooser(remote, context, credentialsId, tool, null, TaskListener.NULL,true); //According to size of repo, "jgit" should be recommended but it is not installed by the user //Hence, in this case GitToolChooser should return a NONE @@ -302,7 +301,7 @@ public void testGitToolChooserWithBothGitAndJGit() throws Exception { GitTool jgitTool = new JGitTool(Collections.>emptyList()); jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(tool, jgitTool); - GitToolChooser gitToolChooser = new GitToolChooser(remote, context, credentialsId, tool, true); + GitToolChooser gitToolChooser = new GitToolChooser(remote, context, credentialsId, tool, null, TaskListener.NULL,true); assertThat(gitToolChooser.getGitTool(), is("jgit")); } @@ -321,7 +320,7 @@ public void testGitToolChooserWithAllTools() throws Exception { GitTool jGitApacheTool = new JGitApacheTool(Collections.>emptyList()); jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(tool, jgitTool, jGitApacheTool); - GitToolChooser gitToolChooser = new GitToolChooser(remote, context, credentialsId, tool, true); + GitToolChooser gitToolChooser = new GitToolChooser(remote, context, credentialsId, tool, null, TaskListener.NULL,true); assertThat(gitToolChooser.getGitTool(), is("jgit")); } @@ -340,7 +339,7 @@ public void testGitToolChooserWithJGitApache() throws Exception { GitTool jGitApacheTool = new JGitApacheTool(Collections.>emptyList()); jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(tool, jGitApacheTool); - GitToolChooser gitToolChooser = new GitToolChooser(remote, context, credentialsId, jGitApacheTool, true); + GitToolChooser gitToolChooser = new GitToolChooser(remote, context, credentialsId, jGitApacheTool, null, TaskListener.NULL,true); assertThat(gitToolChooser.getGitTool(), is("jgitapache")); } @@ -357,7 +356,7 @@ public void testGitToolChooserWithJGitApacheAndGit() throws Exception { GitTool jGitApacheTool = new JGitApacheTool(Collections.>emptyList()); jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(jGitApacheTool); - GitToolChooser gitToolChooser = new GitToolChooser(remote, context, credentialsId, jGitApacheTool, true); + GitToolChooser gitToolChooser = new GitToolChooser(remote, context, credentialsId, jGitApacheTool, null, TaskListener.NULL,true); assertThat(gitToolChooser.getGitTool(), is("jgitapache")); } @@ -378,7 +377,7 @@ public void testGitToolChooserWithDefaultTool() throws Exception { // Assuming no tool is installed and git is present in the machine GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); - GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool, true); + GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool, null, TaskListener.NULL,true); assertThat(sizeEstimator.getGitTool(), containsString("git")); } @@ -400,8 +399,8 @@ public void testGitToolChooserWithOnlyJGit() throws Exception { // Assuming no tool is installed and git is present in the machine String gitExe = jGitTool.getGitExe(); - GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", jGitTool, true); - assertThat(sizeEstimator.getGitTool(), is("jgit")); + GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", jGitTool, null, TaskListener.NULL,true); + assertThat(sizeEstimator.getGitTool(), is("NONE")); // Since git is not available and that should be the recommendation } @Test @@ -422,7 +421,7 @@ public void testGitToolChooserWithCustomGitTool_2() throws Exception { // Assuming no tool is installed and git is present in the machine String gitExe = tool.getGitExe(); - GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool, true); + GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool, null, TaskListener.NULL,true); assertThat(sizeEstimator.getGitTool(), is(SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git")); } @@ -446,7 +445,7 @@ public void testGitToolChooserWithAllTools_2() throws Exception { // Assuming no tool is installed and git is present in the machine String gitExe = tool.getGitExe(); - GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool, true); + GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool, null, TaskListener.NULL,true); assertThat(sizeEstimator.getGitTool(), is(SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git")); } From 1ceed4d65795ddcc5782b2b346745d683f87e2a3 Mon Sep 17 00:00:00 2001 From: Rishabh Budhouliya Date: Fri, 4 Sep 2020 12:10:09 +0530 Subject: [PATCH 03/12] Add better assertion for unit test --- src/test/java/jenkins/plugins/git/GitToolChooserTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/jenkins/plugins/git/GitToolChooserTest.java b/src/test/java/jenkins/plugins/git/GitToolChooserTest.java index 29133626e4..4414356b30 100644 --- a/src/test/java/jenkins/plugins/git/GitToolChooserTest.java +++ b/src/test/java/jenkins/plugins/git/GitToolChooserTest.java @@ -76,8 +76,8 @@ public void testResolveGitTool() throws IOException, InterruptedException { Item context = Mockito.mock(Item.class); String credentialsId = null; - HelloToolInstaller inst = new HelloToolInstaller("master", "echo Hello", "updated/git"); - GitTool t = new GitTool("myGit", "default/git", Collections.singletonList( + HelloToolInstaller inst = new HelloToolInstaller("master", "echo Hello", SystemUtils.IS_OS_WINDOWS ? "updated/git.exe" : "updated/git"); + GitTool t = new GitTool("myGit", SystemUtils.IS_OS_WINDOWS ? "default/git.exe" : "default/git", Collections.singletonList( new InstallSourceProperty(Collections.singletonList(inst)))); GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); @@ -86,7 +86,7 @@ public void testResolveGitTool() throws IOException, InterruptedException { GitToolChooser r = new GitToolChooser(remote, context, credentialsId, JTool, null, TaskListener.NULL,true); - assertThat(r.getGitTool().contains("default/git/updated/git"), is(true)); + assertThat(r.getGitTool(), containsString(SystemUtils.IS_OS_WINDOWS ? "default/git.exe/updated/git.exe" : "default/git/updated/git")); } /* From 26eb95ed510d4d8eda52b53e732aa4324dac939e Mon Sep 17 00:00:00 2001 From: Rishabh Budhouliya Date: Sat, 5 Sep 2020 02:07:27 +0530 Subject: [PATCH 04/12] Add more tests to describe the behavior of GitToolChooser if user's choice is JGit and the size of repo> 5M For the GitToolChooser, the most difficult scenario is when the user has chosen jgit and size of repo > 5M. In this case, the system has to decide which command line git implementation will be the right implementation for that particular environment without breaking any possible use-case. With the added test cases what we want to say is (keeping the above mentioned conditions same): - if there is a node specific installation present, that will be suggested after being verified that it is actually installed in that environment. - if there is no node specific installation, the default implementation will be suggested. There is one side-effect of this implementation, if the only implementation is `jgit` in the system then that will be recommended instead of command line git. But that wouldn't affect the user since there would be no possible change in the git implementation used for the git client. --- .../jenkins/plugins/git/GitToolChooser.java | 2 +- .../plugins/git/GitToolChooserTest.java | 85 +++++++++++++++++-- 2 files changed, 77 insertions(+), 10 deletions(-) diff --git a/src/main/java/jenkins/plugins/git/GitToolChooser.java b/src/main/java/jenkins/plugins/git/GitToolChooser.java index 2c090feb7f..576065f564 100644 --- a/src/main/java/jenkins/plugins/git/GitToolChooser.java +++ b/src/main/java/jenkins/plugins/git/GitToolChooser.java @@ -183,7 +183,7 @@ private GitTool resolveGitToolForRecommendation(GitTool userChoice, String recom public GitTool recommendGitToolOnAgent(GitTool userChoice) { List preferredToolList = new ArrayList<>(); - GitTool correctTool = null; + GitTool correctTool = GitTool.getDefaultInstallation(); String toolName = userChoice.getName(); if (toolName.equals(JGitTool.MAGIC_EXENAME) || toolName.equals(JGitApacheTool.MAGIC_EXENAME)) { GitTool[] toolList = Jenkins.get().getDescriptorByType(GitTool.DescriptorImpl.class).getInstallations(); diff --git a/src/test/java/jenkins/plugins/git/GitToolChooserTest.java b/src/test/java/jenkins/plugins/git/GitToolChooserTest.java index 4414356b30..93aa343fa4 100644 --- a/src/test/java/jenkins/plugins/git/GitToolChooserTest.java +++ b/src/test/java/jenkins/plugins/git/GitToolChooserTest.java @@ -6,11 +6,10 @@ import com.cloudbees.plugins.credentials.impl.UsernamePasswordCredentialsImpl; import hudson.FilePath; import hudson.model.*; +import hudson.model.labels.LabelAtom; import hudson.plugins.git.GitTool; -import hudson.tools.CommandInstaller; -import hudson.tools.InstallSourceProperty; -import hudson.tools.ToolInstallation; -import hudson.tools.ToolProperty; +import hudson.slaves.DumbSlave; +import hudson.tools.*; import hudson.util.StreamTaskListener; import jenkins.model.Jenkins; import jenkins.plugins.git.traits.BranchDiscoveryTrait; @@ -69,6 +68,10 @@ public void enableSystemCredentialsProvider() { assertThat("The system credentials provider is enabled", store, notNullValue()); } + /* + This test checks the GitToolChooser in a scenario where repo size>5M, user's choice is `jgit`. + In the event of having no node-specific installations, GitToolChooser will choose to return the default installation. + */ @Issue("JENKINS-63519") @Test public void testResolveGitTool() throws IOException, InterruptedException { @@ -76,17 +79,81 @@ public void testResolveGitTool() throws IOException, InterruptedException { Item context = Mockito.mock(Item.class); String credentialsId = null; - HelloToolInstaller inst = new HelloToolInstaller("master", "echo Hello", SystemUtils.IS_OS_WINDOWS ? "updated/git.exe" : "updated/git"); + GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); + GitTool JTool = new JGitTool(Collections.>emptyList()); + jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(tool, JTool); + + GitToolChooser r = new GitToolChooser(remote, context, credentialsId, JTool, null, TaskListener.NULL,true); + + assertThat(r.getGitTool(), containsString(SystemUtils.IS_OS_WINDOWS ? "updated/git.exe" : "updated/git")); + } + + /* + This test checks the GitToolChooser in a scenario where repo size>5M, user's choice is `jgit`. + There is no specific agent(node=null). In this case agent = Jenkins.get(). + In the event of running the GitToolChooser on the agent, it should correctly predict the git installation for + that specific agent. + */ + @Issue("JENKINS-63519") + @Test + public void testResolveGitToolWithJenkins() throws IOException, InterruptedException { + String remote = "https://gitlab.com/rishabhBudhouliya/git-plugin.git"; + Item context = Mockito.mock(Item.class); + String credentialsId = null; + + TestToolInstaller inst = new TestToolInstaller("master", "echo Hello", SystemUtils.IS_OS_WINDOWS ? "updated/git.exe" : "updated/git"); GitTool t = new GitTool("myGit", SystemUtils.IS_OS_WINDOWS ? "default/git.exe" : "default/git", Collections.singletonList( new InstallSourceProperty(Collections.singletonList(inst)))); GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); GitTool JTool = new JGitTool(Collections.>emptyList()); - jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(JTool, tool, t); + jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(tool, JTool, t); GitToolChooser r = new GitToolChooser(remote, context, credentialsId, JTool, null, TaskListener.NULL,true); - assertThat(r.getGitTool(), containsString(SystemUtils.IS_OS_WINDOWS ? "default/git.exe/updated/git.exe" : "default/git/updated/git")); + assertThat(r.getGitTool(), containsString(SystemUtils.IS_OS_WINDOWS ? "updated/git.exe" : "updated/git")); + } + + /* + This test checks the GitToolChooser in a scenario where repo size>5M, user's choice is `jgit`. + There is an agent labeled -> `agent-windows` + In the event of running the GitToolChooser on the agent, it should correctly predict the git installation for + that specific agent. + */ + @Issue("JENKINS-63519") + @Test + public void testResolutionGitToolOnAgent() throws Exception { + String remote = "https://gitlab.com/rishabhBudhouliya/git-plugin.git"; + Item context = Mockito.mock(Item.class); + String credentialsId = null; + + LabelAtom label = new LabelAtom("agent-windows"); + DumbSlave agent = jenkins.createOnlineSlave(label); + agent.setMode(Node.Mode.NORMAL); + agent.setLabelString("agent-windows"); + + TestToolInstaller inst = new TestToolInstaller("master", "echo Hello", SystemUtils.IS_OS_WINDOWS ? "myGit/git.exe" : "myGit/git"); + GitTool toolOnMaster = new GitTool("myGit", SystemUtils.IS_OS_WINDOWS ? "default/git.exe" : "default/git", Collections.singletonList( + new InstallSourceProperty(Collections.singletonList(inst)))); + + TestToolInstaller instonAgent = new TestToolInstaller("agent-windows", "echo Hello", SystemUtils.IS_OS_WINDOWS ? "my-git/git.exe" : "my-git/git"); + GitTool toolOnAgent = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.singletonList(new InstallSourceProperty(Collections.singletonList(instonAgent)))); + + GitTool JTool = new JGitTool(Collections.>emptyList()); + + jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(toolOnMaster, toolOnAgent, JTool); + agent.getNodeProperties().add(new ToolLocationNodeProperty(new ToolLocationNodeProperty.ToolLocation( + jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class), toolOnMaster.getName(), toolOnMaster.getHome()))); + + agent.getNodeProperties().add(new ToolLocationNodeProperty(new ToolLocationNodeProperty.ToolLocation( + jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class), toolOnAgent.getName(), toolOnAgent.getHome()))); + + agent.getNodeProperties().add(new ToolLocationNodeProperty(new ToolLocationNodeProperty.ToolLocation( + jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class), JTool.getName(), null))); + + GitToolChooser r = new GitToolChooser(remote, context, credentialsId, JTool, agent, TaskListener.NULL,true); + + assertThat(r.getGitTool(), containsString(SystemUtils.IS_OS_WINDOWS ? "my-git/git.exe" : "my-git/git")); } /* @@ -527,11 +594,11 @@ public Long getSizeOfRepository(String remote, Item context, String credentialsI } } - private static class HelloToolInstaller extends CommandInstaller { + private static class TestToolInstaller extends CommandInstaller { private boolean invoked; - public HelloToolInstaller(String label, String command, String toolHome) { + public TestToolInstaller(String label, String command, String toolHome) { super(label, command, toolHome); } From 193b7e5f6521b6957b225e513cbc461ca90466d8 Mon Sep 17 00:00:00 2001 From: Rishabh Budhouliya Date: Sat, 5 Sep 2020 13:50:15 +0530 Subject: [PATCH 05/12] Remove assertion of NONE to jgit for testGitToolWithOnlyJGit --- src/test/java/jenkins/plugins/git/GitToolChooserTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/jenkins/plugins/git/GitToolChooserTest.java b/src/test/java/jenkins/plugins/git/GitToolChooserTest.java index 93aa343fa4..d6e7525feb 100644 --- a/src/test/java/jenkins/plugins/git/GitToolChooserTest.java +++ b/src/test/java/jenkins/plugins/git/GitToolChooserTest.java @@ -467,7 +467,7 @@ public void testGitToolChooserWithOnlyJGit() throws Exception { String gitExe = jGitTool.getGitExe(); GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", jGitTool, null, TaskListener.NULL,true); - assertThat(sizeEstimator.getGitTool(), is("NONE")); // Since git is not available and that should be the recommendation + assertThat(sizeEstimator.getGitTool(), is("jgit")); // Since git is not available, we suggest `jgit` which doesn't make any difference } @Test From 34093c333178c1cfb6fee4db3dad12d22c3d1dd6 Mon Sep 17 00:00:00 2001 From: Rishabh Budhouliya Date: Sat, 5 Sep 2020 15:40:43 +0530 Subject: [PATCH 06/12] Fix another test assertion statement --- src/test/java/jenkins/plugins/git/GitToolChooserTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/jenkins/plugins/git/GitToolChooserTest.java b/src/test/java/jenkins/plugins/git/GitToolChooserTest.java index d6e7525feb..e6969af3ca 100644 --- a/src/test/java/jenkins/plugins/git/GitToolChooserTest.java +++ b/src/test/java/jenkins/plugins/git/GitToolChooserTest.java @@ -85,7 +85,7 @@ public void testResolveGitTool() throws IOException, InterruptedException { GitToolChooser r = new GitToolChooser(remote, context, credentialsId, JTool, null, TaskListener.NULL,true); - assertThat(r.getGitTool(), containsString(SystemUtils.IS_OS_WINDOWS ? "updated/git.exe" : "updated/git")); + assertThat(r.getGitTool(), containsString(SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git")); } /* From 25239d11fa0c9fe7f4550a864f5df24e184890d5 Mon Sep 17 00:00:00 2001 From: Rishabh Budhouliya Date: Sat, 5 Sep 2020 20:04:29 +0530 Subject: [PATCH 07/12] Remove unreachable check condition in case of command line git is recommended --- src/main/java/jenkins/plugins/git/GitToolChooser.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/main/java/jenkins/plugins/git/GitToolChooser.java b/src/main/java/jenkins/plugins/git/GitToolChooser.java index 576065f564..82034d1984 100644 --- a/src/main/java/jenkins/plugins/git/GitToolChooser.java +++ b/src/main/java/jenkins/plugins/git/GitToolChooser.java @@ -149,9 +149,6 @@ String determineSwitchOnSize(Long sizeOfRepo, GitTool tool) { return rTool.getGitExe(); } else { GitTool rTool = resolveGitToolForRecommendation(tool, "git"); - if (rTool == null) { - return "NONE"; - } return rTool.getGitExe(); } } From 43c3094124d679b35b1e67f27670c876e94c852b Mon Sep 17 00:00:00 2001 From: Rishabh Budhouliya Date: Sat, 5 Sep 2020 20:07:46 +0530 Subject: [PATCH 08/12] Add missing params for javadoc --- src/main/java/jenkins/plugins/git/GitToolChooser.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/jenkins/plugins/git/GitToolChooser.java b/src/main/java/jenkins/plugins/git/GitToolChooser.java index 82034d1984..0de52b95e7 100644 --- a/src/main/java/jenkins/plugins/git/GitToolChooser.java +++ b/src/main/java/jenkins/plugins/git/GitToolChooser.java @@ -47,7 +47,9 @@ public class GitToolChooser { * @param remoteName the repository url * @param projectContext the context where repository size is being estimated * @param credentialsId credential used to access the repository or null if no credential is required - * @param gitExe name of the git tool ('git', 'jgit', 'jgitapache') to be used as the default tool + * @param gitExe Git tool ('git', 'jgit', 'jgitapache') to be used as the default tool + * @param n A Jenkins agent used to check validity of git installation + * @param listener TaskListener required by GitUtils.resolveGitTool() * @param useJGit if true the JGit is allowed as an implementation * @throws IOException on error * @throws InterruptedException on error From 7bc2b8499c5d8588cd13c4952ca3b1d493ff4843 Mon Sep 17 00:00:00 2001 From: Rishabh Budhouliya Date: Mon, 7 Sep 2020 11:32:58 +0530 Subject: [PATCH 09/12] Fix Windows specific tests for git tool, use isWindows() Use isWindows() technique to maintain consistency across the plugin --- .../plugins/git/GitToolChooserTest.java | 51 ++++++++++--------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/src/test/java/jenkins/plugins/git/GitToolChooserTest.java b/src/test/java/jenkins/plugins/git/GitToolChooserTest.java index e6969af3ca..b05f1d84d1 100644 --- a/src/test/java/jenkins/plugins/git/GitToolChooserTest.java +++ b/src/test/java/jenkins/plugins/git/GitToolChooserTest.java @@ -15,7 +15,6 @@ import jenkins.plugins.git.traits.BranchDiscoveryTrait; import jenkins.scm.api.trait.SCMSourceTrait; -import org.apache.commons.lang.SystemUtils; import org.jenkinsci.plugins.gitclient.JGitApacheTool; import org.jenkinsci.plugins.gitclient.JGitTool; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; @@ -79,13 +78,13 @@ public void testResolveGitTool() throws IOException, InterruptedException { Item context = Mockito.mock(Item.class); String credentialsId = null; - GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); + GitTool tool = new GitTool("my-git", isWindows() ? "git.exe" : "git", Collections.>emptyList()); GitTool JTool = new JGitTool(Collections.>emptyList()); jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(tool, JTool); GitToolChooser r = new GitToolChooser(remote, context, credentialsId, JTool, null, TaskListener.NULL,true); - assertThat(r.getGitTool(), containsString(SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git")); + assertThat(r.getGitTool(), containsString(isWindows() ? "git.exe" : "git")); } /* @@ -101,17 +100,17 @@ public void testResolveGitToolWithJenkins() throws IOException, InterruptedExcep Item context = Mockito.mock(Item.class); String credentialsId = null; - TestToolInstaller inst = new TestToolInstaller("master", "echo Hello", SystemUtils.IS_OS_WINDOWS ? "updated/git.exe" : "updated/git"); - GitTool t = new GitTool("myGit", SystemUtils.IS_OS_WINDOWS ? "default/git.exe" : "default/git", Collections.singletonList( + TestToolInstaller inst = new TestToolInstaller("master", "echo Hello", isWindows() ? "updated/git.exe" : "updated/git"); + GitTool t = new GitTool("myGit", isWindows() ? "default/git.exe" : "default/git", Collections.singletonList( new InstallSourceProperty(Collections.singletonList(inst)))); - GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); + GitTool tool = new GitTool("my-git", isWindows() ? "git.exe" : "git", Collections.>emptyList()); GitTool JTool = new JGitTool(Collections.>emptyList()); jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(tool, JTool, t); GitToolChooser r = new GitToolChooser(remote, context, credentialsId, JTool, null, TaskListener.NULL,true); - assertThat(r.getGitTool(), containsString(SystemUtils.IS_OS_WINDOWS ? "updated/git.exe" : "updated/git")); + assertThat(r.getGitTool(), containsString(isWindows() ? "updated/git.exe" : "updated/git")); } /* @@ -132,12 +131,12 @@ public void testResolutionGitToolOnAgent() throws Exception { agent.setMode(Node.Mode.NORMAL); agent.setLabelString("agent-windows"); - TestToolInstaller inst = new TestToolInstaller("master", "echo Hello", SystemUtils.IS_OS_WINDOWS ? "myGit/git.exe" : "myGit/git"); - GitTool toolOnMaster = new GitTool("myGit", SystemUtils.IS_OS_WINDOWS ? "default/git.exe" : "default/git", Collections.singletonList( + TestToolInstaller inst = new TestToolInstaller("master", "echo Hello", isWindows() ? "myGit/git.exe" : "myGit/git"); + GitTool toolOnMaster = new GitTool("myGit", isWindows() ? "default/git.exe" : "default/git", Collections.singletonList( new InstallSourceProperty(Collections.singletonList(inst)))); - TestToolInstaller instonAgent = new TestToolInstaller("agent-windows", "echo Hello", SystemUtils.IS_OS_WINDOWS ? "my-git/git.exe" : "my-git/git"); - GitTool toolOnAgent = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.singletonList(new InstallSourceProperty(Collections.singletonList(instonAgent)))); + TestToolInstaller instonAgent = new TestToolInstaller("agent-windows", "echo Hello", isWindows() ? "my-git/git.exe" : "my-git/git"); + GitTool toolOnAgent = new GitTool("my-git", isWindows() ? "git.exe" : "git", Collections.singletonList(new InstallSourceProperty(Collections.singletonList(instonAgent)))); GitTool JTool = new JGitTool(Collections.>emptyList()); @@ -153,7 +152,7 @@ public void testResolutionGitToolOnAgent() throws Exception { GitToolChooser r = new GitToolChooser(remote, context, credentialsId, JTool, agent, TaskListener.NULL,true); - assertThat(r.getGitTool(), containsString(SystemUtils.IS_OS_WINDOWS ? "my-git/git.exe" : "my-git/git")); + assertThat(r.getGitTool(), containsString(isWindows() ? "my-git/git.exe" : "my-git/git")); } /* @@ -173,7 +172,7 @@ public void testSizeEstimationWithNoGitCache() throws Exception { List list = jenkins.jenkins.getItems(); //Since no installation is provided, the gitExe will be git - GitTool rTool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); + GitTool rTool = new GitTool("my-git", isWindows() ? "git.exe" : "git", Collections.>emptyList()); GitToolChooser repoSizeEstimator = new GitToolChooser(instance.getRemote(), list.get(0), "github", rTool, null, TaskListener.NULL, true); String tool = repoSizeEstimator.getGitTool(); @@ -244,7 +243,7 @@ public void testSizeEstimationWithAPIForGit() throws Exception { List list = jenkins.jenkins.getItems(); // Assuming no tool is installed and git is present in the machine - GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); + GitTool tool = new GitTool("my-git", isWindows() ? "git.exe" : "git", Collections.>emptyList()); GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool, null, TaskListener.NULL,true); @@ -288,7 +287,7 @@ public void testSizeEstimationWithBitbucketAPIs() throws Exception { List list = jenkins.jenkins.getItems(); // Assuming no tool is installed by user and git is present in the machine - GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); + GitTool tool = new GitTool("my-git", isWindows() ? "git.exe" : "git", Collections.>emptyList()); GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool,null, TaskListener.NULL,true); assertThat(sizeEstimator.getGitTool(), is("NONE")); @@ -309,7 +308,7 @@ public void testSizeEstimationWithException() throws Exception { List list = jenkins.jenkins.getItems(); // Assuming no tool is installed by user and git is present in the machine - GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); + GitTool tool = new GitTool("my-git", isWindows() ? "git.exe" : "git", Collections.>emptyList()); GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool,null, TaskListener.NULL,true); @@ -328,7 +327,7 @@ public void testSizeEstimationWithNoCredentials() throws Exception { List list = jenkins.jenkins.getItems(); // Assuming no tool is installed by user and git is present in the machine - GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); + GitTool tool = new GitTool("my-git", isWindows() ? "git.exe" : "git", Collections.>emptyList()); GitToolChooser sizeEstimator = new GitToolChooser(sampleRepo.toString(), list.get(0), null, tool, null, TaskListener.NULL,true); @@ -346,7 +345,7 @@ public void testGitToolChooserWithCustomGitTool() throws Exception { String credentialsId = null; // With JGit, we don't ask the name and home of the tool - GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); + GitTool tool = new GitTool("my-git", isWindows() ? "git.exe" : "git", Collections.>emptyList()); jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(tool); GitToolChooser gitToolChooser = new GitToolChooser(remote, context, credentialsId, tool, null, TaskListener.NULL,true); @@ -364,7 +363,7 @@ public void testGitToolChooserWithBothGitAndJGit() throws Exception { String credentialsId = null; // With JGit, we don't ask the name and home of the tool - GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); + GitTool tool = new GitTool("my-git", isWindows() ? "git.exe" : "git", Collections.>emptyList()); GitTool jgitTool = new JGitTool(Collections.>emptyList()); jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(tool, jgitTool); @@ -442,7 +441,7 @@ public void testGitToolChooserWithDefaultTool() throws Exception { List list = jenkins.jenkins.getItems(); // Assuming no tool is installed and git is present in the machine - GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); + GitTool tool = new GitTool("my-git", isWindows() ? "git.exe" : "git", Collections.>emptyList()); GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool, null, TaskListener.NULL,true); assertThat(sizeEstimator.getGitTool(), containsString("git")); @@ -478,7 +477,7 @@ public void testGitToolChooserWithCustomGitTool_2() throws Exception { store.save(); // With JGit, we don't ask the name and home of the tool - GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); + GitTool tool = new GitTool("my-git", isWindows() ? "git.exe" : "git", Collections.>emptyList()); jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(tool); buildAProject(sampleRepo, false); @@ -489,7 +488,7 @@ public void testGitToolChooserWithCustomGitTool_2() throws Exception { String gitExe = tool.getGitExe(); GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool, null, TaskListener.NULL,true); - assertThat(sizeEstimator.getGitTool(), is(SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git")); + assertThat(sizeEstimator.getGitTool(), is(isWindows() ? "git.exe" : "git")); } @Test @@ -500,7 +499,7 @@ public void testGitToolChooserWithAllTools_2() throws Exception { store.save(); // With JGit, we don't ask the name and home of the tool - GitTool tool = new GitTool("my-git", SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git", Collections.>emptyList()); + GitTool tool = new GitTool("my-git", isWindows() ? "git.exe" : "git", Collections.>emptyList()); GitTool jgitTool = new JGitTool(Collections.>emptyList()); GitTool jGitApacheTool = new JGitApacheTool(Collections.>emptyList()); jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(tool, jgitTool, jGitApacheTool); @@ -513,7 +512,7 @@ public void testGitToolChooserWithAllTools_2() throws Exception { String gitExe = tool.getGitExe(); GitToolChooser sizeEstimator = new GitToolChooser(remote, list.get(0), "github", tool, null, TaskListener.NULL,true); - assertThat(sizeEstimator.getGitTool(), is(SystemUtils.IS_OS_WINDOWS ? "git.exe" : "git")); + assertThat(sizeEstimator.getGitTool(), is(isWindows() ? "git.exe" : "git")); } @Test @@ -634,4 +633,8 @@ private StandardCredentials createCredential(CredentialsScope scope, String id) return new UsernamePasswordCredentialsImpl(scope, id, "desc: " + id, "username", "password"); } + /** inline ${@link hudson.Functions#isWindows()} to prevent a transient remote classloader issue */ + private boolean isWindows() { + return File.pathSeparatorChar==';'; + } } From cfa06614b959da06a59eacc3aba28fd539521c7c Mon Sep 17 00:00:00 2001 From: Rishabh Budhouliya Date: Mon, 7 Sep 2020 11:46:09 +0530 Subject: [PATCH 10/12] Fix Windows specific paths --- .../jenkins/plugins/git/GitToolChooserTest.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/test/java/jenkins/plugins/git/GitToolChooserTest.java b/src/test/java/jenkins/plugins/git/GitToolChooserTest.java index b05f1d84d1..72101300d6 100644 --- a/src/test/java/jenkins/plugins/git/GitToolChooserTest.java +++ b/src/test/java/jenkins/plugins/git/GitToolChooserTest.java @@ -100,8 +100,8 @@ public void testResolveGitToolWithJenkins() throws IOException, InterruptedExcep Item context = Mockito.mock(Item.class); String credentialsId = null; - TestToolInstaller inst = new TestToolInstaller("master", "echo Hello", isWindows() ? "updated/git.exe" : "updated/git"); - GitTool t = new GitTool("myGit", isWindows() ? "default/git.exe" : "default/git", Collections.singletonList( + TestToolInstaller inst = new TestToolInstaller("master", "echo Hello", isWindows() ? "updated\\git.exe" : "updated/git"); + GitTool t = new GitTool("myGit", isWindows() ? "default\\git.exe" : "default/git", Collections.singletonList( new InstallSourceProperty(Collections.singletonList(inst)))); GitTool tool = new GitTool("my-git", isWindows() ? "git.exe" : "git", Collections.>emptyList()); @@ -110,7 +110,7 @@ public void testResolveGitToolWithJenkins() throws IOException, InterruptedExcep GitToolChooser r = new GitToolChooser(remote, context, credentialsId, JTool, null, TaskListener.NULL,true); - assertThat(r.getGitTool(), containsString(isWindows() ? "updated/git.exe" : "updated/git")); + assertThat(r.getGitTool(), containsString(isWindows() ? "updated\\git.exe" : "updated/git")); } /* @@ -131,11 +131,11 @@ public void testResolutionGitToolOnAgent() throws Exception { agent.setMode(Node.Mode.NORMAL); agent.setLabelString("agent-windows"); - TestToolInstaller inst = new TestToolInstaller("master", "echo Hello", isWindows() ? "myGit/git.exe" : "myGit/git"); - GitTool toolOnMaster = new GitTool("myGit", isWindows() ? "default/git.exe" : "default/git", Collections.singletonList( + TestToolInstaller inst = new TestToolInstaller("master", "echo Hello", isWindows() ? "myGit\\git.exe" : "myGit/git"); + GitTool toolOnMaster = new GitTool("myGit", isWindows() ? "default\\git.exe" : "default/git", Collections.singletonList( new InstallSourceProperty(Collections.singletonList(inst)))); - TestToolInstaller instonAgent = new TestToolInstaller("agent-windows", "echo Hello", isWindows() ? "my-git/git.exe" : "my-git/git"); + TestToolInstaller instonAgent = new TestToolInstaller("agent-windows", "echo Hello", isWindows() ? "my-git\\git.exe" : "my-git/git"); GitTool toolOnAgent = new GitTool("my-git", isWindows() ? "git.exe" : "git", Collections.singletonList(new InstallSourceProperty(Collections.singletonList(instonAgent)))); GitTool JTool = new JGitTool(Collections.>emptyList()); @@ -152,7 +152,7 @@ public void testResolutionGitToolOnAgent() throws Exception { GitToolChooser r = new GitToolChooser(remote, context, credentialsId, JTool, agent, TaskListener.NULL,true); - assertThat(r.getGitTool(), containsString(isWindows() ? "my-git/git.exe" : "my-git/git")); + assertThat(r.getGitTool(), containsString(isWindows() ? "my-git\\git.exe" : "my-git/git")); } /* From 2f985e351f5b2799fc388410091ae24846965c02 Mon Sep 17 00:00:00 2001 From: Rishabh Budhouliya Date: Mon, 7 Sep 2020 12:01:41 +0530 Subject: [PATCH 11/12] Add javadoc for getResolvedGitTool --- src/main/java/jenkins/plugins/git/GitToolChooser.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/jenkins/plugins/git/GitToolChooser.java b/src/main/java/jenkins/plugins/git/GitToolChooser.java index 0de52b95e7..79be4eb52e 100644 --- a/src/main/java/jenkins/plugins/git/GitToolChooser.java +++ b/src/main/java/jenkins/plugins/git/GitToolChooser.java @@ -200,6 +200,11 @@ public GitTool recommendGitToolOnAgent(GitTool userChoice) { return correctTool; } + /** + * Provide a git tool considering the node specific installations + * @param recommendation: Tool name + * @return resolved git tool + */ private GitTool getResolvedGitTool(String recommendation) { if (currentNode == null) { currentNode = Jenkins.get(); From 34a146d7fa38b3282fa473eec062abfcc45fc789 Mon Sep 17 00:00:00 2001 From: Rishabh Budhouliya Date: Tue, 8 Sep 2020 23:31:39 +0530 Subject: [PATCH 12/12] Ignore platform specific test cases on Windows Two of the test cases contains a node-specific tool installation implementation which can perform an installation only on Unix based platforms. Considering that shortcoming this change limits those tests to Unix based platforms only. --- .../plugins/git/GitToolChooserTest.java | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/test/java/jenkins/plugins/git/GitToolChooserTest.java b/src/test/java/jenkins/plugins/git/GitToolChooserTest.java index 72101300d6..5d36a05863 100644 --- a/src/test/java/jenkins/plugins/git/GitToolChooserTest.java +++ b/src/test/java/jenkins/plugins/git/GitToolChooserTest.java @@ -96,21 +96,25 @@ There is no specific agent(node=null). In this case agent = Jenkins.get(). @Issue("JENKINS-63519") @Test public void testResolveGitToolWithJenkins() throws IOException, InterruptedException { + if (isWindows()) { // Runs on Unix only + /* Do not distract warnings system by using assumeThat to skip tests */ + return; + } String remote = "https://gitlab.com/rishabhBudhouliya/git-plugin.git"; Item context = Mockito.mock(Item.class); String credentialsId = null; - TestToolInstaller inst = new TestToolInstaller("master", "echo Hello", isWindows() ? "updated\\git.exe" : "updated/git"); - GitTool t = new GitTool("myGit", isWindows() ? "default\\git.exe" : "default/git", Collections.singletonList( + TestToolInstaller inst = new TestToolInstaller("master", "echo Hello", "updated/git"); + GitTool t = new GitTool("myGit", "default/git", Collections.singletonList( new InstallSourceProperty(Collections.singletonList(inst)))); - GitTool tool = new GitTool("my-git", isWindows() ? "git.exe" : "git", Collections.>emptyList()); + GitTool tool = new GitTool("my-git", "git", Collections.>emptyList()); GitTool JTool = new JGitTool(Collections.>emptyList()); jenkins.jenkins.getDescriptorByType(GitTool.DescriptorImpl.class).setInstallations(tool, JTool, t); GitToolChooser r = new GitToolChooser(remote, context, credentialsId, JTool, null, TaskListener.NULL,true); - assertThat(r.getGitTool(), containsString(isWindows() ? "updated\\git.exe" : "updated/git")); + assertThat(r.getGitTool(), containsString("updated/git")); } /* @@ -122,6 +126,10 @@ public void testResolveGitToolWithJenkins() throws IOException, InterruptedExcep @Issue("JENKINS-63519") @Test public void testResolutionGitToolOnAgent() throws Exception { + if (isWindows()) { // Runs on Unix only + /* Do not distract warnings system by using assumeThat to skip tests */ + return; + } String remote = "https://gitlab.com/rishabhBudhouliya/git-plugin.git"; Item context = Mockito.mock(Item.class); String credentialsId = null; @@ -131,12 +139,12 @@ public void testResolutionGitToolOnAgent() throws Exception { agent.setMode(Node.Mode.NORMAL); agent.setLabelString("agent-windows"); - TestToolInstaller inst = new TestToolInstaller("master", "echo Hello", isWindows() ? "myGit\\git.exe" : "myGit/git"); - GitTool toolOnMaster = new GitTool("myGit", isWindows() ? "default\\git.exe" : "default/git", Collections.singletonList( + TestToolInstaller inst = new TestToolInstaller("master", "echo Hello", "myGit/git"); + GitTool toolOnMaster = new GitTool("myGit", "default/git", Collections.singletonList( new InstallSourceProperty(Collections.singletonList(inst)))); - TestToolInstaller instonAgent = new TestToolInstaller("agent-windows", "echo Hello", isWindows() ? "my-git\\git.exe" : "my-git/git"); - GitTool toolOnAgent = new GitTool("my-git", isWindows() ? "git.exe" : "git", Collections.singletonList(new InstallSourceProperty(Collections.singletonList(instonAgent)))); + TestToolInstaller instonAgent = new TestToolInstaller("agent-windows", "echo Hello", "my-git/git"); + GitTool toolOnAgent = new GitTool("my-git", "git", Collections.singletonList(new InstallSourceProperty(Collections.singletonList(instonAgent)))); GitTool JTool = new JGitTool(Collections.>emptyList()); @@ -152,7 +160,7 @@ public void testResolutionGitToolOnAgent() throws Exception { GitToolChooser r = new GitToolChooser(remote, context, credentialsId, JTool, agent, TaskListener.NULL,true); - assertThat(r.getGitTool(), containsString(isWindows() ? "my-git\\git.exe" : "my-git/git")); + assertThat(r.getGitTool(), containsString("my-git/git")); } /*