From 8e35880b8ae0457ece68fd6e4acb20334786be71 Mon Sep 17 00:00:00 2001 From: Tony Breeds Date: Fri, 7 Sep 2018 12:15:47 +1000 Subject: [PATCH] Factor out the logic to collect the candidate files Currently we use glob.glob() to get a list of files to validate. We make a simple choice to only use 'TC' for TC elections and '*' for PTL elections. This mostly works when the TC election is after the PTL election. Consider the following tree: candidates/some_election/TC/candidate@one candidates/some_election/TC/candidate@two candidates/some_election/project_one/candidate@three candidates/some_election/project_one/candidate@four candidates/some_election/project_two/candidate@five The current logic will check only candidate@one and candidate@two for a TC election but all files for a PTL election if the PTL election happens after the TC election. So factor out the file system logic so we can correctly include/exclude the TC based on the election type. Change-Id: I493d3663afda9d5e849176b3e6a9d042e0af7f3e --- openstack_election/tests/test_utils.py | 46 +++++++++++++------- openstack_election/utils.py | 58 +++++++++++++++----------- 2 files changed, 65 insertions(+), 39 deletions(-) diff --git a/openstack_election/tests/test_utils.py b/openstack_election/tests/test_utils.py index 30408266..7dd19d7b 100644 --- a/openstack_election/tests/test_utils.py +++ b/openstack_election/tests/test_utils.py @@ -49,15 +49,35 @@ class TestGerritUtils(ElectionTestCase): self.assertEqual(dirname, utils.name2dir(name)) +class TestFindCandidateFiles(ElectionTestCase): + @mock.patch.object(utils, 'is_tc_election', return_value=False) + @mock.patch('os.path.exists', return_value=True) + @mock.patch('os.listdir', side_effect=[['SomeProject', 'TC'], + ['invalid@example.com']]) + def test_ptl_lists(self, mock_listdir, mock_path_exists, + mock_is_tc_election): + candidate_files = utils.find_candidate_files(election='fake') + self.assertEqual(['candidates/fake/SomeProject/invalid@example.com'], + candidate_files) + + @mock.patch.object(utils, 'is_tc_election', return_value=True) + @mock.patch('os.path.exists', return_value=True) + @mock.patch('os.listdir', side_effect=[['SomeProject', 'TC'], + ['invalid@example.com']]) + def test_tc_lists(self, mock_listdir, mock_path_exists, + mock_is_tc_election): + candidate_files = utils.find_candidate_files(election='fake') + self.assertEqual(['candidates/fake/TC/invalid@example.com'], + candidate_files) + + class TestBuildCandidatesList(ElectionTestCase): @mock.patch.object(utils, 'lookup_member') - @mock.patch.object(utils, 'is_tc_election', - return_value=False) - @mock.patch('os.path.exists', return_value=True) - @mock.patch('os.listdir', side_effect=[['SomeProject'], - ['invalid@example.com']]) - def test_invalid_candidate(self, mock_listdir, mock_path_exists, - mock_is_tc_election, mock_lookup_member): + @mock.patch.object(utils, 'find_candidate_files') + def test_invalid_candidate(self, mock_candidates_list, mock_lookup_member): + mock_candidates_list.return_value = [ + 'candidates/fake/SomeProject/invalid@example.com' + ] mock_lookup_member.return_value = dict(data=[]) self.assertRaises(exception.MemberNotFoundException, @@ -65,14 +85,12 @@ class TestBuildCandidatesList(ElectionTestCase): 'fake') @mock.patch.object(utils, 'lookup_member') - @mock.patch.object(utils, 'is_tc_election', - return_value=False) - @mock.patch('os.path.exists', return_value=True) - @mock.patch('os.listdir', side_effect=[['SomeProject'], - ['invalid@example.com']]) - def test_valid_candidate(self, mock_listdir, mock_path_exists, - mock_is_tc_election, mock_lookup_member): + @mock.patch.object(utils, 'find_candidate_files') + def test_valid_candidate(self, mock_candidates_list, mock_lookup_member): + mock_candidates_list.return_value = [ + 'candidates/fake/SomeProject/invalid@example.com' + ] member = dict(irc='ircnick', first_name='Avery', last_name='Developer') diff --git a/openstack_election/utils.py b/openstack_election/utils.py index e261a73a..8e12c454 100644 --- a/openstack_election/utils.py +++ b/openstack_election/utils.py @@ -278,7 +278,7 @@ def election_is_running(): return False -def build_candidates_list(election=conf['release']): +def find_candidate_files(election=conf['release']): election_path = os.path.join(CANDIDATE_PATH, election) if os.path.exists(election_path): project_list = os.listdir(election_path) @@ -296,36 +296,44 @@ def build_candidates_list(election=conf['release']): project_list )) - project_list.sort() - candidates_lists = {} + candidate_files = [] for project in project_list: - project_prefix = os.path.join(CANDIDATE_PATH, election, project) - file_list = list(filter( + project_prefix = os.path.join(election_path, project) + candidate_files += list(filter( lambda x: '@' in x, - os.listdir(project_prefix), + [os.path.join(project_prefix, i) + for i in os.listdir(project_prefix)], )) - candidates_list = [] - for candidate_file in file_list: - filepath = os.path.join(project_prefix, candidate_file) - email = get_email(filepath) - member = lookup_member(email) - if member.get('data', []) == []: - raise exception.MemberNotFoundException(email=email) + candidate_files.sort() + return candidate_files - candidates_list.append( - { - 'url': ('%s/%s/plain/%s' % - (CGIT_URL, ELECTION_REPO, - quote_plus(filepath, safe='/'))), - 'email': email, - 'ircname': get_irc(member), - 'fullname': get_fullname(member, filepath=filepath) - }) - candidates_list.sort(key=lambda x: x['fullname']) - candidates_lists[project] = candidates_list +def build_candidates_list(election=conf['release']): + candidate_files = find_candidate_files(election=election) + candidates_lists = {} + projects = set() + for filepath in candidate_files: + project = os.path.basename(os.path.dirname(filepath)) + if project not in candidates_lists: + candidates_lists[project] = [] + projects.add(project) + + email = get_email(filepath) + member = lookup_member(email) + + if member.get('data', []) == []: + raise exception.MemberNotFoundException(email=email) + + candidates_lists[project].append({ + 'url': ('%s/%s/plain/%s' % + (CGIT_URL, ELECTION_REPO, + quote_plus(filepath, safe='/'))), + 'email': email, + 'ircname': get_irc(member), + 'fullname': get_fullname(member, filepath=filepath) + }) return {'election': election, - 'projects': project_list, + 'projects': list(projects), 'candidates': candidates_lists}