jjb
/
python-jenkins
Code Issues Proposed changes

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
This commit is contained in:
Tomas Janousek 2018-08-03 22:33:31 +02:00
parent 49d6c92d0c
commit 05986a64d7
3 changed files with 62 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

100
tests/jobs/base.py
View File

@ -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

9
tests/jobs/test_get.py
View File

@ -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)

26
tests/jobs/test_getall.py
View File

@ -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()