From 49d6c92d0cb74403949f84d7adc4ee98574520f0 Mon Sep 17 00:00:00 2001 From: Tomas Janousek Date: Fri, 3 Aug 2018 20:40:10 +0200 Subject: [PATCH] Clean up job/folder path handling Avoid repeatedly serializing and splitting the root path component of jobs being added to the list to be processed and returned. Attempting to split a string path using 'job' introduces a bug if a job ever happens to also use the name 'job' as part of its name. This could be fixed by using root.split('/job/') but it still seems hackish. Maintain the root path to the folder in the hierarchy as a list of path components, add to it when descending and store the result of joining it with '/job/' as a separate variable to avoid needing to re-parse. Change-Id: I074ad74ad935c50716141a79822ca84999734c3f --- jenkins/__init__.py | 12 +++++------- tests/jobs/base.py | 5 +++++ tests/jobs/test_getall.py | 20 ++++++++++++++++++++ 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/jenkins/__init__.py b/jenkins/__init__.py index 716208a..d5c5e8e 100755 --- a/jenkins/__init__.py +++ b/jenkins/__init__.py @@ -1025,25 +1025,23 @@ class Jenkins(object): """ jobs_list = [] - jobs = [(0, "", self.get_info(query=JOBS_QUERY)['jobs'])] + jobs = [(0, [], self.get_info(query=JOBS_QUERY)['jobs'])] for lvl, root, lvl_jobs in jobs: if not isinstance(lvl_jobs, list): lvl_jobs = [lvl_jobs] for job in lvl_jobs: + path = root + [job[u'name']] # insert fullname info if it doesn't exist to # allow callers to easily reference unambiguously if u'fullname' not in job: - job[u'fullname'] = '/'.join( - [p for p in root.split('/') - if p and p != 'job'] + - [job[u'name']]) + job[u'fullname'] = '/'.join(path) jobs_list.append(job) if 'jobs' in job: # folder if folder_depth is None or lvl < folder_depth: - path = '/job/'.join((root, job[u'name'])) + url_path = ''.join(['/job/' + p for p in path]) jobs.append( (lvl + 1, path, - self.get_info(path, + self.get_info(url_path, query=JOBS_QUERY)['jobs'])) return jobs_list diff --git a/tests/jobs/base.py b/tests/jobs/base.py index 4b8bbfa..85d0eac 100644 --- a/tests/jobs/base.py +++ b/tests/jobs/base.py @@ -49,6 +49,11 @@ class JenkinsGetJobsTestBase(JenkinsJobsTestBase): ] ) + jobs_in_folder_named_job = [ + [{'name': 'job', 'jobs': None}], # actually a folder :-) + [{'name': 'my_job'}] # "job" folder jobs + ] + def build_jobs_list_responses(jobs_list, server_url): responses = [] diff --git a/tests/jobs/test_getall.py b/tests/jobs/test_getall.py index 723a873..4e38a00 100644 --- a/tests/jobs/test_getall.py +++ b/tests/jobs/test_getall.py @@ -106,3 +106,23 @@ class JenkinsGetAllJobsTest(JenkinsGetJobsTestBase): ] self.assertEqual(expected_request_urls, self.got_request_urls(jenkins_mock)) + + @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) + + jobs_info = self.j.get_all_jobs() + + expected_fullnames = [u"job", u"job/my_job"] + self.assertEqual(len(expected_fullnames), len(jobs_info)) + got_fullnames = [job[u"fullname"] for job in jobs_info] + self.assertEqual(expected_fullnames, got_fullnames) + + expected_request_urls = [ + self.make_url('api/json'), + self.make_url('job/job/api/json'), + ] + self.assertEqual(expected_request_urls, + self.got_request_urls(jenkins_mock))