FIXED JENKINS-34613

Set default value of Generator in Java instead of jelly, for improved usability in pipeline plugin
jenkinsci · May 6, 2016 · 7fc4866 · 7fc4866
1 parent 1181977
commit 7fc4866
Show file tree

Hide file tree

Showing 5 changed files with 94 additions and 30 deletions.
diff --git a/src/main/java/hudson/plugins/cmake/CmakeBuilder.java b/src/main/java/hudson/plugins/cmake/CmakeBuilder.java
@@ -38,7 +38,10 @@ public class CmakeBuilder extends AbstractCmakeBuilder {
      */
     public static final String ENV_VAR_NAME_CMAKE_BUILD_TOOL = "CMAKE_BUILD_TOOL";
 
-    /** allowed to be empty but not {@code null} */
+    /**
+     * the name of cmake´s buildscript generator or {@code null} if the default
+     * generator should be used
+     */
     private String generator;
     private String sourceDir;
     private String buildType;
@@ -59,14 +62,27 @@ public class CmakeBuilder extends AbstractCmakeBuilder {
      * @param installationName
      *            the name of the cmake tool installation from the global config
      *            page.
+     */
+    @DataBoundConstructor
+    public CmakeBuilder(String installationName) {
+        super(installationName);
+    }
+
+    /**
+     * Old constructor.
+     *
+     * @param installationName
+     *            the name of the cmake tool installation from the global config
+     *            page.
      * @param generator
      *            the name of cmake´s buildscript generator. May be empty but
      *            not {@code null}
+     * @deprecated to minimize number of mandatory field values.
      */
-    @DataBoundConstructor
+    @Deprecated
     public CmakeBuilder(String installationName, String generator) {
         super(installationName);
-        this.generator = Util.fixNull(generator);
+        setGenerator(generator);
     }
 
     // for backward compatibility with < 2.4.
@@ -81,8 +97,23 @@ protected Object readResolve() {
         return this;
     }
 
+    /**
+     * Sets the name of the build-script generator.
+     * 
+     * @param generator
+     *            the name of cmake´s build-script generator or {@code null} or
+     *            empty if the default generator should be used
+     */
+    @DataBoundSetter
+    public void setGenerator(String generator) {
+        generator = Util.fixEmptyAndTrim(generator);
+        this.generator = DescriptorImpl.getDefaultGenerator().equals(generator)
+                ? null : generator;
+    }
+
     public String getGenerator() {
-        return this.generator;
+        return this.generator == null ? DescriptorImpl.getDefaultGenerator()
+                : generator;
     }
 
     @DataBoundSetter
@@ -203,7 +234,7 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher,
             FilePath theSourceDir = makeRemotePath(workSpace,
                     Util.replaceMacro(sourceDir, envs));
             ArgumentListBuilder cmakeCall = buildCMakeCall(cmakeBin,
-                    Util.replaceMacro(this.generator, envs),
+                    Util.replaceMacro(getGenerator(), envs),
                     Util.replaceMacro(this.preloadScript, envs), theSourceDir,
                     Util.replaceMacro(this.buildType, envs),
                     Util.replaceMacro(getCmakeArgs(), envs));
@@ -380,24 +411,18 @@ public DescriptorImpl() {
         }
 
         /**
-         * This human readable name is used in the configuration screen.
+         * Gets the default generator to use if the builder`s generator field is
+         * <code>null</code>.
          */
-        public String getDisplayName() {
-            return "CMake Build";
+        public static String getDefaultGenerator() {
+            return "Unix Makefiles";
         }
 
         /**
-         * Performs on-the-fly validation of the form field 'generator'.
-         *
-         * @param value
+         * This human readable name is used in the configuration screen.
          */
-        public FormValidation doCheckGenerator(
-                @QueryParameter final String value)
-                        throws IOException, ServletException {
-            if (value.length() == 0) {
-                return FormValidation.error("Please set a generator name");
-            }
-            return FormValidation.ok();
+        public String getDisplayName() {
+            return "CMake Build";
         }
 
         /**
@@ -408,7 +433,7 @@ public FormValidation doCheckGenerator(
         public FormValidation doCheckSourceDir(
                 @AncestorInPath AbstractProject<?, ?> project,
                 @QueryParameter final String value)
-                        throws IOException, ServletException {
+                throws IOException, ServletException {
             FilePath ws = project.getSomeWorkspace();
             if (ws == null)
                 return FormValidation.ok();

diff --git a/src/main/resources/hudson/plugins/cmake/CmakeBuilder/config.jelly b/src/main/resources/hudson/plugins/cmake/CmakeBuilder/config.jelly
@@ -4,7 +4,7 @@
     <f:select />
   </f:entry>
   <f:entry title="Script Generator" field="generator">
-    <f:textbox default="Unix Makefiles" />
+    <f:textbox default="${descriptor.defaultGenerator}" />
   </f:entry>
   <f:entry title="Source Directory" field="sourceDir">
     <f:textbox />
@@ -27,9 +27,8 @@
       <f:expandableTextbox />
     </f:entry>
   </f:advanced>
-  <!--
-  -->
-  <f:entry title="${%Build tool}&lt;br>${%invocations}" field="runTool" >
+
+  <f:entry title="${%Build tool}" field="runTool" >
     <f:repeatable field="steps" header="${%Run build tool}" add="${%Add build tool invocation}">
       <table width="100%">
         <st:include page="config.jelly" class="hudson.plugins.cmake.BuildToolStep" />

diff --git a/src/test/java/hudson/plugins/cmake/CmakeBuilderBuildTest.java b/src/test/java/hudson/plugins/cmake/CmakeBuilderBuildTest.java
@@ -52,7 +52,7 @@ public void testOnMaster() throws Exception {
         FreeStyleProject p = j.createFreeStyleProject();
         p.setScm(scm);
 
-        CmakeBuilder cmb = new CmakeBuilder(CmakeTool.DEFAULT, "Unix Makefiles");
+        CmakeBuilder cmb = new CmakeBuilder(CmakeTool.DEFAULT);
         cmb.setCleanBuild(true);
         cmb.setSourceDir("src");
         cmb.setBuildDir("src");
@@ -76,7 +76,7 @@ public void testOnSlave() throws Exception {
                 .get("cmakebuilder-test-slave"));
         p.setAssignedLabel(slave.getSelfLabel());
 
-        CmakeBuilder cmb = new CmakeBuilder(CmakeTool.DEFAULT, "Unix Makefiles");
+        CmakeBuilder cmb = new CmakeBuilder(CmakeTool.DEFAULT);
         cmb.setCleanBuild(true);
         cmb.setSourceDir("src");
         cmb.setBuildDir("src");
@@ -102,7 +102,7 @@ public void testBuildVariables() throws Exception {
         StringParameterDefinition pd3 = new StringParameterDefinition(
                 "BUILDTYPE", "Release");
         StringParameterDefinition pd4 = new StringParameterDefinition(
-                "BUILDGENERATOR", "Unix Makefiles");
+                "BUILDGENERATOR");
         StringParameterDefinition pd5 = new StringParameterDefinition(
                 "BUILDTOOL", "make");
         StringParameterDefinition pd6 = new StringParameterDefinition(
@@ -165,7 +165,7 @@ public void testBuildToolStep() throws Exception {
         FreeStyleProject p = j.createFreeStyleProject();
         p.setScm(scm);
 
-        CmakeBuilder cmb = new CmakeBuilder(CmakeTool.DEFAULT, "Unix Makefiles");
+        CmakeBuilder cmb = new CmakeBuilder(CmakeTool.DEFAULT);
         cmb.setCleanBuild(true);
         cmb.setSourceDir("src");
         cmb.setBuildDir("build/debug");
@@ -198,7 +198,7 @@ public void testBuildToolStepWithCmake() throws Exception {
         FreeStyleProject p = j.createFreeStyleProject();
         p.setScm(scm);
 
-        CmakeBuilder cmb = new CmakeBuilder(CmakeTool.DEFAULT, "Unix Makefiles");
+        CmakeBuilder cmb = new CmakeBuilder(CmakeTool.DEFAULT);
         cmb.setCleanBuild(true);
         cmb.setSourceDir("src");
         cmb.setBuildDir("build/debug");

diff --git a/src/test/java/hudson/plugins/cmake/CmakeBuilderFormRoundTripTest.java b/src/test/java/hudson/plugins/cmake/CmakeBuilderFormRoundTripTest.java
@@ -15,8 +15,7 @@ public class CmakeBuilderFormRoundTripTest {
     public void checkValidation() throws Exception {
 
         FreeStyleProject p = j.createFreeStyleProject();
-        CmakeBuilder before = new CmakeBuilder(CmakeTool.DEFAULT,
-                "Unix Makefiles");
+        CmakeBuilder before = new CmakeBuilder(CmakeTool.DEFAULT);
         p.getBuildersList().add(before);
 
         j.submit(j.createWebClient().getPage(p, "configure")

diff --git a/src/test/java/hudson/plugins/cmake/CmakeBuilderTest.java b/src/test/java/hudson/plugins/cmake/CmakeBuilderTest.java
@@ -0,0 +1,41 @@
+package hudson.plugins.cmake;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import org.junit.Test;
+
+/**
+ * Tests the CmakeBuilder`s getters and setters.
+ *
+ * @author Martin Weber
+ */
+public class CmakeBuilderTest {
+
+    /**
+     * Verify that a default generator is set.
+     */
+    @Test
+    public void testDefaultGenerator() throws Exception {
+        CmakeBuilder cmb = new CmakeBuilder(CmakeTool.DEFAULT);
+        final String generator = cmb.getGenerator();
+        assertNotNull(generator);
+        assertTrue(generator.trim().length() > 0);
+    }
+
+    /**
+     * Verify that a default generator is set even if set to null.
+     */
+    @Test
+    public void testDefaultGeneratorSet() throws Exception {
+        CmakeBuilder cmb = new CmakeBuilder(CmakeTool.DEFAULT);
+        final String setGenerator = "null: gipsnich";
+        cmb.setGenerator(setGenerator);
+        assertEquals(setGenerator, cmb.getGenerator());
+        cmb.setGenerator(null);
+        String generator = cmb.getGenerator();
+        assertNotNull(generator);
+        assertTrue(generator.trim().length() > 0);
+    }
+}