From 7610501c66a704214091aa0ba89065a53719f9cd Mon Sep 17 00:00:00 2001 From: Tong He <68936428+unnecessary-username@users.noreply.github.com> Date: Fri, 11 Nov 2022 04:18:01 +0800 Subject: [PATCH] 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 * 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 --- ...gical-flaw-when-deleting-jenkins-build.yml | 2 + plugins/modules/jenkins_build.py | 10 +++- .../plugins/modules/test_jenkins_build.py | 48 +++++++++++++++++-- 3 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 changelogs/fragments/5514-fix-logical-flaw-when-deleting-jenkins-build.yml diff --git a/changelogs/fragments/5514-fix-logical-flaw-when-deleting-jenkins-build.yml b/changelogs/fragments/5514-fix-logical-flaw-when-deleting-jenkins-build.yml new file mode 100644 index 0000000000..818ee95146 --- /dev/null +++ b/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). \ No newline at end of file diff --git a/plugins/modules/jenkins_build.py b/plugins/modules/jenkins_build.py index 09304ccfbc..b02027f229 100644 --- a/plugins/modules/jenkins_build.py +++ b/plugins/modules/jenkins_build.py @@ -183,7 +183,10 @@ class JenkinsBuild: 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 @@ class JenkinsBuild: 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: diff --git a/tests/unit/plugins/modules/test_jenkins_build.py b/tests/unit/plugins/modules/test_jenkins_build.py index adee2fce99..44c6307ac9 100644 --- a/tests/unit/plugins/modules/test_jenkins_build.py +++ b/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 @@ class JenkinsMock(): } 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 @@ class JenkinsMockIdempotent(): 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 @@ class TestJenkinsBuild(unittest.TestCase): @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",