fedora-qa / blockerbugs

Clone
Source Code
GIT

#213 pagure_bot: generate correct message when updating a ticket without votes
Merged 3 years ago by kparal. Opened 3 years ago by kparal.
bug/discussion-no-votes  into  develop

file modified
+10 -7 
@@ -36,7 +36,6 @@ 
 

  OUTCOME_RE = r'(accepted|rejected)' + TRACKER_RE
 

  OUTCOME_MATCHER = re.compile(OUTCOME_RE)
 

  

- SUMMARY_HEADER = ""
 

  CLOSED_VOTES_HEADER = "The following votes have been closed:"
 

  

  # Containers for parsed commands
 
@@ -342,14 +341,15 @@ 
 

      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
 

+ def summary(trackers: dict[str, BugVoteTracker], non_voting_users: dict[str, int] = None,
 

+             last_comment_id: int = None, header: str = "") -> str:
 

+     '''Create an overall vote summary in the 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 non_voting_users: users who haven't voted yet as returned by :func:`voting_info`
 

      :param int last_comment_id: the last comment id that was counted
 

-     :param str head: an initial message text
 

+     :param str header: an initial message text
 

      '''
 

      md_text = "%s\n\n" % header
 

  
@@ -382,6 +382,9 @@ 
 

                      "processed comment was [#comment-%s](#comment-%s)*" % (
 

                          time, last_comment_id, last_comment_id))
 

  

+     if not md_text or md_text.isspace():
 

+         md_text = 'Nobody voted yet.'
 

+ 

      return md_text
 

  

  
@@ -449,11 +452,11 @@ 
 

          if bot_loop_detected(comments):
 

              app.logger.error('Bot is probably stuck in loop, not posting summary comment!')
 

          else:
 

-             comment = summary(trackers_without_summary)
 

+             comment = summary(trackers_without_summary, header=CLOSED_VOTES_HEADER)
 

              pagure_interface.post_comment(issue_id, comment)
 

  

      app.logger.debug('Updating issue summary')
 

-     vote_summary = summary(trackers, non_voting_users, last_comment_id, SUMMARY_HEADER)
 

+     vote_summary = summary(trackers, non_voting_users, last_comment_id)
 

      pagure_interface.update_issue(issue_id, vote_summary)
 

  

      app.logger.debug('Saving voting info to database')

file modified
+186 -21 
@@ -8,11 +8,7 @@ 
 

  

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

  

- def powerset(iterable):
 

-     s = list(iterable)
 

-     return itertools.chain.from_iterable(itertools.combinations(s, r) for r in range(len(s) + 1))
 

- 

- stub_admin_user1 = "fakey"
 

+ stub_admin_user1 = "fakey1"
 

  stub_admin_user2 = "fakey2"
 

  stub_group_data = {
 

      "creator": {
 
@@ -28,10 +24,17 @@ 
 

          stub_admin_user1,
 

          stub_admin_user2,
 

          "fakey3",
 

+         "lbrabec",
 

      ],
 

      "name": "fake-group"
 

  }
 

  

+ 

+ def powerset(iterable):
 

+     s = list(iterable)
 

+     return itertools.chain.from_iterable(itertools.combinations(s, r) for r in range(len(s) + 1))
 

+ 

+ 

  def expand_fe(tracker):
 

      if tracker == 'betafe':
 

          return 'betafreezeexception'
 
@@ -40,7 +43,22 @@ 
 

      else:
 

          return tracker
 

  

- @pytest.fixture
 

+ 

+ def line_contains(lines: str, words: list[str]) -> bool:
 

+     '''Find out if there is a line in ``lines`` which contains all the ``words``
 

+     '''
 

+     for line in lines.splitlines():
 

+         for word in words:
 

+             if word not in line:
 

+                 break
 

+         else:
 

+             # we didn't break, i.e. we found all words
 

+             return True
 

+ 

+     return False
 

+ 

+ 

+ @pytest.fixture(autouse=True)
 

  def setup_stubs(monkeypatch):
 

      stub_pagure_group_call = mock.MagicMock(return_value=stub_group_data)
 

      monkeypatch.setattr(pagure_interface, 'get_group', stub_pagure_group_call)
 
@@ -97,6 +115,138 @@ 
 

          }
 

  

  

+ class TestSummary:
 

+     def test_summary_empty(self):
 

+         summary = pagure_bot.summary(trackers={})
 

+         assert summary.strip() == 'Nobody voted yet.'
 

+ 

+     @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)
 

+     def test_summary_proposed(self, keyword):
 

+         nicer_keyword = pagure_bot.NICER[expand_fe(keyword)]
 

+         comments = [pagure_bot.Comment(c) for c in [
 

+             {
 

+                 'id': 1,
 

+                 'comment': "%s +1" % keyword,
 

+                 'user': {
 

+                     'name': stub_admin_user1
 

+                 }
 

+             },
 

+         ]]
 

+         trackers = pagure_bot.parse_comments_to_trackers(comments)
 

+         non_voting_users, last_comment_id = pagure_bot.voting_info(comments, trackers)
 

+         summary = pagure_bot.summary(trackers, non_voting_users, last_comment_id)
 

+ 

+         assert line_contains(summary, [nicer_keyword, '+1', ' 0,', '-0'])
 

+         assert line_contains(summary, ['+1 by', stub_admin_user1])
 

+         assert "Commented but haven't voted yet" not in summary
 

+         assert line_contains(summary, ['the last processed comment was', '#comment-1'])
 

+ 

+     @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)
 

+     def test_summary_accepted(self, keyword):
 

+         nicer_keyword = pagure_bot.NICER[expand_fe(keyword)]
 

+         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" % keyword,
 

+                 'user': {
 

+                     'name': stub_admin_user1
 

+                 }
 

+             },
 

+         ]]
 

+         trackers = pagure_bot.parse_comments_to_trackers(comments)
 

+         non_voting_users, last_comment_id = pagure_bot.voting_info(comments, trackers)
 

+         summary = pagure_bot.summary(trackers, non_voting_users, last_comment_id)
 

+ 

+         assert line_contains(summary, ['Accepted', nicer_keyword, '+1', ' 0,', '-0'])
 

+         assert line_contains(summary, ['+1 by', stub_admin_user1])
 

+         assert "Commented but haven't voted yet" not in summary
 

+         assert line_contains(summary, ['the last processed comment was', '#comment-2'])
 

+ 

+     @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)
 

+     def test_summary_rejected(self, keyword):
 

+         nicer_keyword = pagure_bot.NICER[expand_fe(keyword)]
 

+         comments = [pagure_bot.Comment(c) for c in [
 

+             {
 

+                 'id': 1,
 

+                 'comment': "%s +1" % keyword,
 

+                 'user': {
 

+                     'name': stub_admin_user1
 

+                 }
 

+             },
 

+             {
 

+                 'id': 2,
 

+                 'comment': "AGREED Rejected%s" % keyword,
 

+                 'user': {
 

+                     'name': stub_admin_user1
 

+                 }
 

+             },
 

+         ]]
 

+         trackers = pagure_bot.parse_comments_to_trackers(comments)
 

+         non_voting_users, last_comment_id = pagure_bot.voting_info(comments, trackers)
 

+         summary = pagure_bot.summary(trackers, non_voting_users, last_comment_id)
 

+ 

+         assert line_contains(summary, ['Rejected', nicer_keyword, '+1', ' 0,', '-0'])
 

+         assert line_contains(summary, ['+1 by', stub_admin_user1])
 

+         assert "Commented but haven't voted yet" not in summary
 

+         assert line_contains(summary, ['the last processed comment was', '#comment-2'])
 

+ 

+     def test_summary_non_voting(self):
 

+         comments = [pagure_bot.Comment(c) for c in [
 

+             {
 

+                 'id': 1,
 

+                 'comment': "BetaBlocker +1",
 

+                 'user': {
 

+                     'name': stub_admin_user1
 

+                 }
 

+             },
 

+             {
 

+                 'id': 2,
 

+                 'comment': "Oooh shiny!",
 

+                 'user': {
 

+                     'name': stub_admin_user2
 

+                 }
 

+             },
 

+         ]]
 

+         trackers = pagure_bot.parse_comments_to_trackers(comments)
 

+         non_voting_users, last_comment_id = pagure_bot.voting_info(comments, trackers)
 

+         summary = pagure_bot.summary(trackers, non_voting_users, last_comment_id)
 

+ 

+         assert line_contains(summary, ['BetaBlocker', '+1', ' 0,', '-0'])
 

+         assert line_contains(summary, ['+1 by', stub_admin_user1])
 

+         assert line_contains(summary, ["Commented but haven't voted yet", stub_admin_user2])
 

+         assert not line_contains(summary, ["Commented but haven't voted yet", stub_admin_user1])
 

+         assert line_contains(summary, ['the last processed comment was', '#comment-2'])
 

+ 

+     @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)
 

+     @pytest.mark.parametrize("outcome", ['Accepted', 'Rejected'])
 

+     def test_summary_close_without_votes(self, keyword, outcome):
 

+         nicer_keyword = pagure_bot.NICER[expand_fe(keyword)]
 

+         comments = [pagure_bot.Comment(c) for c in [
 

+             {
 

+                 'id': 1,
 

+                 'comment': "AGREED %s%s" % (outcome, keyword),
 

+                 'user': {
 

+                     'name': stub_admin_user1
 

+                 }
 

+             },
 

+         ]]
 

+         trackers = pagure_bot.parse_comments_to_trackers(comments)
 

+         non_voting_users, last_comment_id = pagure_bot.voting_info(comments, trackers)
 

+         summary = pagure_bot.summary(trackers, non_voting_users, last_comment_id)
 

+ 

+         assert line_contains(summary, [outcome, nicer_keyword, '+0', ' 0,', '-0'])
 

+         assert not line_contains(summary, ['+1 by', stub_admin_user1])
 

+         assert line_contains(summary, ["Commented but haven't voted yet", stub_admin_user1])
 

+         assert line_contains(summary, ['the last processed comment was', '#comment-1'])
 

+ 

+ 

  class TestPagureBotTypos(object):
 

      @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)
 

      @pytest.mark.parametrize("sign", ['+', '-'])
 
@@ -490,7 +640,7 @@ 
 

  class TestPagureBotAgreedRevote(object):
 

      @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)
 

      @pytest.mark.parametrize("outcome", ['Accepted', 'Rejected'])
 

-     def test_agreed_single_acceptedrejected_needs_summary(self, outcome, keyword, setup_stubs):
 

+     def test_agreed_single_acceptedrejected_needs_summary(self, outcome, keyword):
 

          comments = [pagure_bot.Comment(c) for c in [
 

              {
 

                  'id': 1,
 
@@ -519,7 +669,7 @@ 
 

  

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

+     def test_agreed_single_acceptedrejected_no_justification_needs_summary(self, outcome, keyword):
 

          comments = [pagure_bot.Comment(c) for c in [
 

              {
 

                  'id': 1,
 
@@ -548,7 +698,7 @@ 
 

  

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

+     def test_agreed_single_acceptedrejected_by_impostor_need_summary(self, outcome, keyword):
 

          comments = [pagure_bot.Comment(c) for c in [
 

              {
 

                  'id': 1,
 
@@ -583,7 +733,7 @@ 
 

          assert trackers[expand_fe(keyword)].need_summary_post
 

  

      @pytest.mark.parametrize("keywords", powerset(EXTENDED_TRACKERS))
 

-     def test_agreed_multiple_accepts_need_summary(self, keywords, setup_stubs):
 

+     def test_agreed_multiple_accepts_need_summary(self, keywords):
 

          outcomes = " ".join(["accepted%s" % keyword for keyword in keywords])
 

          comments = [pagure_bot.Comment(c) for c in [
 

              {
 
@@ -600,7 +750,7 @@ 
 

              assert trackers[expand_fe(keyword)].need_summary_post
 

  

      @pytest.mark.parametrize("keywords", powerset(EXTENDED_TRACKERS))
 

-     def test_agreed_multiple_accepts_no_justification_need_summary(self, keywords, setup_stubs):
 

+     def test_agreed_multiple_accepts_no_justification_need_summary(self, keywords):
 

          outcomes = " ".join(["accepted%s" % keyword for keyword in keywords])
 

          comments = [pagure_bot.Comment(c) for c in [
 

              {
 
@@ -617,7 +767,7 @@ 
 

              assert trackers[expand_fe(keyword)].need_summary_post
 

  

      @pytest.mark.parametrize("keywords", powerset(EXTENDED_TRACKERS))
 

-     def test_agreed_multiple_accepts_has_summary(self, keywords, setup_stubs):
 

+     def test_agreed_multiple_accepts_has_summary(self, keywords):
 

          outcomes = " ".join(["accepted%s" % keyword for keyword in keywords])
 

          comments = [pagure_bot.Comment(c) for c in [
 

              {
 
@@ -642,7 +792,7 @@ 
 

              assert not trackers[expand_fe(keyword)].need_summary_post
 

  

      @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)
 

-     def test_revote_single(self, keyword, setup_stubs):
 

+     def test_revote_single(self, keyword):
 

          comments = [pagure_bot.Comment(c) for c in [
 

              {
 

                  'id': 1,
 
@@ -679,7 +829,7 @@ 
 

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

+     def test_revote_without_previous_summary(self, keyword):
 

          '''If the summary didn't arrive in time and the tracker got revoted in
 

          the meantime, no need to supply summary anymore.
 

          '''
 
@@ -713,7 +863,7 @@ 
 

  

      @pytest.mark.parametrize("keyword_a, keyword_b",
 

                               itertools.permutations(EXTENDED_TRACKERS, 2))
 

-     def test_agreed_stops_counting_counts_other(self, keyword_a, keyword_b, setup_stubs):
 

+     def test_agreed_stops_counting_counts_other(self, keyword_a, keyword_b):
 

          # if we hit two keywords which are synonyms for each other,
 

          # move on
 

          if keyword_a.replace("fe", "freezeexception") == keyword_b.replace("fe", "freezeexception"):
 
@@ -869,7 +1019,7 @@ 
 

  

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

+     def test_agreed_single_acceptedrejected_leading_wspace_needs_summary(self, outcome, keyword):
 

          comments = [pagure_bot.Comment(c) for c in [
 

              {
 

                  'id': 1,
 
@@ -892,7 +1042,7 @@ 
 

  

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

+     def test_agreed_single_acceptedrejected_trailing_wspace_needs_summary(self, outcome, keyword):
 

          comments = [pagure_bot.Comment(c) for c in [
 

              {
 

                  'id': 1,
 
@@ -915,7 +1065,7 @@ 
 

  

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

+     def test_agreed_single_acceptedrejected_multi_wspace_inside_needs_summary(self, outcome, keyword):
 

          comments = [pagure_bot.Comment(c) for c in [
 

              {
 

                  'id': 1,
 
@@ -938,7 +1088,7 @@ 
 

  

      @pytest.mark.parametrize("keyword", EXTENDED_TRACKERS)
 

      @pytest.mark.parametrize("outcome", ['Accepted', 'Rejected'])
 

-     def test_agreed_single_acceptedrejected_newline_inside(self, outcome, keyword, setup_stubs):
 

+     def test_agreed_single_acceptedrejected_newline_inside(self, outcome, keyword):
 

          comments = [pagure_bot.Comment(c) for c in [
 

              {
 

                  'id': 1,
 
@@ -960,7 +1110,7 @@ 
 

          assert not trackers[expand_fe(keyword)].need_summary_post
 

  

      @pytest.mark.parametrize("keywords", powerset(EXTENDED_TRACKERS))
 

-     def test_agreed_multiple_accepts_multi_wspace_inside_need_summary(self, keywords, setup_stubs):
 

+     def test_agreed_multiple_accepts_multi_wspace_inside_need_summary(self, keywords):
 

          outcomes = "   ".join(["accepted%s" % keyword for keyword in keywords])
 

          comments = [pagure_bot.Comment(c) for c in [
 

              {
 
@@ -1140,7 +1290,7 @@ 
 

           }
 

      ]
 

  

-     def test_realworld(self, setup_stubs):
 

+     def test_realworld(self):
 

          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)
 
@@ -1215,3 +1365,18 @@ 
 

          }
 

          assert non_voting_users == {'jskladan': 9}
 

          assert last_comment_id == 14
 

+ 

+     def test_summary(self):
 

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

+         summary = pagure_bot.summary(trackers, non_voting_users, last_comment_id)
 

+ 

+         print(summary)
 

+         assert line_contains(summary, ['Accepted', 'FinalBlocker', '+3', ' 0,', '-1'])
 

+         assert line_contains(summary, ['Rejected', 'BetaBlocker', '+1', ' 0,', '-3'])
 

+         assert line_contains(summary, ['BetaFreezeException', '+3', ' 0,', '-0'])
 

+         assert line_contains(summary, ['FinalFreezeException', '+1', ' 1,', '-0'])
 

+         assert line_contains(summary, ['0Day', '+0', ' 1,', '-0'])
 

+         assert line_contains(summary, ["Commented but haven't voted yet", 'jskladan'])
 

+         assert line_contains(summary, ['the last processed comment was', '#comment-14'])

This fixes a small glitch that if we updated a discussion ticket which haven't
had any votes yet, we'd have the vote summary completely empty instead of saying
'Nobody voted yet'. This fixes the problem and covers the function with unit
tests as a bonus (it was completely untested before).

Build succeeded.

Commit 2f85601 fixes this pull-request

Pull-Request has been merged by kparal
3 years ago
Metadata
None
No Tags
Changes Summary 2
+10 -7
file changed
blockerbugs/util/pagure_bot.py
+186 -21
file changed
testing/test_pagure_bot.py
Powered by Pagure 5.14.1
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.