PR#410: More specific regex to detect bugs in changelog - fedpkg

fedpkg

#410 More specific regex to detect bugs in changelog

Merged 3 years ago by onosek. Opened 3 years ago by onosek.

onosek/fedpkg bodhi_regex into master

More specific regex to detect bugs in changelog

Ondrej Nosek • 3 years ago

e97f292

fedpkg/__main__.py

file modified

+1 -1

		`@@ -33,7 +33,7 @@`
		`default_user_config_path = os.path.join(`
		`os.path.expanduser('~'), '.config', 'rpkg', '%s.conf' % cli_name)`
		`# Setup an argparser and parse the known commands to get the config file`
		`- # - use the custom ArgumentParser class from pyrpkg.cli and dissable`
		`+ # - use the custom ArgumentParser class from pyrpkg.cli and disable`
		`# argument abbreviation to ensure that --user will be not treated as`
		`# --user-config`
		`parser = pyrpkg.cli.ArgumentParser(add_help=False, allow_abbrev=False)`

fedpkg/cli.py

file modified

+34 -1

		`@@ -686,6 +686,39 @@`
		`return True`
		`return False`

		`+ @staticmethod`
		`+ def _extract_issue_ids(text):`
		`+ """`
		`+ matching based on slightly modified expression taken from there:`
		`+ https://github.com/fedora-infra/bodhi/blob/5.3/bodhi/server/config.py`
		`+`
		`+ Examples:`
		`+ Fixes: rhbz#11111`
		`+ Fix: rh#22222`
		`+ resolves:rhbz#33333, rhbz#44444 rh#55555`
		`+ close: fedora#6666`
		`+ fix:epel#77777`
		`+ """`
		`+ # matches one Bugzilla bug ID (in case there are more IDs with one common prefix)`
		`+ id_pattern_raw = r'(?:fedora\|epel\|rh(?:bz)?)#(\d{5,})'`
		`+ bz_pattern = re.compile(`
		`+ # says: there is at least one complete (including prefix) Bugzilla bug occurrence`
		`+ r'(?:fix(?:es)?\|close(?:s)?\|resolve(?:s)?)(?::\|:\s+\|\s+)' + id_pattern_raw,`
		`+ re.IGNORECASE,`
		`+ )`
		`+ id_pattern = re.compile(id_pattern_raw, re.IGNORECASE)`
		`+`
		`+ issue_ids = set()`
		`+ for line in text.splitlines():`
		`+ # is there complete Bugzilla ID including prefix`
		`+ bz_match = bz_pattern.match(line)`
		`+ if bz_match is not None:`
		`+ # gather all Bugzilla IDs behind the prefix`
		`+ line_ids = re.findall(id_pattern, line)`
		`+ issue_ids.update(line_ids)`
		`+`
		`+ return sorted(list(issue_ids))`
		`+`
		`def _prepare_bodhi_template(self, template_file):`
		`try:`
		`nvr = self.cmd.nvr`
		`@@ -727,7 +760,7 @@`
		`bodhi_args['bugs'] = ','.join(self.args.bugs)`
		`else:`
		`# Extract bug numbers from the latest changelog entry`
		`- bugs = re.findall(r'#([0-9]+)', clog)`
		`+ bugs = self._extract_issue_ids(clog)`
		`if bugs:`
		`bodhi_args['bugs'] = ','.join(bugs)`

git-changelog

file modified

+28 -15

		`@@ -51,21 +51,34 @@`
		`return ret`

		`def _extract_issue_ids(self, s):`
		`- prefix_pattern = re.compile(r'^(Fix\|Fixes\|Bug\|Resolves?):? (.+)$', re.IGNORECASE)`
		`- prefix_match = prefix_pattern.match(s)`
		`- if prefix_match is None:`
		`- return`
		`- rest_s = prefix_match.groups()[1]`
		`- issue_ids = []`
		`- for match in re.findall(r'(BZ\|RHBZ\|bz\|rhbz)(:\|: \| )?(#?\d+)', rest_s):`
		`- issue_ids.append('{0}{1}'.format(match[0], match[2]))`
		`- if issue_ids:`
		`- # Sometimes, "BZ #1234" could confuse next step of search. So,`
		`- # remove them once found to avoid such confusion.`
		`- rest_s = re.sub(r'(BZ\|RHBZ\|bz\|rhbz)(:\|: \| )?(#?\d+)', '', rest_s)`
		`- issue_ids.extend(re.findall(r'#?\d+', rest_s))`
		`- issue_ids.sort()`
		`- return issue_ids`
		`+ """`
		`+ matching based on slightly modified expression taken from there:`
		`+ https://github.com/fedora-infra/bodhi/blob/5.3/bodhi/server/config.py`
		`+`
		`+ Examples:`
		`+ Fixes: rhbz#11111`
		`+ Fix: rh#22222`
		`+ resolves:rhbz#33333, rhbz#44444 rh#55555`
		`+ close: fedora#6666`
		`+ fix:epel#77777`
		`+ """`
		`+ # matches one Bugzilla bug ID (in case there are more IDs with one common prefix)`
		`+ id_pattern_raw = r'(?:fedora\|epel\|rh(?:bz)?)#(\d{5,})'`
		`+ bz_pattern = re.compile(`
		`+ # says: there is at least one complete (including prefix) Bugzilla bug occurrence`
		`+ r'(?:fix(?:es)?\|close(?:s)?\|resolve(?:s)?)(?::\|:\s+\|\s+)' + id_pattern_raw,`
		`+ re.IGNORECASE,`
		`+ )`
		`+ id_pattern = re.compile(id_pattern_raw, re.IGNORECASE)`
		`+`
		`+ # is there complete Bugzilla ID including prefix`
		`+ bz_match = bz_pattern.match(s)`
		`+ if bz_match is not None:`
		`+ # gather all Bugzilla IDs behind the prefix`
		`+ issue_ids = re.findall(id_pattern, s)`
		`+ return(sorted(issue_ids))`
		`+`
		`+ return []`

		`def _get_fixed_issues(self, commit):`
		`"""Get fixed issue or bug IDs from commit message body`

test/test_cli.py

file modified

+9 -7

		`@@ -120,9 +120,11 @@`
		`# Logs will be read in the tests which do not specify --notes option`
		`self.fake_clog = list(six.moves.map(six.u, [`
		`'Add tests for command update',`
		`- 'New command update - #1000',`
		`- 'Fix tests - #2000, #notabug',`
		`+ 'New command update - #1000', # invalid bug id format`
		`+ 'Fix tests - #2000, #notabug', # both invalid bug id format`
		`'处理一些Unicode字符číář',`
		`+ 'fix: rh#10001'`
		`+ 'Fixes: rhbz#20001'`
		`]))`
		`clog_file = os.path.join(self.cloned_repo_path, 'clog')`
		`with io.open(clog_file, 'w', encoding='utf-8') as f:`
		`@@ -188,7 +190,7 @@`

		`expected_data = {`
		`'autokarma': 'True',`
		`- 'bugs': '1000,2000',`
		`+ 'bugs': '10001,20001',`
		`'display_name': six.u(''),`
		`'builds': ' {0} '.format(self.mock_nvr.return_value),`
		`'close_bugs': True,`
		`@@ -226,7 +228,7 @@`
		`with io.open('bodhi.template', encoding='utf-8') as f:`
		`bodhi_template = f.read()`
		`self.assertTrue(self.mock_nvr.return_value in bodhi_template)`
		`- self.assertTrue('1000,2000' in bodhi_template)`
		`+ self.assertTrue('10001,20001' in bodhi_template)`
		`if notes:`
		`self.assertTrue(notes.replace('\n', '\n ') in bodhi_template)`
		`else:`
		`@@ -927,7 +929,7 @@`
		`@patch('fedpkg.cli.get_release_branches')`
		`@patch('sys.stdout', new=StringIO())`
		`def test_request_branch_override(self, mock_grb, mock_request_post):`
		`- """Tests request-branch with an overriden package and branch name"""`
		`+ """Tests request-branch with an overridden package and branch name"""`
		`mock_grb.return_value = {'fedora': ['f25', 'f26', 'f27'],`
		`'epel': ['el6', 'epel7']}`
		`mock_rv = Mock()`
		`@@ -1368,7 +1370,7 @@`
		`@patch('requests.get')`
		`def test_request_branch_invalid_epel_package(self, mock_get):`
		`"""Test request-branch raises an exception when an EPEL branch is`
		`- requested but ths package is already an EL package on all supported`
		`+ requested but this package is already an EL package on all supported`
		`arches"""`
		`mock_rv = Mock()`
		`mock_rv.ok = True`
		`@@ -2359,7 +2361,7 @@`
		`with patch('sys.argv', ['fedpkg', '--release', branch, 'retire', 'retire_message']):`
		`cli = self.new_cli(cfg='fedpkg-test.conf')`
		`# retire method in rpkg would be called, but there is not environment configured`
		`- # therefore just Exception is catched`
		`+ # therefore just Exception is caught`
		`with self.assertRaises(Exception):`
		`cli.args.path = '/repo_path'`
		`cli.retire()`

onosek commented 3 years ago

Use regex similar to Bodhi's regex (with some minor changes)

Fixes: #404
JIRA: RHELCMP-815
Signed-off-by: Ondrej Nosek onosek@redhat.com

lsedlar commented 3 years ago

Generally looks good to me. If this is based on some Bodhi code, it might be nice to add a link to it in the commit message.

:thumbsup:

onosek commented 3 years ago

Actually, I didn't find a corresponding code in Bodhi. This was suggested to me by @mattia as "Bodhi code". Where is the repo I can check it? Otherwise, I will remove (or update) this comment to more correct form.
I tried just https://github.com/fedora-infra/bodhi and didn't find it there.