PR#163: Make Pagure vote parser more liberal and also safer - fedora-qa/blockerbugs

		`@@ -1,6 +1,5 @@`
		`#!/usr/bin/python3`

		`- import itertools`
		`import datetime`
		`import re`

		`@@ -10,63 +9,155 @@`
		`TRACKER_KEYWORDS = [`
		`'betablocker',`
		`'finalblocker',`
		`- 'betafe',`
		`- 'finalfe',`
		`+ 'betafreezeexception',`
		`+ 'finalfreezeexception',`
		`'0day',`
		`'previousrelease'`
		`]`

		`-`
		`- VOTE_KEYWORDS = [' '.join(v) for v in`
		`- itertools.product(TRACKER_KEYWORDS, ['-1', '0', '+1'])]`
		`-`
		`- OUTCOME_KEYWORDS = [''.join(r) for r in`
		`- itertools.product(['accepted', 'rejected'], TRACKER_KEYWORDS)]`
		`-`
		`- REVOTE_KEYWORDS = ['revote ' + t for t in TRACKER_KEYWORDS]`
		`-`
		`- #ADMIN_KEYWORDS = RESULT_KEYWORDS + REVOTE_KEYWORDS`
		`+ VOTES = ("+1", "0", "-1")`
		`+ TRACKER_RE = r'((beta\|final)(blocker\|fe\|freezeexception)\|0day\|previousrelease)'`
		`+ TRACKER_MATCHER = re.compile(TRACKER_RE)`
		`+ OUTCOME_RE = r'(accepted\|rejected)' + TRACKER_RE`
		`+ OUTCOME_MATCHER = re.compile(OUTCOME_RE)`

		`SUMMARY_HEADER = ""`
		`CLOSED_VOTES_HEADER = "The following votes have been closed:"`


		`- def agreed_parser(line):`
		`+ def agreed_revote_parser(line):`
		`+ """Parse an AGREED or a REVOTE line. If there's anything violating the`
		`+ rules for the line (e.g. it doesn't start with the expected command, or`
		`+ there's some extra text that shouldn't be there), ignore the line contents`
		`+ and return []. All instances of "fe" get expanded to "freezeexception".`
		`+`
		`+ :param str line: a single line`
		`+ :return: A list of commands with one tracker per line (or an empty list).`
		`+ E.g. for line 'agreed acceptedbetablocker rejectedfinalfe' this returns:`
		`+ ['agreed acceptedbetablocker',`
		`+ 'agreed rejectedfinalfreezeexception']`
		`+ For line 'revote finalblocker 0day' this returns:`
		`+ ['revote finalblocker',`
		`+ 'revote 0day']`
		`+ """`
		`+ # parsing word by word avoids complex and fragile regex syntax`
		`+ words = line.split()`
		`+ # first word must be 'agreed' or 'revote'`
		`+ if len(words) <= 0:`
		`+ return []`
		`+ command = words[0]`
		`+ if command == 'agreed':`
		`+ matcher = OUTCOME_MATCHER`
		`+ summary = True`
		`+ elif command == 'revote':`
		`+ matcher = TRACKER_MATCHER`
		`+ summary = False`
		`+`
		`out = []`
		`- outcome_pattern = '\|'.join(OUTCOME_KEYWORDS)`
		`- #pattern = '^agreed (({}) )+.*$'.format(outcome_pattern)`
		`- pattern = '^agreed ({})( ({}))( .)?$'.format(outcome_pattern, outcome_pattern)`
		`- m = re.match(pattern, line)`
		`- if m:`
		`- for outcome in OUTCOME_KEYWORDS:`
		`- if outcome in line:`
		`- out.append('agreed ' + outcome)`
		`+ for word in words[1:]:`
		`+ if matcher.fullmatch(word):`
		`+ # if we get a match, save it as a valid command`
		`+ word = expand_fe(word)`
		`+ out.append(f'{command} {word}')`
		`+ else:`
		`+ # if not, we break and return the matches we got so far if summary`
		`+ # is allowed, return empty if not`
		`+ if summary:`
		`+ break`
		`+ return []`
		`return out`

		`- def revote_parser(line):`
		`+`
		`+ def vote_parser(line):`
		`+ """Look for votes in a single line. We accept pairs of a vote and a match`
		`+ of TRACKER_RE, in either order, and nothing else; any other text`
		`+ invalidates the line. All instances of "fe" get expanded to "freezeexception".`
		`+`
		`+ :param str line: a line which might contain a vote`
		`+ :return: A list of votes with one tracker per line (or an empty list).`
		`+ E.g. for line 'betafe +1 finalblocker -1' this returns:`
		`+ ['betafreezeexception +1',`
		`+ 'finalblocker -1']`
		`+ """`
		`out = []`
		`- trackers = '\|'.join(TRACKER_KEYWORDS)`
		`- pattern = '^revote (({}) ?)+$'.format(trackers)`
		`- m = re.match(pattern, line)`
		`- if m:`
		`- for tracker in TRACKER_KEYWORDS:`
		`- if tracker in line:`
		`- out.append('revote ' + tracker)`
		`+ words = line.split()`
		`+ # if we have an odd number of words we don't have a clean vote line`
		`+ if (len(words) % 2) != 0:`
		`+ return out`
		`+ # split list into pairs:`
		`+ # https://stackoverflow.com/questions/312443`
		`+ for pair in (words[i:i + 2] for i in range(0, len(words), 2)):`
		`+ if TRACKER_MATCHER.fullmatch(pair[0]) and pair[1] in VOTES:`
		`+ pair[0] = expand_fe(pair[0])`
		`+ out.append(" ".join(pair))`
		`+ elif pair[0] in VOTES and TRACKER_MATCHER.fullmatch(pair[1]):`
		`+ pair[1] = expand_fe(pair[1])`
		`+ pair.reverse()`
		`+ out.append(" ".join(pair))`
		`+ else:`
		`+ # we found something other than a vote pair, so per`
		`+ # Kamil's Strict Vote Parsing Regime, we reject the line`
		`+ return []`
		`return out`


		`+ def expand_fe(keyword):`
		`+ '''Expand "FE" into "FreezeException" in tracker-related keywords.`
		`+`
		`+ While we accept "FE" as a handy user abbreviation, we want to process`
		`+ it as "FreezeException" internally, so that we don't need to handle both`
		`+ variants everywhere in our code.`
		`+`
		`+ :param str keyword: a tracker or an outcome keyword, lowercased`
		`+ :return: a keyword expanded to the full "FreezeException" variant if`
		`+ applicable, or the keyword unchanged)`
		`+ '''`
		`+ tracker_match = TRACKER_MATCHER.fullmatch(keyword)`
		`+ outcome_match = OUTCOME_MATCHER.fullmatch(keyword)`
		`+`
		`+ if tracker_match:`
		`+ # the 'fe' string is in group #3 in TRACKER_RE`
		`+ fe_group_num = 3`
		`+ match = tracker_match`
		`+ elif outcome_match:`
		`+ # the 'fe' string is in group #4 in OUTCOME_RE`
		`+ fe_group_num = 4`
		`+ match = outcome_match`
		`+ else:`
		`+ return keyword`
		`+`
		`+ if match.group(fe_group_num) != 'fe':`
		`+ return keyword`
		`+`
		`+ start = match.start(fe_group_num)`
		`+ end = start + len('fe')`
		`+ return keyword[:start] + 'freezeexception' + keyword[end:]`
		`+`
		`+`
		`def bot_loop_detected(comments):`
		`+ '''Detect a loop of bot comments in the list of comments. The loop is`
		`+ defined as a continuous stream of more than $threshold comments, all from`
		`+ the bot, including the last comment in the list. The threshold is defined`
		`+ in config.`
		`+`
		`+ :param comments: a list of Comment instances`
		`+ :return: bool whether there is a loop`
		`+ '''`
		`is_bot_comment = [comment.user == app.config['PAGURE_BOT_USERNAME'] for comment in comments]`
		`- if len(is_bot_comment) < app.config['PAGURE_BOT_LOOP_THRESHOLD']:`
		`+ threshold = app.config['PAGURE_BOT_LOOP_THRESHOLD']`
		`+ if len(is_bot_comment) < threshold:`
		`return False`

		`- return all(is_bot_comment[-app.config['PAGURE_BOT_LOOP_THRESHOLD']:])`
		`+ return all(is_bot_comment[-threshold:])`


		`class Comment():`
		`+ '''This represents a single comment from the voting ticket.`
		`+ '''`
		`def __init__(self, raw_comment):`
		`self.text = raw_comment['comment'].replace('\r', '')`
		`self.user = raw_comment['user']['name']`
		`+ #: Pagure's comment id (int)`
		`self.id = raw_comment['id']`

		`def __repr__(self):`
		`@@ -78,93 +169,141 @@`
		`return self.user in pagure_interface.get_group('fedora-qa')['members']`

		`def keywords(self):`
		`+ '''Search for command keywords in this comment, purge anything`
		`+ unrelated and return it.`
		`+`
		`+ :return: a list with one vote or command per line, e.g.:`
		`+ ['betablocker +1',`
		`+ 'agreed acceptedbetablocker',`
		`+ 'revote betablocker',`
		`+ 'betablocker -1']`
		`+ '''`
		`out = []`
		`for line in self.text.lower().split('\n'):`
		`- line = line.rstrip()`
		`- line = re.sub(' +', ' ', line)`
		`- if line in VOTE_KEYWORDS:`
		`- out.append(line)`
		`- elif line.startswith('agreed'):`
		`- out.extend(agreed_parser(line))`
		`- elif line.startswith('revote'):`
		`- out.extend(revote_parser(line))`
		`+ line = line.strip()`
		`+ if line.startswith('agreed') or line.startswith('revote'):`
		`+ out.extend(agreed_revote_parser(line))`
		`+ else:`
		`+ out.extend(vote_parser(line))`
		`return out`

		`def is_summary_post_of(self, tracker_keyword):`
		`- is_bot = self.user == app.config['PAGURE_BOT_USERNAME']`
		`- is_summary = CLOSED_VOTES_HEADER.lower() in self.text.lower()`
		`- is_tracker_relevant = tracker_keyword in self.text.lower()`
		`+ '''Return True/False whether this particular post is a summary of`
		`+ voting results for a particular tracker.`
		`+ '''`
		`+ is_bot = (self.user == app.config['PAGURE_BOT_USERNAME'])`
		`+ is_summary = (CLOSED_VOTES_HEADER.lower() in self.text.lower())`
		`+ is_tracker_relevant = (tracker_keyword in self.text.lower())`

		`return is_bot and is_summary and is_tracker_relevant`


		`class BugVoteTracker():`
		`- def __init__(self, accepted_keyword):`
		`+ '''Count votes specific for a single tracker. Just feed all comments to`
		`+ this class and it'll reflect the tracker's final state in its variables.`
		`+ '''`
		`+`
		`+ def __init__(self, tracker):`
		`self.open = True`
		`+ #: 'accepted' or 'rejected'`
		`self.outcome = None`
		`- self.accepted_keyword = accepted_keyword`
		`+ #: one of TRACKER_KEYWORDS`
		`+ self.tracker = tracker`
		`self.need_summary_post = False`

		`- # vote format: {`
		`- # 'user': {`
		`- # 'vote': '+1'`
		`- # 'comment_id': '21383'`
		`- # }`
		`- # }`
		`+ #: user votes in this format:`
		`+ #: {`
		`+ #: 'user': {`
		`+ #: 'vote': '+1'`
		`+ #: 'comment_id': 21383`
		`+ #: }`
		`+ #: }`
		`self.votes = {}`

		`- def _filter_relevant(self, keywords):`
		`- accepted_votes = [self.accepted_keyword + v for v in [' +1', ' 0', ' -1']]`
		`- accepted_agreed = ['agreed ' + o + self.accepted_keyword for o in ['accepted', 'rejected']]`
		`- accepted_revote = ['revote ' + self.accepted_keyword]`
		`- accepted_keywords = accepted_votes + accepted_agreed + accepted_revote`
		`- return [keyword for keyword in keywords if keyword in accepted_keywords]`
		`-`
		`def parse_comment(self, comment):`
		`- if self.need_summary_post:`
		`- self.need_summary_post = not comment.is_summary_post_of(self.accepted_keyword)`
		`+ '''Take a comment and parse all commands from it that are relevant to`
		`+ this particular tracker. Update the instance variables.'''`
		`+`
		`+ if comment.is_summary_post_of(self.tracker):`
		`+ self.need_summary_post = False`
		`+ return`
		`+`
		`+ # now that we detected summary posts, we can ignore all posts from the bot`
		`+ if comment.user == app.config['PAGURE_BOT_USERNAME']:`
		`+ return`
		`+`
		`+ for words in [line.split() for line in comment.keywords()]:`
		`+ agreed = (words[0] == 'agreed')`
		`+ revote = (words[0] == 'revote')`
		`+`
		`+ if agreed:`
		`+ match = OUTCOME_MATCHER.fullmatch(words[1])`
		`+ outcome = match.group(1)`
		`+ tracker = match.group(2)`
		`+ elif revote:`
		`+ tracker = words[1]`
		`+ else: # a vote`
		`+ tracker = words[0]`
		`+`
		`+ if tracker != self.tracker:`
		`+ continue`

		`- keywords = self._filter_relevant(comment.keywords())`
		`- for keyword in keywords:`
		`- # FIXME log rejected (non admin) users?`
		`- if keyword.startswith('agreed') and comment.user_is_admin():`
		`+ if (agreed or revote) and not comment.user_is_admin():`
		`+ app.logger.debug(`
		`+ f'A non-admin user {comment.user} tries to perform '`
		`+ f'administrative commands in comment {comment.id}, '`
		`+ 'ignoring.')`
		`+ continue`
		`+`
		`+ if agreed:`
		`self.open = False`
		`- self.outcome = keyword[7:15] # eh...`
		`+ self.outcome = outcome`
		`self.need_summary_post = True`
		`- elif keyword.startswith('revote') and comment.user_is_admin():`
		`+ elif revote:`
		`self.open = True`
		`+ self.outcome = None`
		`+ self.need_summary_post = False`
		`self.votes = {}`
		`- else:`
		`+ else: # a vote`
		`if not self.open:`
		`continue`
		`- for vote in ['-1', '0', '+1']:`
		`- if keyword.endswith(vote):`
		`- self.votes[comment.user] = {`
		`- 'vote': vote,`
		`- 'comment_id': comment.id`
		`- }`
		`- return keywords`
		`+ assert words[1] in VOTES`
		`+ self.votes[comment.user] = {`
		`+ 'vote': words[1],`
		`+ 'comment_id': comment.id`
		`+ }`

		`def enumerate_votes(self):`
		`- out = {`
		`- '-1': [],`
		`- '0': [],`
		`- '+1': []`
		`+ '''Return self.votes in a format:`
		`+ { '-1': [('user1', comment_id), ('user2', comment_id)],`
		`+ '0': [...],`
		`+ '+1': [...],`
		`}`
		`+ '''`
		`+ out = {vote: [] for vote in VOTES}`
		`for user, vote in self.votes.items():`
		`out[vote['vote']].append((user, vote['comment_id']))`

		`return out`

		`+`
		`def link(user, comment_id):`
		`+ '''Create a Markdown snippet containing a link to the user's comment.`
		`+ '''`
		`return "[%s](#comment-%s)" % (user, comment_id)`

		`+`
		`def tracker_summary(tracker_name, tracker):`
		`+ '''Create a summary of votes for a given tracker in a Markdown format.`
		`+`
		`+ :param str tracker_name: e.g. 'BetaBlocker'`
		`+ :param tracker: an instance of BugVoteTracker`
		`+ '''`
		`nicer = {`
		`'betablocker': 'BetaBlocker',`
		`'finalblocker': 'FinalBlocker',`
		`- 'betafe': 'BetaFE',`
		`- 'finalfe': 'FinalFE',`
		`+ 'betafreezeexception': 'BetaFreezeException',`
		`+ 'finalfreezeexception': 'FinalFreezeException',`
		`'0day': '0Day',`
		`'previousrelease': 'PreviousRelease',`
		`'accepted': 'Accepted',`
		`@@ -196,7 +335,16 @@`

		`return out`

		`+`
		`def summary(trackers, non_voting_users=None, last_comment_id=None, header=CLOSED_VOTES_HEADER):`
		`+ '''Create a summary text in a Markdown format which can be used either in`
		`+ a ticket description, or as a comment when a vote is closed.`
		`+`
		+ :param trackers: a dict as returned from :func:`parse_comments_to_trackers`
		`+ :param non_voting_users: a list of users who haven't voted yet`
		`+ :param int last_comment_id: the last comment id that was counted`
		`+ :param str head: an initial message text`
		`+ '''`
		`md_text = "%s\n\n" % header`

		`for tracker_name, tracker in trackers.items():`
		`@@ -209,27 +357,39 @@`

		`md_text += tracker_summary(tracker_name, tracker)`

		`- for neco in ['+1', '0', '-1']:`
		`- votes[neco] = [link(user, comment_id) for (user, comment_id) in votes[neco]]`
		`- if len(votes[neco]) == 0:`
		`+ for vote in VOTES:`
		`+ votes[vote] = [link(user, comment_id) for (user, comment_id) in votes[vote]]`
		`+ if len(votes[vote]) == 0:`
		`continue`
		`- md_text += " * %s by %s\n" % (neco, ', '.join(votes[neco]))`
		`+ md_text += " * %s by %s\n" % (vote, ', '.join(votes[vote]))`

		`md_text += '\n'`

		`if non_voting_users:`
		`- md_text += "Commented but haven't voted yet: %s\n\n" % ', '.join([link(u, c) for (u, c) in non_voting_users.items()])`
		`+ md_text += "Commented but haven't voted yet: %s\n\n" % ', '.join(`
		`+ [link(u, c) for (u, c) in non_voting_users.items()])`

		`if last_comment_id:`
		`time = datetime.datetime.utcnow().strftime("%Y-%m-%d %H:%M")`

		`md_text += ("*The votes have been last counted at %s UTC and the last "`
		`- "processed comment was [#comment-%s](#comment-%s)*" % (time, last_comment_id, last_comment_id))`
		`+ "processed comment was [#comment-%s](#comment-%s)*" % (`
		`+ time, last_comment_id, last_comment_id))`

		`return md_text`


		`def parse_comments_to_trackers(comments):`
		`+ '''Create BugVoteTracker for each tracker keyword supported, and then feed`
		`+ all comments to each tracker to fully update them.`
		`+`
		`+ :param comments: a list of Comment instances`
		`+ :return: A dict in format:`
		`+ {'betablocker': BugVoteTracker,`
		`+ 'betafreezeexception': BugVoteTracker,`
		`+ ...`
		`+ }`
		`+ '''`
		`trackers = {tracker: BugVoteTracker(tracker) for tracker in TRACKER_KEYWORDS}`

		`for comment in comments:`
		`@@ -240,6 +400,16 @@`


		`def voting_info(comments, trackers):`
		`+ '''Compute users who haven't voted yet, and the last processed comment`
		`+`
		`+ :param comments: a list of Comment instances`
		+ :param trackers: a dict as returned from :func:`parse_comments_to_trackers`
		`+ :return: a tuple of (dict of non_voting_users, last_comment_id), e.g.:`
		`+ ({'user1': 1234,`
		`+ 'user2': 1235`
		`+ },`
		`+ 1238)`
		`+ '''`
		`last_comment_id = 0`
		`non_voting_users = {}`
		`voting_users = set()`
		`@@ -259,6 +429,8 @@`


		`def webhook_handler(issue_id):`
		`+ '''React to a Pagure webhook notification that a discussion ticket changed.`
		`+ '''`
		`comments = [Comment(c) for c in pagure_interface.get_issue_comments(issue_id)]`

		`trackers = parse_comments_to_trackers(comments)`

docs/source/review-voting.md

file modified

+16 -10

		`@@ -18,16 +18,17 @@`

		`You can Watch issues in this repo, which will send you an email for every new proposed bug and every new comment in any ticket. If you wish to participate in blocker review discussions in general and not just in a single topic, this is the best way to subscribe.`

		`+`
		`### How to vote`
		`Place a voting command on a separate line in the form of:<br/>`
		`TRACKER VOTE`

		- where `TRACKER` is one of:
		+ where `TRACKER` is one of these words (case-insensitive):

		* `BetaBlocker`
		* `FinalBlocker`
		- * `BetaFE` - FE standing for FreezeException
		- * `FinalFE` - FE standing for FreezeException
		+ * `BetaFreezeException` (or `BetaFE`)
		+ * `FinalFreezeException` (or `FinalFE`)
		* `0Day`
		* `PreviousRelease`

		`@@ -37,18 +38,23 @@`
		* `0` ([no strong feelings one way or the other](https://www.youtube.com/watch?v=ussCHoQttyQ) or to take your vote back)
		* `-1` (to vote against)

		`- If you want to cast several votes, use several separate lines (in one or more comments):`
		`+ Example:`
		```
		`- BetaBlocker -1`
		`- BetaFE +1`
		`- FinalBlocker +1`
		`+ BetaBlocker +1`
		+ ```
		`+ If you want to cast several votes, you can use several separate lines (in one or more comments), or a single line:`
		+ ```
		`+ BetaBlocker -1 BetaFE +1 FinalBlocker +1`
		+ ```
		`+ You can swap the word order as well:`
		+ ```
		`+ +1 FinalBlocker`
		```
		`Voting commands must not be mixed with arbitrary other words, this will not be counted:`
		```
		`- BetaBlocker +1 haha`
		`+ BetaBlocker +1 for sure`
		```
		`-`
		`- If you vote for the same tracker multiple times, only your last vote is counted. For example, if you submit these three comments:`
		`+ If you vote for the same tracker multiple times, only your last vote is counted. For example, if you submit these three votes (in one or more comments):`
		```
		`BetaBlocker +1`
		`BetaFE +1`

testing/test_pagure_bot.py

file modified

+447 -194

		`@@ -6,6 +6,7 @@`
		`from blockerbugs.util import pagure_interface`
		`from blockerbugs.util import pagure_bot`

		`+ EXTENDED_TRACKERS = pagure_bot.TRACKER_KEYWORDS + ["betafe", "finalfe"]`

		`def powerset(iterable):`
		`s = list(iterable)`
		`@@ -31,6 +32,14 @@`
		`"name": "fake-group"`
		`}`

		`+ def expand_fe(tracker):`
		`+ if tracker == 'betafe':`
		`+ return 'betafreezeexception'`
		`+ elif tracker == 'finalfe':`
		`+ return 'finalfreezeexception'`
		`+ else:`
		`+ return tracker`
		`+`
		`@pytest.fixture`
		`def setup_stubs(monkeypatch):`
		`stub_pagure_group_call = mock.MagicMock(return_value=stub_group_data)`
		`@@ -38,13 +47,14 @@`


		`class TestPagureBotBasics(object):`
		`- @pytest.mark.parametrize("keyword", pagure_bot.TRACKER_KEYWORDS)`
		`+ @pytest.mark.parametrize("inversed", [False, True])`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`@pytest.mark.parametrize("vote", ['+1', '-1', '0'])`
		`- def test_basic_vote(self, vote, keyword):`
		`+ def test_basic_vote(self, vote, keyword, inversed):`
		`comments = [pagure_bot.Comment(c) for c in [`
		`{`
		`'id': 1,`
		`- 'comment': "%s %s" % (keyword, vote),`
		`+ 'comment': "%s %s" % ((vote, keyword) if inversed else (keyword, vote)),`
		`'user': {`
		`'name': stub_admin_user1`
		`}`
		`@@ -52,14 +62,14 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword].votes == {`
		`+ assert trackers[expand_fe(keyword)].votes == {`
		`stub_admin_user1: {`
		`'vote': '%s' % vote,`
		`'comment_id': 1`
		`}`
		`}`

		`- @pytest.mark.parametrize("keyword", pagure_bot.TRACKER_KEYWORDS)`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`def test_basic_vote_repeated(self, keyword):`
		`comments = [pagure_bot.Comment(c) for c in [`
		`{`
		`@@ -79,7 +89,7 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword].votes == {`
		`+ assert trackers[expand_fe(keyword)].votes == {`
		`stub_admin_user1: {`
		`'vote': '-1',`
		`'comment_id': 2`
		`@@ -88,7 +98,7 @@`


		`class TestPagureBotTypos(object):`
		`- @pytest.mark.parametrize("keyword", pagure_bot.TRACKER_KEYWORDS)`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`@pytest.mark.parametrize("sign", ['+', '-'])`
		`def test_typo(self, sign, keyword):`
		`keyword_typo = list(keyword)`
		`@@ -105,17 +115,48 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword].votes == {}`
		`+ assert trackers[expand_fe(keyword)].votes == {}`
		`+`
		`+ def test_multivote_extra_comma(self):`
		`+ comments = [pagure_bot.Comment(c) for c in [`
		`+ {`
		`+ 'id': 1,`
		`+ 'comment': "BetaBlocker -1 BetaFE +1, FinalBocker -1",`
		`+ 'user': {`
		`+ 'name': stub_admin_user1`
		`+ }`
		`+ }`
		`+ ]]`
		`+`
		`+ trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`+ for tracker in trackers.values():`
		`+ assert tracker.votes == {}`
		`+`
		`+ def test_multivote_extra_vote(self):`
		`+ comments = [pagure_bot.Comment(c) for c in [`
		`+ {`
		`+ 'id': 1,`
		`+ 'comment': "BetaBlocker -1 +1 FinalBlocker -1 BetaFE -1",`
		`+ 'user': {`
		`+ 'name': stub_admin_user1`
		`+ }`
		`+ }`
		`+ ]]`
		`+`
		`+ trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`+ for tracker in trackers.values():`
		`+ assert tracker.votes == {}`


		`class TestPagureBotGotchas(object):`
		`- @pytest.mark.parametrize("keyword", pagure_bot.TRACKER_KEYWORDS)`
		`+ @pytest.mark.parametrize("inversed", [False, True])`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`@pytest.mark.parametrize("sign", ['＋', '–']) # these chars are not + or -`
		`- def test_unicode_plus(self, sign, keyword):`
		`+ def test_unicode_plus(self, sign, keyword, inversed):`
		`comments = [pagure_bot.Comment(c) for c in [`
		`{`
		`'id': 1,`
		`- 'comment': "%s %s1" % (keyword, sign),`
		`+ 'comment': "%s1 %s" % (sign, keyword) if inversed else "%s %s1" % (keyword, sign),`
		`'user': {`
		`'name': stub_admin_user1`
		`}`
		`@@ -123,15 +164,16 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword].votes == {}`
		`+ assert trackers[expand_fe(keyword)].votes == {}`

		`- @pytest.mark.parametrize("keyword", pagure_bot.TRACKER_KEYWORDS)`
		`+ @pytest.mark.parametrize("inversed", [False, True])`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`@pytest.mark.parametrize("sign", ['+', '-'])`
		`- def test_plusminus_eleven(self, sign, keyword):`
		`+ def test_plusminus_eleven(self, sign, keyword, inversed):`
		`comments = [pagure_bot.Comment(c) for c in [`
		`{`
		`'id': 1,`
		`- 'comment': "%s %s11" % (keyword, sign),`
		`+ 'comment': "%s11 %s" % (sign, keyword) if inversed else "%s %s11" % (keyword, sign),`
		`'user': {`
		`'name': stub_admin_user1`
		`}`
		`@@ -139,15 +181,16 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword].votes == {}`
		`+ assert trackers[expand_fe(keyword)].votes == {}`

		`- @pytest.mark.parametrize("keyword", pagure_bot.TRACKER_KEYWORDS)`
		`+ @pytest.mark.parametrize("inversed", [False, True])`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`@pytest.mark.parametrize("sign", ['+', '-'])`
		`- def test_plusminus_two(self, sign, keyword):`
		`+ def test_plusminus_two(self, sign, keyword, inversed):`
		`comments = [pagure_bot.Comment(c) for c in [`
		`{`
		`'id': 1,`
		`- 'comment': "%s %s2" % (keyword, sign),`
		`+ 'comment': "%s2 %s" % (sign, keyword) if inversed else "%s %s2" % (keyword, sign),`
		`'user': {`
		`'name': stub_admin_user1`
		`}`
		`@@ -155,15 +198,16 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword].votes == {}`
		`+ assert trackers[expand_fe(keyword)].votes == {}`

		`- @pytest.mark.parametrize("keyword", pagure_bot.TRACKER_KEYWORDS)`
		`+ @pytest.mark.parametrize("inversed", [False, True])`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`@pytest.mark.parametrize("sign", ['+', '-'])`
		`- def test_plusminus_typo(self, sign, keyword):`
		`+ def test_plusminus_typo(self, sign, keyword, inversed):`
		`comments = [pagure_bot.Comment(c) for c in [`
		`{`
		`'id': 1,`
		`- 'comment': "%s %s!" % (keyword, sign),`
		`+ 'comment': "%s! %s" % (sign, keyword) if inversed else "%s %s!" % (keyword, sign),`
		`'user': {`
		`'name': stub_admin_user1`
		`}`
		`@@ -171,15 +215,16 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword].votes == {}`
		`+ assert trackers[expand_fe(keyword)].votes == {}`

		`- @pytest.mark.parametrize("keyword", pagure_bot.TRACKER_KEYWORDS)`
		`+ @pytest.mark.parametrize("inversed", [False, True])`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`@pytest.mark.parametrize("sign", ['+', '-'])`
		`- def test_plusminus_float(self, sign, keyword):`
		`+ def test_plusminus_float(self, sign, keyword, inversed):`
		`comments = [pagure_bot.Comment(c) for c in [`
		`{`
		`'id': 1,`
		`- 'comment': "%s %s0.5" % (keyword, sign),`
		`+ 'comment': "%s0.5 %s" % (sign, keyword) if inversed else "%s %s0.5" % (keyword, sign),`
		`'user': {`
		`'name': stub_admin_user1`
		`}`
		`@@ -187,15 +232,16 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword].votes == {}`
		`+ assert trackers[expand_fe(keyword)].votes == {}`

		`- @pytest.mark.parametrize("keyword", pagure_bot.TRACKER_KEYWORDS)`
		`+ @pytest.mark.parametrize("inversed", [False, True])`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`@pytest.mark.parametrize("sign", ['+', '-'])`
		`- def test_plusminus_emoji(self, sign, keyword):`
		`+ def test_plusminus_emoji(self, sign, keyword, inversed):`
		`comments = [pagure_bot.Comment(c) for c in [`
		`{`
		`'id': 1,`
		`- 'comment': "%s %s👎" % (keyword, sign),`
		`+ 'comment': "%s👎 %s" % (sign, keyword) if inversed else "%s %s👎" % (keyword, sign),`
		`'user': {`
		`'name': stub_admin_user1`
		`}`
		`@@ -203,15 +249,16 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword].votes == {}`
		`+ assert trackers[expand_fe(keyword)].votes == {}`

		`- @pytest.mark.parametrize("keyword", pagure_bot.TRACKER_KEYWORDS)`
		`+ @pytest.mark.parametrize("inversed", [False, True])`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`@pytest.mark.parametrize("sign", ['+', '-'])`
		`- def test_plusminus_backtick(self, sign, keyword):`
		`+ def test_plusminus_backtick(self, sign, keyword, inversed):`
		`comments = [pagure_bot.Comment(c) for c in [`
		`{`
		`'id': 1,`
		- 'comment': "`%s %s1`" % (keyword, sign),
		+ 'comment': "`%s1 %s`" % (sign, keyword) if inversed else "`%s %s1`" % (keyword, sign),
		`'user': {`
		`'name': stub_admin_user1`
		`}`
		`@@ -219,17 +266,18 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword].votes == {}`
		`+ assert trackers[expand_fe(keyword)].votes == {}`


		`class TestPagureBotMultiline(object):`
		`- @pytest.mark.parametrize("keyword", pagure_bot.TRACKER_KEYWORDS)`
		`+ @pytest.mark.parametrize("inversed", [False, True])`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`@pytest.mark.parametrize("sign", ['+', '-'])`
		`- def test_multiline_singlevote(self, sign, keyword):`
		`+ def test_multiline_singlevote(self, sign, keyword, inversed):`
		`comments = [pagure_bot.Comment(c) for c in [`
		`{`
		`'id': 1,`
		`- 'comment': "foobar\n%s %s1\nbarbaz" % (keyword, sign),`
		`+ 'comment': "foobar\n%s1 %s\nbarbaz" % (sign, keyword) if inversed else "foobar\n%s %s1\nbarbaz" % (keyword, sign),`
		`'user': {`
		`'name': stub_admin_user1`
		`}`
		`@@ -237,24 +285,96 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword].votes == {`
		`+ assert trackers[expand_fe(keyword)].votes == {`
		`stub_admin_user1: {`
		`'vote': '%s1' % sign,`
		`'comment_id': 1`
		`}`
		`}`

		`- @pytest.mark.parametrize("keyword", pagure_bot.TRACKER_KEYWORDS)`
		`+ @pytest.mark.parametrize("inversed", itertools.product([False, True], [False, True]))`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`+ @pytest.mark.parametrize("vote", [('+1', '-1'), ('-1', '+1'), ('+1', '0'),`
		`+ ('-1', '0'), ('0', '-1'), ('0', '+1')])`
		`+ def test_multiline_multivote_same(self, vote, keyword, inversed):`
		`+ vote_first = vote[0]`
		`+ vote_second = vote[1]`
		`+ first = (vote_first, keyword) if inversed[0] else (keyword, vote_first)`
		`+ second = (vote_second, keyword) if inversed[1] else (keyword, vote_second)`
		`+ comments = [pagure_bot.Comment(c) for c in [`
		`+ {`
		`+ 'id': 1,`
		`+ 'comment': "foobar\n%s %s\nbarbaz\n%s %s" % (first + second),`
		`+ 'user': {`
		`+ 'name': stub_admin_user1`
		`+ }`
		`+ }`
		`+ ]]`
		`+`
		`+ trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`+ assert trackers[expand_fe(keyword)].votes == {`
		`+ stub_admin_user1: {`
		`+ 'vote': '%s' % vote_second,`
		`+ 'comment_id': 1`
		`+ }`
		`+ }`
		`+`
		`+ @pytest.mark.parametrize("inversed", itertools.product([False, True], [False, True]))`
		`+ @pytest.mark.parametrize("keyword", itertools.permutations(EXTENDED_TRACKERS, 2))`
		`+ @pytest.mark.parametrize("vote", [('+1', '-1'), ('-1', '+1'), ('+1', '0'),`
		`+ ('-1', '0'), ('0', '-1'), ('0', '+1'),`
		`+ ('0', '0'), ('-1', '-1'), ('+1', '+1')])`
		`+ def test_multiline_multivote_different(self, vote, keyword, inversed):`
		`+ keyword_first = keyword[0]`
		`+ keyword_second = keyword[1]`
		`+ # if we hit two keywords which are synonyms for each other,`
		`+ # move on, test is invalid`
		`+ if keyword_first.replace("fe", "freezeexception") == keyword_second.replace("fe", "freezeexception"):`
		`+ return None`
		`+ vote_first = vote[0]`
		`+ vote_second = vote[1]`
		`+ first = (vote_first, keyword_first) if inversed[0] else (keyword_first, vote_first)`
		`+ second = (vote_second, keyword_second) if inversed[1] else (keyword_second, vote_second)`
		`+ comments = [pagure_bot.Comment(c) for c in [`
		`+ {`
		`+ 'id': 1,`
		`+ 'comment': "foobar\n%s %s\nbarbaz\n%s %s" % (first + second),`
		`+ 'user': {`
		`+ 'name': stub_admin_user1`
		`+ }`
		`+ }`
		`+ ]]`
		`+`
		`+ trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`+ assert trackers[expand_fe(keyword_first)].votes == {`
		`+ stub_admin_user1: {`
		`+ 'vote': '%s' % vote_first,`
		`+ 'comment_id': 1`
		`+ }`
		`+ }`
		`+ assert trackers[expand_fe(keyword_second)].votes == {`
		`+ stub_admin_user1: {`
		`+ 'vote': '%s' % vote_second,`
		`+ 'comment_id': 1`
		`+ }`
		`+ }`
		`+`
		`+`
		`+ class TestPagureBotSinglelineMultivote(object):`
		`+ @pytest.mark.parametrize("inversed", itertools.product([False, True], [False, True]))`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`@pytest.mark.parametrize("vote", [('+1', '-1'), ('-1', '+1'), ('+1', '0'),`
		`('-1', '0'), ('0', '-1'), ('0', '+1')])`
		`- def test_multiline_multivote_same(self, vote, keyword):`
		`+ def test_singleline_multivote_same(self, vote, keyword, inversed):`
		`vote_first = vote[0]`
		`vote_second = vote[1]`
		`+ first = (vote_first, keyword) if inversed[0] else (keyword, vote_first)`
		`+ second = (vote_second, keyword) if inversed[1] else (keyword, vote_second)`
		`comments = [pagure_bot.Comment(c) for c in [`
		`{`
		`'id': 1,`
		`- 'comment': "foobar\n%s %s\nbarbaz\n%s %s" % (keyword, vote_first,`
		`- keyword, vote_second),`
		`+ # Votes should be accepted with second taking precedence`
		`+ 'comment': "%s %s %s %s" % (first + second),`
		`'user': {`
		`'name': stub_admin_user1`
		`}`
		`@@ -262,27 +382,34 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword].votes == {`
		`+ assert trackers[expand_fe(keyword)].votes == {`
		`stub_admin_user1: {`
		`'vote': '%s' % vote_second,`
		`'comment_id': 1`
		`}`
		`}`

		`- @pytest.mark.parametrize("keyword", itertools.permutations(pagure_bot.TRACKER_KEYWORDS, 2))`
		`+ @pytest.mark.parametrize("inversed", itertools.product([False, True], [False, True]))`
		`+ @pytest.mark.parametrize("keyword", itertools.permutations(EXTENDED_TRACKERS, 2))`
		`@pytest.mark.parametrize("vote", [('+1', '-1'), ('-1', '+1'), ('+1', '0'),`
		`('-1', '0'), ('0', '-1'), ('0', '+1'),`
		`('0', '0'), ('-1', '-1'), ('+1', '+1')])`
		`- def test_multiline_multivote_different(self, vote, keyword):`
		`+ def test_singleline_multivote_different(self, vote, keyword, inversed):`
		`keyword_first = keyword[0]`
		`keyword_second = keyword[1]`
		`+ # if we hit two keywords which are synonyms for each other,`
		`+ # move on, test is invalid`
		`+ if keyword_first.replace("fe", "freezeexception") == keyword_second.replace("fe", "freezeexception"):`
		`+ return None`
		`vote_first = vote[0]`
		`vote_second = vote[1]`
		`+ first = (vote_first, keyword_first) if inversed[0] else (keyword_first, vote_first)`
		`+ second = (vote_second, keyword_second) if inversed[1] else (keyword_second, vote_second)`
		`comments = [pagure_bot.Comment(c) for c in [`
		`{`
		`'id': 1,`
		`- 'comment': "foobar\n%s %s\nbarbaz\n%s %s" % (keyword_first, vote_first,`
		`- keyword_second, vote_second),`
		`+ # Both votes should be accepted`
		`+ 'comment': "%s %s %s %s" % (first + second),`
		`'user': {`
		`'name': stub_admin_user1`
		`}`
		`@@ -290,22 +417,78 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword_first].votes == {`
		`+ assert trackers[expand_fe(keyword_first)].votes == {`
		`stub_admin_user1: {`
		`'vote': '%s' % vote_first,`
		`'comment_id': 1`
		`}`
		`}`
		`- assert trackers[keyword_second].votes == {`
		`+ assert trackers[expand_fe(keyword_second)].votes == {`
		`stub_admin_user1: {`
		`'vote': '%s' % vote_second,`
		`'comment_id': 1`
		`}`
		`}`

		`+ def test_singleline_multivote_three_votes_two_inversed(self):`
		`+ comments = [pagure_bot.Comment(c) for c in [`
		`+ {`
		`+ 'id': 1,`
		`+ 'comment': "BetaBlocker -1 +1 FinalBlocker +1 BetaFE",`
		`+ 'user': {`
		`+ 'name': stub_admin_user1`
		`+ }`
		`+ }`
		`+ ]]`
		`+`
		`+ trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`+ assert trackers['betablocker'].votes == {`
		`+ stub_admin_user1: {`
		`+ 'vote': '-1',`
		`+ 'comment_id': 1`
		`+ }`
		`+ }`
		`+ assert trackers['finalblocker'].votes == {`
		`+ stub_admin_user1: {`
		`+ 'vote': '+1',`
		`+ 'comment_id': 1`
		`+ }`
		`+ }`
		`+ assert trackers['betafreezeexception'].votes == {`
		`+ stub_admin_user1: {`
		`+ 'vote': '+1',`
		`+ 'comment_id': 1`
		`+ }`
		`+ }`
		`+`
		`+ def test_singleline_multivote_summary_rejected(self):`
		`+ """Votes should be rejected due to additional text. We need`
		`+ both comments to check we reject both even and odd word counts`
		`+ """`
		`+ comments = [pagure_bot.Comment(c) for c in [`
		`+ {`
		`+ 'id': 1,`
		`+ 'comment': "+1 FinalBlocker -1 BetaBlocker lalala",`
		`+ 'user': {`
		`+ 'name': stub_admin_user1`
		`+ }`
		`+ },`
		`+ {`
		`+ 'id': 2,`
		`+ 'comment': "+1 FinalBlocker -1 BetaBlocker lalala foofoofoo",`
		`+ 'user': {`
		`+ 'name': stub_admin_user1`
		`+ }`
		`+ }`
		`+ ]]`
		`+`
		`+ trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`+ assert trackers["finalblocker"].votes == {}`
		`+ assert trackers["betablocker"].votes == {}`
		`+`

		`class TestPagureBotAgreedRevote(object):`
		`- @pytest.mark.parametrize("keyword", pagure_bot.TRACKER_KEYWORDS)`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`@pytest.mark.parametrize("outcome", ['Accepted', 'Rejected'])`
		`def test_agreed_single_acceptedrejected_needs_summary(self, outcome, keyword, setup_stubs):`
		`comments = [pagure_bot.Comment(c) for c in [`
		`@@ -326,15 +509,15 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword].votes == {`
		`+ assert trackers[expand_fe(keyword)].votes == {`
		`stub_admin_user1: {`
		`'vote': '+1',`
		`'comment_id': 1`
		`}`
		`}`
		`- assert trackers[keyword].need_summary_post`
		`+ assert trackers[expand_fe(keyword)].need_summary_post`

		`- @pytest.mark.parametrize("keyword", pagure_bot.TRACKER_KEYWORDS)`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`@pytest.mark.parametrize("outcome", ['Accepted', 'Rejected'])`
		`def test_agreed_single_acceptedrejected_no_justification_needs_summary(self, outcome, keyword, setup_stubs):`
		`comments = [pagure_bot.Comment(c) for c in [`
		`@@ -355,15 +538,15 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword].votes == {`
		`+ assert trackers[expand_fe(keyword)].votes == {`
		`stub_admin_user1: {`
		`'vote': '+1',`
		`'comment_id': 1`
		`}`
		`}`
		`- assert trackers[keyword].need_summary_post`
		`+ assert trackers[expand_fe(keyword)].need_summary_post`

		`- @pytest.mark.parametrize("keyword", pagure_bot.TRACKER_KEYWORDS)`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`@pytest.mark.parametrize("outcome", ['Accepted', 'Rejected'])`
		`def test_agreed_single_acceptedrejected_by_impostor_need_summary(self, outcome, keyword, setup_stubs):`
		`comments = [pagure_bot.Comment(c) for c in [`
		`@@ -383,7 +566,7 @@`
		`},`
		`{`
		`'id': 3,`
		`- 'comment': "%s\n%s" % (pagure_bot.CLOSED_VOTES_HEADER, keyword),`
		`+ 'comment': "%s\n%s" % (pagure_bot.CLOSED_VOTES_HEADER, expand_fe(keyword)),`
		`'user': {`
		`'name': 'impostor'`
		`}`
		`@@ -391,15 +574,15 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword].votes == {`
		`+ assert trackers[expand_fe(keyword)].votes == {`
		`stub_admin_user1: {`
		`'vote': '+1',`
		`'comment_id': 1`
		`}`
		`}`
		`- assert trackers[keyword].need_summary_post`
		`+ assert trackers[expand_fe(keyword)].need_summary_post`

		`- @pytest.mark.parametrize("keywords", powerset(pagure_bot.TRACKER_KEYWORDS))`
		`+ @pytest.mark.parametrize("keywords", powerset(EXTENDED_TRACKERS))`
		`def test_agreed_multiple_accepts_need_summary(self, keywords, setup_stubs):`
		`outcomes = " ".join(["accepted%s" % keyword for keyword in keywords])`
		`comments = [pagure_bot.Comment(c) for c in [`
		`@@ -414,9 +597,9 @@`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`for keyword in keywords:`
		`- assert trackers[keyword].need_summary_post`
		`+ assert trackers[expand_fe(keyword)].need_summary_post`

		`- @pytest.mark.parametrize("keywords", powerset(pagure_bot.TRACKER_KEYWORDS))`
		`+ @pytest.mark.parametrize("keywords", powerset(EXTENDED_TRACKERS))`
		`def test_agreed_multiple_accepts_no_justification_need_summary(self, keywords, setup_stubs):`
		`outcomes = " ".join(["accepted%s" % keyword for keyword in keywords])`
		`comments = [pagure_bot.Comment(c) for c in [`
		`@@ -431,9 +614,9 @@`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`for keyword in keywords:`
		`- assert trackers[keyword].need_summary_post`
		`+ assert trackers[expand_fe(keyword)].need_summary_post`

		`- @pytest.mark.parametrize("keywords", powerset(pagure_bot.TRACKER_KEYWORDS))`
		`+ @pytest.mark.parametrize("keywords", powerset(EXTENDED_TRACKERS))`
		`def test_agreed_multiple_accepts_has_summary(self, keywords, setup_stubs):`
		`outcomes = " ".join(["accepted%s" % keyword for keyword in keywords])`
		`comments = [pagure_bot.Comment(c) for c in [`
		`@@ -446,7 +629,8 @@`
		`},`
		`{`
		`'id': 2,`
		`- 'comment': "%s\n%s" % (pagure_bot.CLOSED_VOTES_HEADER, "\n".join(keywords)),`
		`+ 'comment': "%s\n%s" % (pagure_bot.CLOSED_VOTES_HEADER,`
		`+ "\n".join([expand_fe(keyword) for keyword in keywords])),`
		`'user': {`
		`'name': app.config['PAGURE_BOT_USERNAME']`
		`}`
		`@@ -455,9 +639,9 @@`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`for keyword in keywords:`
		`- assert not trackers[keyword].need_summary_post`
		`+ assert not trackers[expand_fe(keyword)].need_summary_post`

		`- @pytest.mark.parametrize("keyword", pagure_bot.TRACKER_KEYWORDS)`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`def test_revote_single(self, keyword, setup_stubs):`
		`comments = [pagure_bot.Comment(c) for c in [`
		`{`
		`@@ -475,13 +659,46 @@`
		`}`
		`},`
		`{`
		`- 'id': 2,`
		`- 'comment': "%s\n%s" % (pagure_bot.CLOSED_VOTES_HEADER, keyword),`
		`+ 'id': 3,`
		`+ 'comment': "%s\n%s" % (pagure_bot.CLOSED_VOTES_HEADER, expand_fe(keyword)),`
		`'user': {`
		`'name': app.config['PAGURE_BOT_USERNAME']`
		`}`
		`},`
		`{`
		`+ 'id': 4,`
		`+ 'comment': "REVOTE %s" % keyword,`
		`+ 'user': {`
		`+ 'name': stub_admin_user1`
		`+ }`
		`+ }`
		`+ ]]`
		`+`
		`+ trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`+ assert trackers[expand_fe(keyword)].votes == {}`
		`+ assert not trackers[expand_fe(keyword)].need_summary_post`
		`+`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`+ def test_revote_without_previous_summary(self, keyword, setup_stubs):`
		`+ '''If the summary didn't arrive in time and the tracker got revoted in`
		`+ the meantime, no need to supply summary anymore.`
		`+ '''`
		`+ comments = [pagure_bot.Comment(c) for c in [`
		`+ {`
		`+ 'id': 1,`
		`+ 'comment': "%s +1" % keyword,`
		`+ 'user': {`
		`+ 'name': stub_admin_user1`
		`+ }`
		`+ },`
		`+ {`
		`+ 'id': 2,`
		`+ 'comment': "AGREED Accepted%s foo bar" % keyword,`
		`+ 'user': {`
		`+ 'name': stub_admin_user1`
		`+ }`
		`+ },`
		`+ {`
		`'id': 3,`
		`'comment': "REVOTE %s" % keyword,`
		`'user': {`
		`@@ -491,12 +708,16 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword].votes == {}`
		`- assert not trackers[keyword].need_summary_post`
		`+ assert trackers[expand_fe(keyword)].votes == {}`
		`+ assert not trackers[expand_fe(keyword)].need_summary_post`

		`@pytest.mark.parametrize("keyword_a, keyword_b",`
		`- itertools.permutations(pagure_bot.TRACKER_KEYWORDS, 2))`
		`+ itertools.permutations(EXTENDED_TRACKERS, 2))`
		`def test_agreed_stops_counting_counts_other(self, keyword_a, keyword_b, setup_stubs):`
		`+ # if we hit two keywords which are synonyms for each other,`
		`+ # move on`
		`+ if keyword_a.replace("fe", "freezeexception") == keyword_b.replace("fe", "freezeexception"):`
		`+ return None`
		`comments = [pagure_bot.Comment(c) for c in [`
		`{`
		`'id': 1,`
		`@@ -522,13 +743,13 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword_a].votes == {`
		`+ assert trackers[expand_fe(keyword_a)].votes == {`
		`stub_admin_user1: {`
		`'vote': '+1',`
		`'comment_id': 1`
		`}`
		`}`
		`- assert trackers[keyword_b].votes == {`
		`+ assert trackers[expand_fe(keyword_b)].votes == {`
		`stub_admin_user1: {`
		`'vote': '+1',`
		`'comment_id': 1`
		`@@ -541,13 +762,58 @@`


		`class TestPagureBotWhitespaces(object):`
		`- @pytest.mark.parametrize("keyword", pagure_bot.TRACKER_KEYWORDS)`
		`+ @pytest.mark.parametrize("inversed", [False, True])`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`+ @pytest.mark.parametrize("vote", ['+1', '-1', '0'])`
		`+ def test_trailing_whitespace(self, vote, keyword, inversed):`
		`+ comments = [pagure_bot.Comment(c) for c in [`
		`+ {`
		`+ 'id': 1,`
		`+ 'comment': "%s %s " % ((vote, keyword) if inversed else (keyword, vote)),`
		`+ 'user': {`
		`+ 'name': stub_admin_user1`
		`+ }`
		`+ }`
		`+ ]]`
		`+`
		`+ trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`+ assert trackers[expand_fe(keyword)].votes == {`
		`+ stub_admin_user1: {`
		`+ 'vote': '%s' % vote,`
		`+ 'comment_id': 1`
		`+ }`
		`+ }`
		`+`
		`+ @pytest.mark.parametrize("inversed", [False, True])`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`+ @pytest.mark.parametrize("vote", ['+1', '-1', '0'])`
		`+ def test_leading_whitespace(self, vote, keyword, inversed):`
		`+ comments = [pagure_bot.Comment(c) for c in [`
		`+ {`
		`+ 'id': 1,`
		`+ 'comment': " %s %s" % ((vote, keyword) if inversed else (keyword, vote)),`
		`+ 'user': {`
		`+ 'name': stub_admin_user1`
		`+ }`
		`+ }`
		`+ ]]`
		`+`
		`+ trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`+ assert trackers[expand_fe(keyword)].votes == {`
		`+ stub_admin_user1: {`
		`+ 'vote': '%s' % vote,`
		`+ 'comment_id': 1`
		`+ }`
		`+ }`
		`+`
		`+ @pytest.mark.parametrize("inversed", [False, True])`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`@pytest.mark.parametrize("vote", ['+1', '-1', '0'])`
		`- def test_trailing_whitespace(self, vote, keyword):`
		`+ def test_whitespaces_inside_vote(self, vote, keyword, inversed):`
		`comments = [pagure_bot.Comment(c) for c in [`
		`{`
		`'id': 1,`
		`- 'comment': "%s %s " % (keyword, vote),`
		`+ 'comment': "%s %s" % ((vote, keyword) if inversed else (keyword, vote)),`
		`'user': {`
		`'name': stub_admin_user1`
		`}`
		`@@ -555,20 +821,21 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword].votes == {`
		`+ assert trackers[expand_fe(keyword)].votes == {`
		`stub_admin_user1: {`
		`'vote': '%s' % vote,`
		`'comment_id': 1`
		`}`
		`}`

		`- @pytest.mark.parametrize("keyword", pagure_bot.TRACKER_KEYWORDS)`
		`+ @pytest.mark.parametrize("inversed", [False, True])`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`@pytest.mark.parametrize("vote", ['+1', '-1', '0'])`
		`- def test_whitespaces_inside_vote(self, vote, keyword):`
		`+ def test_trailing_leading_inside_whitespace(self, vote, keyword, inversed):`
		`comments = [pagure_bot.Comment(c) for c in [`
		`{`
		`'id': 1,`
		`- 'comment': "%s %s" % (keyword, vote),`
		`+ 'comment': " %s %s " % ((vote, keyword) if inversed else (keyword, vote)),`
		`'user': {`
		`'name': stub_admin_user1`
		`}`
		`@@ -576,20 +843,44 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword].votes == {`
		`+ assert trackers[expand_fe(keyword)].votes == {`
		`stub_admin_user1: {`
		`'vote': '%s' % vote,`
		`'comment_id': 1`
		`}`
		`}`

		`- @pytest.mark.parametrize("keyword", pagure_bot.TRACKER_KEYWORDS)`
		`+ @pytest.mark.parametrize("inversed", [False, True])`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`@pytest.mark.parametrize("vote", ['+1', '-1', '0'])`
		`- def test_newline_inside_vote_ignored(self, vote, keyword):`
		`+ def test_newline_inside_vote_ignored(self, vote, keyword, inversed):`
		`+ comments = [pagure_bot.Comment(c) for c in [`
		`+ {`
		`+ 'id': 1,`
		`+ 'comment': "%s\n%s" % ((vote, keyword) if inversed else (keyword, vote)),`
		`+ 'user': {`
		`+ 'name': stub_admin_user1`
		`+ }`
		`+ }`
		`+ ]]`
		`+`
		`+ trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`+ assert trackers[expand_fe(keyword)].votes == {}`
		`+`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`+ @pytest.mark.parametrize("outcome", ['Accepted', 'Rejected'])`
		`+ def test_agreed_single_acceptedrejected_leading_wspace_needs_summary(self, outcome, keyword, setup_stubs):`
		`comments = [pagure_bot.Comment(c) for c in [`
		`{`
		`'id': 1,`
		`- 'comment': "%s\n%s" % (keyword, vote),`
		`+ 'comment': "%s +1" % keyword,`
		`+ 'user': {`
		`+ 'name': stub_admin_user1`
		`+ }`
		`+ },`
		`+ {`
		`+ 'id': 2,`
		`+ 'comment': " AGREED %s%s foo bar" % (outcome, keyword),`
		`'user': {`
		`'name': stub_admin_user1`
		`}`
		`@@ -597,9 +888,9 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword].votes == {}`
		`+ assert trackers[expand_fe(keyword)].need_summary_post`

		`- @pytest.mark.parametrize("keyword", pagure_bot.TRACKER_KEYWORDS)`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`@pytest.mark.parametrize("outcome", ['Accepted', 'Rejected'])`
		`def test_agreed_single_acceptedrejected_trailing_wspace_needs_summary(self, outcome, keyword, setup_stubs):`
		`comments = [pagure_bot.Comment(c) for c in [`
		`@@ -620,9 +911,9 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword].need_summary_post`
		`+ assert trackers[expand_fe(keyword)].need_summary_post`

		`- @pytest.mark.parametrize("keyword", pagure_bot.TRACKER_KEYWORDS)`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`@pytest.mark.parametrize("outcome", ['Accepted', 'Rejected'])`
		`def test_agreed_single_acceptedrejected_multi_wspace_inside_needs_summary(self, outcome, keyword, setup_stubs):`
		`comments = [pagure_bot.Comment(c) for c in [`
		`@@ -635,7 +926,7 @@`
		`},`
		`{`
		`'id': 2,`
		`- 'comment': "AGREED %s%s foo bar " % (outcome, keyword),`
		`+ 'comment': " AGREED %s%s foo bar " % (outcome, keyword),`
		`'user': {`
		`'name': stub_admin_user1`
		`}`
		`@@ -643,9 +934,9 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert trackers[keyword].need_summary_post`
		`+ assert trackers[expand_fe(keyword)].need_summary_post`

		`- @pytest.mark.parametrize("keyword", pagure_bot.TRACKER_KEYWORDS)`
		`+ @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)`
		`@pytest.mark.parametrize("outcome", ['Accepted', 'Rejected'])`
		`def test_agreed_single_acceptedrejected_newline_inside(self, outcome, keyword, setup_stubs):`
		`comments = [pagure_bot.Comment(c) for c in [`
		`@@ -666,9 +957,9 @@`
		`]]`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`- assert not trackers[keyword].need_summary_post`
		`+ assert not trackers[expand_fe(keyword)].need_summary_post`

		`- @pytest.mark.parametrize("keywords", powerset(pagure_bot.TRACKER_KEYWORDS))`
		`+ @pytest.mark.parametrize("keywords", powerset(EXTENDED_TRACKERS))`
		`def test_agreed_multiple_accepts_multi_wspace_inside_need_summary(self, keywords, setup_stubs):`
		`outcomes = " ".join(["accepted%s" % keyword for keyword in keywords])`
		`comments = [pagure_bot.Comment(c) for c in [`
		`@@ -683,7 +974,7 @@`

		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`for keyword in keywords:`
		`- assert trackers[keyword].need_summary_post`
		`+ assert trackers[expand_fe(keyword)].need_summary_post`


		`class TestPagureBotHaywire(object):`
		`@@ -776,119 +1067,81 @@`
		`assert not pagure_bot.bot_loop_detected(comments)`


		`- realworld_comments = [`
		`- {`
		`- "comment": "bork bork\r\nFinalBlocker +1\r\nBetaBlocker -1",`
		`- "id": 1,`
		`- "user": {`
		`- "name": "lbrabec"`
		`- }`
		`- },`
		`- {`
		`- "comment": "Obvious\r\nFinalBlocker +1\r\nBetaFE +1\r\n\r\nBut I don't like that much:\r\n"`
		`+ class TestPagureBotRealWorld(object):`
		`+`
		`+ realworld_comments = [`
		`+ {"comment": "bork bork\r\nFinalBlocker +1\r\nBetaBlocker -1",`
		`+ "id": 1,`
		`+ "user": {"name": "lbrabec"}`
		`+ },`
		`+ {"comment": "Obvious\r\nFinalBlocker +1\r\nBetaFE +1\r\n\r\nBut I don't like that much:\r\n"`
		`"BetaBlocker -1\r\n\r\nFor FinalFE, gee, I don't know:\r\nFinalFE 0",`
		`- "id": 2,`
		`- "user": {`
		`- "name": "kparal"`
		`- }`
		`- },`
		`- {`
		`- "comment": "Y'all are insane. Foo is working great for me and I'm ashamed that this"`
		`+ "id": 2,`
		`+ "user": {"name": "kparal"}`
		`+ },`
		`+ {"comment": "Y'all are insane. Foo is working great for me and I'm ashamed that this"`
		`"was ever considered to be a blocker.\r\n\r\nFinalBlocker -1\r\n"`
		`"BetaBlocker -1\r\nBetaFE -1\r\nOP -1",`
		`- "id": 3,`
		`- "user": {`
		`- "name": "tflink"`
		`- }`
		`- },`
		`- {`
		`- "comment": "On the off chance that my vote could be counted twice ...\r\n\r\n"`
		`+ "id": 3,`
		`+ "user": {"name": "tflink"}`
		`+ },`
		`+ {"comment": "On the off chance that my vote could be counted twice ...\r\n\r\n"`
		`"FinalBlocker -1\r\nAlphaBlocker -2",`
		`- "id": 4,`
		`- "user": {`
		`- "name": "tflink"`
		`- }`
		`- },`
		`- {`
		`- "comment": "or maybe I'm wrong .... but that's never happened before in all "`
		`+ "id": 4,`
		`+ "user": {"name": "tflink"}`
		`+ },`
		`+ {"comment": "or maybe I'm wrong .... but that's never happened before in all "`
		`"of recorded history\r\n\r\nBetaFE +1\r\nBetaBlocker +!",`
		`- "id": 5,`
		`- "user": {`
		`- "name": "tflink"`
		`- }`
		`- },`
		`- {`
		`- "comment": "Yeah, those who vote against it do not deserve to use Fedora, "`
		`+ "id": 5,`
		`+ "user": {"name": "tflink"}`
		`+ },`
		`+ {"comment": "Yeah, those who vote against it do not deserve to use Fedora, "`
		`"go to Windows.\r\n\r\nBetaBlocker +1\r\nFinalBlocker +1\r\n"`
		`"BetaFE +1\r\nFinalFE +1",`
		`- "id": 6,`
		`- "user": {`
		`- "name": "lruzicka"`
		`- }`
		`- },`
		`- {`
		- "comment": "Let's be sneaky\r\n`PreviousRelease +1`\r\n\r\n"
		`+ "id": 6,`
		`+ "user": {"name": "lruzicka"}`
		`+ },`
		+ {"comment": "Let's be sneaky\r\n`PreviousRelease +1`\r\n\r\n"
		"And\r\n```\r\n0Day 0\r\n```",
		`- "id": 7,`
		`- "user": {`
		`- "name": "kparal"`
		`- }`
		`- },`
		`- {`
		`- "comment": "日本語で書いたら壊しますか\r\n\r\nBetaFE 👎\r\nFinalBlocker: ＋1\r\n\r\nBetaBlocker: +1",`
		`- "id": 8,`
		`- "user": {`
		`- "name": "tflink"`
		`- }`
		`- },`
		`- {`
		`- "comment": "Nejkulatoulinkatejsi kulicka",`
		`- "id": 9,`
		`- "user": {`
		`- "name": "jskladan"`
		`- }`
		`- },`
		`- {`
		`- "comment": "AGREED AcceptedFinalBlocker Lorem ipsum dolor sit amet",`
		`- "id": 11,`
		`- "user": {`
		`- "name": "lbrabec"`
		`- }`
		`- },`
		`- {`
		`- "comment": "The following votes have been closed:\n\n"`
		`+ "id": 7,`
		`+ "user": {"name": "kparal"}`
		`+ },`
		`+ {"comment": "日本語で書いたら壊しますか\r\n\r\nBetaFE 👎\r\nFinalBlocker: ＋1\r\n\r\nBetaBlocker: +1",`
		`+ "id": 8,`
		`+ "user": {"name": "tflink"}`
		`+ },`
		`+ {"comment": "Nejkulatoulinkatejsi kulicka",`
		`+ "id": 9,`
		`+ "user": {"name": "jskladan"}`
		`+ },`
		`+ {"comment": "AGREED AcceptedFinalBlocker Lorem ipsum dolor sit amet",`
		`+ "id": 11,`
		`+ "user": {"name": "lbrabec"}`
		`+ },`
		`+ {"comment": "The following votes have been closed:\n\n"`
		`"* Accepted FinalBlocker (+3, 0, -1)\n"`
		`" * +1 by [lbrabec](#comment-1), [kparal](#comment-2), "`
		`"[lruzicka](#comment-6)\n"`
		`" * -1 by [tflink](#comment-4)\n",`
		`- "id": 12,`
		`- "user": {`
		`- "name": "blockerbot"`
		`- }`
		`- },`
		`- {`
		`- "comment": "AGREED RejectedBetaBlocker Lorem ipsum dolor sit amet",`
		`- "id": 13,`
		`- "user": {`
		`- "name": "lbrabec"`
		`- }`
		`- },`
		`- {`
		`- "comment": "The following votes have been closed:\n\n"`
		`+ "id": 12,`
		`+ "user": {"name": "blockerbot"}`
		`+ },`
		`+ {"comment": "AGREED RejectedBetaBlocker Lorem ipsum dolor sit amet",`
		`+ "id": 13,`
		`+ "user": {"name": "lbrabec"}`
		`+ },`
		`+ {"comment": "The following votes have been closed:\n\n"`
		`"* Rejected BetaBlocker (+1, 0, -3)\n"`
		`" * +1 by [lruzicka](#comment-6)\n"`
		`" * -1 by [lbrabec](#comment-1), [kparal](#comment-2), "`
		`"[tflink](#comment-3)\n",`
		`- "id": 14,`
		`- "user": {`
		`- "name": "blockerbot"`
		`- }`
		`- }`
		`- ]`
		`+ "id": 14,`
		`+ "user": {"name": "blockerbot"}`
		`+ }`
		`+ ]`

		`- class TestPagureBotRealWorld(object):`
		`def test_realworld(self, setup_stubs):`
		`- comments = [pagure_bot.Comment(c) for c in realworld_comments]`
		`+ comments = [pagure_bot.Comment(c) for c in self.realworld_comments]`
		`trackers = pagure_bot.parse_comments_to_trackers(comments)`
		`non_voting_users, last_comment_id = pagure_bot.voting_info(comments, trackers)`

		`@@ -930,7 +1183,7 @@`
		`'comment_id': 3`
		`}`
		`}`
		`- assert trackers['betafe'].votes == {`
		`+ assert trackers['betafreezeexception'].votes == {`
		`'kparal': {`
		`'vote': '+1',`
		`'comment_id': 2`
		`@@ -944,7 +1197,7 @@`

fedora-qa / blockerbugs

Source Code

#163 Make Pagure vote parser more liberal and also safer Merged 4 years ago by kparal. Opened 4 years ago by kparal. feature/liberal-parser-161 into develop

#163 Make Pagure vote parser more liberal and also safer

Merged 4 years ago by kparal. Opened 4 years ago by kparal.

feature/liberal-parser-161 into develop