From 05986a64d735bd65c40c42cb221648f6eaf5e621 Mon Sep 17 00:00:00 2001 From: Tomas Janousek Date: Fri, 3 Aug 2018 22:33:31 +0200 Subject: [PATCH] Replace build_jobs_list_responses with actual Jenkins responses This abstraction did more harm than good -- pretended to create correct JSON responses from a simpler structure, but created responses that were very far from what Jenkins would return: * "url" fields didn't contain folders, but nobody noticed as these weren't used anyway * "jobs" fields contained the string "null" (not JSON null, an actual JSON string!), whereas real Jenkins returns a list of almost empty objects (just a "class" field, if Jenkins is new enough, otherwise nothing) * "color" field is absent from folders The usage of deepcopy, insert and append totally obstructed the real contents of those responses from most readers. Let's just be a bit verbose here, please. Change-Id: Ida2cfa3662e491e4178228096dc622cdd859202f --- tests/jobs/base.py | 100 +++++++++++++++++++------------------- tests/jobs/test_get.py | 9 +--- tests/jobs/test_getall.py | 26 ++++------ 3 files changed, 62 insertions(+), 73 deletions(-) diff --git a/tests/jobs/base.py b/tests/jobs/base.py index 85d0eac..1ad980e 100644 --- a/tests/jobs/base.py +++ b/tests/jobs/base.py @@ -1,6 +1,3 @@ -import copy -import json - from tests.base import JenkinsTestBase @@ -16,59 +13,62 @@ class JenkinsJobsTestBase(JenkinsTestBase): class JenkinsGetJobsTestBase(JenkinsJobsTestBase): jobs_in_folder = [ - [ - {'name': 'my_job1'}, - {'name': 'my_folder1', 'jobs': None}, - {'name': 'my_job2'} - ], + {'jobs': [ + {'name': 'my_job1', 'color': 'blue', 'url': 'http://...'}, + {'name': 'my_folder1', 'url': 'http://...', 'jobs': [{}, {}]}, + {'name': 'my_job2', 'color': 'blue', 'url': 'http://...'} + ]}, # my_folder1 jobs - [ - {'name': 'my_job3'}, - {'name': 'my_job4'} - ] + {'jobs': [ + {'name': 'my_job3', 'color': 'blue', 'url': 'http://...'}, + {'name': 'my_job4', 'color': 'blue', 'url': 'http://...'} + ]} ] - jobs_in_multiple_folders = copy.deepcopy(jobs_in_folder) - jobs_in_multiple_folders[1].insert( - 0, {'name': 'my_folder2', 'jobs': None}) - jobs_in_multiple_folders.append( + jobs_in_multiple_folders = [ + {'jobs': [ + {'name': 'my_job1', 'color': 'blue', 'url': 'http://...'}, + {'name': 'my_folder1', 'url': 'http://...', 'jobs': [{}, {}, {}]}, + {'name': 'my_job2', 'color': 'blue', 'url': 'http://...'} + ]}, + # my_folder1 jobs + {'jobs': [ + {'name': 'my_folder2', 'url': 'http://...', 'jobs': [{}, {}]}, + {'name': 'my_job3', 'color': 'blue', 'url': 'http://...'}, + {'name': 'my_job4', 'color': 'blue', 'url': 'http://...'} + ]}, # my_folder1/my_folder2 jobs - [ - {'name': 'my_job1'}, - {'name': 'my_job2'} - ] - ) + {'jobs': [ + {'name': 'my_job1', 'color': 'blue', 'url': 'http://...'}, + {'name': 'my_job2', 'color': 'blue', 'url': 'http://...'} + ]} + ] - jobs_in_unsafe_name_folders = copy.deepcopy(jobs_in_folder) - jobs_in_unsafe_name_folders[1].insert( - 0, {'name': 'my spaced folder', 'jobs': None}) - jobs_in_unsafe_name_folders.append( + jobs_in_unsafe_name_folders = [ + {'jobs': [ + {'name': 'my_job1', 'color': 'blue', 'url': 'http://...'}, + {'name': 'my_folder1', 'url': 'http://...', 'jobs': [{}, {}]}, + {'name': 'my_job2', 'color': 'blue', 'url': 'http://...'} + ]}, + # my_folder1 jobs + {'jobs': [ + {'name': 'my spaced folder', 'url': 'http://...', 'jobs': [{}]}, + {'name': 'my_job3', 'color': 'blue', 'url': 'http://...'}, + {'name': 'my_job4', 'color': 'blue', 'url': 'http://...'} + ]}, # my_folder1/my\ spaced\ folder jobs - [ - {'name': 'my job 5'} - ] - ) + {'jobs': [ + {'name': 'my job 5', 'color': 'blue', 'url': 'http://...'} + ]} + ] jobs_in_folder_named_job = [ - [{'name': 'job', 'jobs': None}], # actually a folder :-) - [{'name': 'my_job'}] # "job" folder jobs + # actually a folder :-) + {'jobs': [ + {'name': 'job', 'url': 'http://...', 'jobs': [{}]} + ]}, + # "job" folder jobs + {'jobs': [ + {'name': 'my_job', 'color': 'blue', 'url': 'http://...'} + ]} ] - - -def build_jobs_list_responses(jobs_list, server_url): - responses = [] - for jobs in jobs_list: - get_jobs_response = [] - for job in jobs: - job_json = { - u'url': u'%s/job/%s' % (server_url.rstrip('/'), job['name']), - u'name': job['name'], - u'color': u'blue' - } - if 'jobs' in job: - job_json[u'jobs'] = "null" - get_jobs_response.append(job_json) - - responses.append(json.dumps({u'jobs': get_jobs_response})) - - return responses diff --git a/tests/jobs/test_get.py b/tests/jobs/test_get.py index cb0cc72..36c599b 100644 --- a/tests/jobs/test_get.py +++ b/tests/jobs/test_get.py @@ -3,7 +3,6 @@ from mock import patch import jenkins from tests.helper import build_response_mock -from tests.jobs.base import build_jobs_list_responses from tests.jobs.base import JenkinsGetJobsTestBase @@ -30,9 +29,7 @@ class JenkinsGetJobsTest(JenkinsGetJobsTestBase): @patch.object(jenkins.Jenkins, 'jenkins_open') def test_folders_simple(self, jenkins_mock): - response = build_jobs_list_responses( - self.jobs_in_folder, self.make_url('')) - jenkins_mock.side_effect = iter(response) + jenkins_mock.side_effect = map(json.dumps, self.jobs_in_folder) jobs_info = self.j.get_jobs() @@ -45,9 +42,7 @@ class JenkinsGetJobsTest(JenkinsGetJobsTestBase): @patch.object(jenkins.Jenkins, 'jenkins_open') def test_folders_additional_level(self, jenkins_mock): - response = build_jobs_list_responses( - self.jobs_in_folder, self.make_url('')) - jenkins_mock.side_effect = iter(response) + jenkins_mock.side_effect = map(json.dumps, self.jobs_in_folder) jobs_info = self.j.get_jobs(folder_depth=1) diff --git a/tests/jobs/test_getall.py b/tests/jobs/test_getall.py index 4e38a00..1daf84c 100644 --- a/tests/jobs/test_getall.py +++ b/tests/jobs/test_getall.py @@ -1,7 +1,7 @@ +import json from mock import patch import jenkins -from tests.jobs.base import build_jobs_list_responses from tests.jobs.base import JenkinsGetJobsTestBase @@ -9,9 +9,7 @@ class JenkinsGetAllJobsTest(JenkinsGetJobsTestBase): @patch.object(jenkins.Jenkins, 'jenkins_open') def test_simple(self, jenkins_mock): - response = build_jobs_list_responses( - self.jobs_in_folder, 'http://example.com/') - jenkins_mock.side_effect = iter(response) + jenkins_mock.side_effect = map(json.dumps, self.jobs_in_folder) jobs_info = self.j.get_all_jobs() @@ -32,9 +30,8 @@ class JenkinsGetAllJobsTest(JenkinsGetJobsTestBase): @patch.object(jenkins.Jenkins, 'jenkins_open') def test_multi_level(self, jenkins_mock): - response = build_jobs_list_responses( - self.jobs_in_multiple_folders, 'http://example.com/') - jenkins_mock.side_effect = iter(response) + jenkins_mock.side_effect = map( + json.dumps, self.jobs_in_multiple_folders) jobs_info = self.j.get_all_jobs() @@ -61,9 +58,8 @@ class JenkinsGetAllJobsTest(JenkinsGetJobsTestBase): @patch.object(jenkins.Jenkins, 'jenkins_open') def test_folders_depth(self, jenkins_mock): - response = build_jobs_list_responses( - self.jobs_in_multiple_folders, 'http://example.com/') - jenkins_mock.side_effect = iter(response) + jenkins_mock.side_effect = map( + json.dumps, self.jobs_in_multiple_folders) jobs_info = self.j.get_all_jobs(folder_depth=1) @@ -84,9 +80,8 @@ class JenkinsGetAllJobsTest(JenkinsGetJobsTestBase): @patch.object(jenkins.Jenkins, 'jenkins_open') def test_unsafe_chars(self, jenkins_mock): - response = build_jobs_list_responses( - self.jobs_in_unsafe_name_folders, 'http://example.com/') - jenkins_mock.side_effect = iter(response) + jenkins_mock.side_effect = map( + json.dumps, self.jobs_in_unsafe_name_folders) jobs_info = self.j.get_all_jobs() @@ -109,9 +104,8 @@ class JenkinsGetAllJobsTest(JenkinsGetJobsTestBase): @patch.object(jenkins.Jenkins, 'jenkins_open') def test_folder_named_job(self, jenkins_mock): - response = build_jobs_list_responses( - self.jobs_in_folder_named_job, 'http://example.com/') - jenkins_mock.side_effect = iter(response) + jenkins_mock.side_effect = map( + json.dumps, self.jobs_in_folder_named_job) jobs_info = self.j.get_all_jobs()