Add test for PropertyBasedJobInclusionStrategy class (#420)

* Add test for PropertyBasedJobInclusionStrategy class * Made cloudbeesFolder package-private * Add constructor to JobGroup class Added a constructor to initialize description, priority, and jobGroupStrategy fields. This change allows creating JobGroup instances with specific values for these fields. * Changed FreestyleProject from project to j * Check results from decision logger Do not use wildcard import of org.junit.Assert because that includes the deprecated assertThat method. Use the assertThat method from hamcrest because it is not deprecated and works very well with hamcrest assertions. Name the strategy uniquely for each test by including the test name in the strategy name. The TestName Rule is a very handy way to assure that test data is unique for each test method without needing to worry about the uniqueness directly. Split the Before methods so that they can be named more specifically. * Replace JobGroup changes with more test code Don't need a new constructor when setters are already available for all the attributes used in the new constructor. * Rename getDescriptor and make it package protected for tests Reduce the amount of change in the production object by not creating a new method that overrides the method from the superclass. Limit the accessibility of the new method so that it is package protected and not available to classes outside its own package. Remove two duplicated test methods * Improve documentation and add tests for PropertyBasedJobInclusionStrategy - Added detailed Javadoc comments for `getThisDescriptor` and `getDescriptor` methods. - Enhanced inline comments to clarify the use of `@SuppressWarnings("unchecked")`. * Add package protected comment --------- Co-authored-by: Mark Waite <[email protected]>
jenkinsci · Dec 30, 2024 · 3f3fe0b · 3f3fe0b
1 parent 37f738b
commit 3f3fe0b
Show file tree

Hide file tree

Showing 3 changed files with 254 additions and 1 deletion.
diff --git a/src/main/java/jenkins/advancedqueue/jobinclusion/JobInclusionStrategy.java b/src/main/java/jenkins/advancedqueue/jobinclusion/JobInclusionStrategy.java
@@ -53,6 +53,8 @@ public String getDisplayName() {
     }
     ;
 
+    // Suppress unchecked cast warning because Jenkins.get().getDescriptor(this.getClass())
+    // returns a Descriptor of the specific JobInclusionStrategy subclass, which is safe to cast
     @SuppressWarnings("unchecked")
     public Descriptor<JobInclusionStrategy> getDescriptor() {
         return Jenkins.get().getDescriptor(this.getClass());

diff --git a/...n/java/jenkins/advancedqueue/jobinclusion/strategy/PropertyBasedJobInclusionStrategy.java b/...n/java/jenkins/advancedqueue/jobinclusion/strategy/PropertyBasedJobInclusionStrategy.java
@@ -45,7 +45,8 @@ public class PropertyBasedJobInclusionStrategy extends JobInclusionStrategy {
     @Extension
     public static class PropertyBasedJobInclusionStrategyDescriptor extends Descriptor<JobInclusionStrategy> {
 
-        private boolean cloudbeesFolders = true;
+        /* Package protected for testing */
+        boolean cloudbeesFolders = true;
 
         @Override
         public String getDisplayName() {
@@ -65,6 +66,17 @@ public PropertyBasedJobInclusionStrategyDescriptor() {
     }
     ;
 
+    /* Package protected for testing */
+    /* Intentionally does not override getDescriptor() from the super class */
+    /**
+     * Returns the descriptor for the PropertyBasedJobInclusionStrategy class.
+     *
+     * @return PropertyBasedJobInclusionStrategyDescriptor
+     */
+    PropertyBasedJobInclusionStrategy.PropertyBasedJobInclusionStrategyDescriptor getThisDescriptor() {
+        return (PropertyBasedJobInclusionStrategy.PropertyBasedJobInclusionStrategyDescriptor) super.getDescriptor();
+    }
+
     private String name;
 
     @DataBoundConstructor

diff --git a/...va/jenkins/advancedqueue/jobinclusion/strategy/PropertyBasedJobInclusionStrategyTest.java b/...va/jenkins/advancedqueue/jobinclusion/strategy/PropertyBasedJobInclusionStrategyTest.java
@@ -0,0 +1,239 @@
+package jenkins.advancedqueue.jobinclusion.strategy;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.hasItems;
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import hudson.model.FreeStyleProject;
+import hudson.util.ListBoxModel;
+import java.util.ArrayList;
+import java.util.List;
+import jenkins.advancedqueue.DecisionLogger;
+import jenkins.advancedqueue.JobGroup;
+import jenkins.advancedqueue.PriorityConfiguration;
+import jenkins.advancedqueue.jobinclusion.strategy.PropertyBasedJobInclusionStrategy.PropertyBasedJobInclusionStrategyDescriptor;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TestName;
+import org.jvnet.hudson.test.JenkinsRule;
+
+public class PropertyBasedJobInclusionStrategyTest {
+
+    @ClassRule
+    public static JenkinsRule j = new JenkinsRule();
+
+    @Rule
+    public TestName testName = new TestName();
+
+    private FreeStyleProject project;
+    private String strategyName;
+    private PropertyBasedJobInclusionStrategy strategy;
+    private DecisionLogger decisionLogger;
+    private List<String> loggedMessages;
+
+    @Before
+    public void createStrategy() throws Exception {
+        strategyName = "testGroup-" + testName.getMethodName();
+        strategy = new PropertyBasedJobInclusionStrategy(strategyName);
+    }
+
+    @Before
+    public void createProject() throws Exception {
+        // Name each freestyle project with the name of the test creating it
+        project = j.createFreeStyleProject("freestyle-" + testName.getMethodName());
+    }
+
+    @Before
+    public void createDecisionLogger() throws Exception {
+        loggedMessages = new ArrayList<>();
+        decisionLogger = new DecisionLogger() {
+            @Override
+            public DecisionLogger addDecisionLog(int indent, String log) {
+                loggedMessages.add(log);
+                return this;
+            }
+        };
+    }
+
+    @After
+    public void deleteProject() throws Exception {
+        project.delete();
+    }
+
+    @Test
+    public void all() {
+        List<JobGroup> jobGroups = PriorityConfiguration.get().getJobGroups();
+        assertNotNull(jobGroups);
+        assertThat(loggedMessages, is(empty()));
+    }
+
+    @Test
+    public void getName() {
+        assertEquals(strategyName, strategy.getName());
+        assertThat(loggedMessages, is(empty()));
+    }
+
+    @Test
+    public void contains() {
+        boolean result = strategy.contains(decisionLogger, project);
+        assertFalse(result); // Assuming the project does not have the required property
+        assertThat(loggedMessages, hasItems("No match ..."));
+    }
+
+    @Test
+    public void getPropertyBasesJobGroups() {
+        ListBoxModel jobGroups = PropertyBasedJobInclusionStrategy.getPropertyBasesJobGroups();
+        assertNotNull(jobGroups);
+        assertThat(loggedMessages, is(empty()));
+    }
+
+    @Test
+    public void getDisplayNameContainsJobs() {
+        PropertyBasedJobInclusionStrategy.PropertyBasedJobInclusionStrategyDescriptor descriptor =
+                strategy.getThisDescriptor();
+        assertTrue(descriptor.getDisplayName().contains("Jobs"));
+        assertThat(loggedMessages, is(empty()));
+    }
+
+    @Test
+    public void allReturnsNonNullJobGroups() {
+        List<JobGroup> jobGroups = PriorityConfiguration.get().getJobGroups();
+        assertNotNull(jobGroups);
+        assertThat(loggedMessages, is(empty()));
+    }
+
+    @Test
+    public void getNameReturnsCorrectName() {
+        assertEquals(strategyName, strategy.getName());
+        assertThat(loggedMessages, is(empty()));
+    }
+
+    @Test
+    public void containsReturnsFalseForProjectWithoutProperty() {
+        boolean result = strategy.contains(decisionLogger, project);
+        assertFalse(result);
+        assertThat(loggedMessages, hasItems("No match ..."));
+    }
+
+    @Test
+    public void getPropertyBasesJobGroupsReturnsNonNullListBoxModel() {
+        ListBoxModel jobGroups = PropertyBasedJobInclusionStrategy.getPropertyBasesJobGroups();
+        assertNotNull(jobGroups);
+        assertThat(loggedMessages, is(empty()));
+    }
+
+    @Test
+    public void containsReturnsTrueForProjectWithMatchingJobGroup() throws Exception {
+        project.addProperty(new JobInclusionJobProperty(true, strategyName));
+
+        boolean result = strategy.contains(decisionLogger, project);
+        assertTrue(result);
+        assertThat(
+                loggedMessages,
+                hasItems(
+                        "JobGroup is enabled on job, with JobGroup [" + strategyName + "] ...",
+                        "Job is included in JobGroup ..."));
+    }
+
+    @Test
+    public void containsReturnsFalseForProjectWithNonMatchingJobGroup() throws Exception {
+        project.addProperty(new JobInclusionJobProperty(true, "nonMatchingGroup"));
+
+        boolean result = strategy.contains(decisionLogger, project);
+        assertFalse(result);
+        assertThat(
+                loggedMessages,
+                hasItems(
+                        "JobGroup is enabled on job, with JobGroup [nonMatchingGroup] ...",
+                        "Job is not included in JobGroup ..."));
+    }
+
+    @Test
+    public void containsReturnsFalseWhenCloudBeesFoldersPluginNotEnabled() throws Exception {
+        // Simulate CloudBees Folders plugin not enabled
+        PropertyBasedJobInclusionStrategyDescriptor descriptor = strategy.getThisDescriptor();
+        descriptor.cloudbeesFolders = false;
+
+        boolean result = strategy.contains(decisionLogger, project);
+        assertFalse(result);
+        assertThat(loggedMessages, hasItems("Checking for Job Property inclusion for [" + strategyName + "]..."));
+    }
+
+    @Test
+    public void getPropertyBasesJobGroupsReturnsEmptyListBoxModelWhenNoJobGroups() {
+        // Simulate no job groups
+        PriorityConfiguration.get().getJobGroups().clear();
+
+        ListBoxModel jobGroups = PropertyBasedJobInclusionStrategy.getPropertyBasesJobGroups();
+        assertNotNull(jobGroups);
+        assertTrue(jobGroups.isEmpty());
+        assertThat(loggedMessages, is(empty()));
+    }
+
+    @Test
+    public void containsReturnsTrueForProjectWithMatchingPropertyAndCloudBeesFoldersEnabled() throws Exception {
+        project.addProperty(new JobInclusionJobProperty(true, strategyName));
+        PropertyBasedJobInclusionStrategyDescriptor descriptor = strategy.getThisDescriptor();
+        descriptor.cloudbeesFolders = true;
+
+        boolean result = strategy.contains(decisionLogger, project);
+        assertTrue(result);
+        assertThat(
+                loggedMessages,
+                hasItems(
+                        "JobGroup is enabled on job, with JobGroup [" + strategyName + "] ...",
+                        "Job is included in JobGroup ..."));
+    }
+
+    @Test
+    public void containsReturnsFalseForProjectWithNonMatchingPropertyAndCloudBeesFoldersEnabled() throws Exception {
+        project.addProperty(new JobInclusionJobProperty(true, "nonMatchingGroup"));
+        PropertyBasedJobInclusionStrategyDescriptor descriptor = strategy.getThisDescriptor();
+        descriptor.cloudbeesFolders = true;
+
+        boolean result = strategy.contains(decisionLogger, project);
+        assertFalse(result);
+        assertThat(
+                loggedMessages,
+                hasItems(
+                        "JobGroup is enabled on job, with JobGroup [nonMatchingGroup] ...",
+                        "Job is not included in JobGroup ..."));
+    }
+
+    @Test
+    public void containsReturnsFalseForProjectWithoutPropertyAndCloudBeesFoldersEnabled() throws Exception {
+        PropertyBasedJobInclusionStrategyDescriptor descriptor = strategy.getThisDescriptor();
+        descriptor.cloudbeesFolders = true;
+
+        boolean result = strategy.contains(decisionLogger, project);
+        assertFalse(result);
+        assertThat(loggedMessages, hasItems("No match ..."));
+    }
+
+    @Test
+    public void getPropertyBasesJobGroupsReturnsNonNullListBoxModelWhenMultipleJobGroups() {
+        PriorityConfiguration.get().getJobGroups().add(createJobGroup("group1", 1));
+        PriorityConfiguration.get().getJobGroups().add(createJobGroup("group2", 2));
+
+        ListBoxModel jobGroups = PropertyBasedJobInclusionStrategy.getPropertyBasesJobGroups();
+        assertNotNull(jobGroups);
+        assertEquals(2, jobGroups.size());
+        assertThat(loggedMessages, is(empty()));
+    }
+
+    private JobGroup createJobGroup(String description, int priority) {
+        JobGroup group = new JobGroup();
+        group.setDescription(description);
+        group.setPriority(priority);
+        group.setJobGroupStrategy(new PropertyBasedJobInclusionStrategy(description));
+        return group;
+    }
+}