Fix a logical flaw when deleting a build in the jenkins_build module (#…

…5514)

* Fix the logical flaw when deleting a build in the jenkins_build module.

* Fix the logical flaw when deleting a Jenkins build in the jenkins_build module.

* Adding changelogs.

* Update tests/unit/plugins/modules/test_jenkins_build.py

Co-authored-by: Felix Fontein <[email protected]>

* Attempt to mock the exception classes.

* Remedy the CI issues when mocking the exception classes.

* Assuming a way to mock the get_build_status function.

* Near to the feasible approach.

* Calls the correct class when unit testing.

* Fix sending wrong arguments when unit testing.

* Directly assign the argument value in the unit testing.

* Fix errors calling different classes.

Co-authored-by: Felix Fontein <[email protected]>

changelogs/fragments/5514-fix-logical-flaw-when-deleting-jenkins-build.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - jenkins_build - fix the logical flaw when deleting a Jenkins build (https://github.com/ansible-collections/community.general/pull/5514).

plugins/modules/jenkins_build.py

@@ -183,7 +183,10 @@ def get_build_status(self):
         try:
             response = self.server.get_build_info(self.name, self.build_number)
             return response
-
+        except jenkins.JenkinsException as e:
+            response = {}
+            response["result"] = "ABSENT"
+            return response
         except Exception as e:
             self.module.fail_json(msg='Unable to fetch build information, %s' % to_native(e),
                                   exception=traceback.format_exc())
@@ -231,7 +234,10 @@ def get_result(self):
             if self.state == "stopped" and build_status['result'] == "ABORTED":
                 result['changed'] = True
                 result['build_info'] = build_status
-            elif build_status['result'] == "SUCCESS":
+            elif self.state == "absent" and build_status['result'] == "ABSENT":
+                result['changed'] = True
+                result['build_info'] = build_status
+            elif self.state != "absent" and build_status['result'] == "SUCCESS":
                 result['changed'] = True
                 result['build_info'] = build_status
             else:

tests/unit/plugins/modules/test_jenkins_build.py

@@ -43,6 +43,28 @@ def fail_json(*args, **kwargs):
     raise AnsibleFailJson(kwargs)
 
 
+class jenkins:
+    class JenkinsException(Exception):
+        pass
+
+    class NotFoundException(JenkinsException):
+        pass
+
+
+class JenkinsBuildMock():
+    def get_build_status(self):
+        try:
+            instance = JenkinsMock()
+            response = JenkinsMock.get_build_info(instance, 'host-delete', 1234)
+            return response
+        except jenkins.JenkinsException as e:
+            response = {}
+            response["result"] = "ABSENT"
+            return response
+        except Exception as e:
+            fail_json(msg='Unable to fetch build information, {0}'.format(e))
+
+
 class JenkinsMock():
 
     def get_job_info(self, name):
@@ -51,6 +73,8 @@ def get_job_info(self, name):
         }
 
     def get_build_info(self, name, build_number):
+        if name == "host-delete":
+            raise jenkins.JenkinsException("job {0} number {1} does not exist".format(name, build_number))
         return {
             "building": True,
             "result": "SUCCESS"
@@ -83,7 +107,7 @@ def build_job(self, *args):
         return None
 
     def delete_build(self, name, build_number):
-        return None
+        raise jenkins.NotFoundException("job {0} number {1} does not exist".format(name, build_number))
 
     def stop_build(self, name, build_number):
         return None
@@ -167,13 +191,31 @@ def test_module_stop_build_again(self, jenkins_connection, test_deps):
 
     @patch('ansible_collections.community.general.plugins.modules.jenkins_build.test_dependencies')
     @patch('ansible_collections.community.general.plugins.modules.jenkins_build.JenkinsBuild.get_jenkins_connection')
-    def test_module_delete_build(self, jenkins_connection, test_deps):
+    @patch('ansible_collections.community.general.plugins.modules.jenkins_build.JenkinsBuild.get_build_status')
+    def test_module_delete_build(self, build_status, jenkins_connection, test_deps):
         test_deps.return_value = None
         jenkins_connection.return_value = JenkinsMock()
+        build_status.return_value = JenkinsBuildMock().get_build_status()
 
         with self.assertRaises(AnsibleExitJson):
             set_module_args({
-                "name": "host-check",
+                "name": "host-delete",
+                "build_number": "1234",
+                "state": "absent",
+                "user": "abc",
+                "token": "xyz"
+            })
+            jenkins_build.main()
+
+    @patch('ansible_collections.community.general.plugins.modules.jenkins_build.test_dependencies')
+    @patch('ansible_collections.community.general.plugins.modules.jenkins_build.JenkinsBuild.get_jenkins_connection')
+    def test_module_delete_build_again(self, jenkins_connection, test_deps):
+        test_deps.return_value = None
+        jenkins_connection.return_value = JenkinsMockIdempotent()
+
+        with self.assertRaises(AnsibleFailJson):
+            set_module_args({
+                "name": "host-delete",
                 "build_number": "1234",
                 "state": "absent",
                 "user": "abc",