#188 Fixes for unretire packages plugin
Merged 2 months ago by zlopez. Opened 2 months ago by amedvede.
fedora-infra/ amedvede/toddlers unretire_package_bug_fixes  into  main

@@ -102,7 +102,7 @@ 

              caplog.records[-1].message == "The issue 100 is not open. Skipping message."

          )

  

-     def test_process_keyword_in_title(self, caplog, toddler):

+     def test_process_keyword_not_in_title(self, caplog, toddler):

          """

          Assert that message without keyword in title will be skipped.

          """
@@ -128,18 +128,49 @@ 

  

          assert (

              caplog.records[-1].message

-             == "The issue doesn't contain keyword 'unretire' in the title 'unrere popa'"

+             == "The issue doesn't contain keyword 'unretire' in the title 'unrere popa', or it's "

+             "not a first word in title"

+         )

+ 

+     def test_process_keyword_not_on_first_position_in_title(self, caplog, toddler):

+         """

+         Assert that message without keyword in title will be skipped.

+         """

+         caplog.set_level(logging.INFO)

+ 

+         body = {

+             "project": {"fullname": pdc_unretire_packages.PROJECT_NAMESPACE},

+             "issue": {

+                 "id": 100,

+                 "status": "Open",

+                 "title": "Pac unretire",

+                 "user": {"name": "zlopez"},

+             },

+         }

+         msg = IssueNewV1(body=body)

+ 

+         with patch(

+             "toddlers.plugins.pdc_unretire_packages.PDCUnretirePackages.process_ticket"

+         ) as mock_process_ticket:

+             toddler.process({}, msg)

+ 

+             mock_process_ticket.assert_not_called()

+ 

+         assert (

+             caplog.records[-1].message

+             == "The issue doesn't contain keyword 'unretire' in the title 'Pac unretire', or it's "

+             "not a first word in title"

          )

  

      @patch("toddlers.utils.pdc.pdc_client_for_config")

      @patch("toddlers.utils.pagure.set_pagure")

      @patch("toddlers.utils.bugzilla_system.set_bz")

      @patch("koji.ClientSession")

-     def test_process_base_exception_during_ticket_processing(

-         self, mock_koji, mock_bugzilla, mock_pagure, mock_pdc, toddler

+     def test_process_base_exception_appeared(

+         self, mock_koji, mock_bz, mock_pagure, mock_pdc, caplog, toddler

      ):

          """

-         Assert that ticket will be close if BaseException will appear during ticket process

+         Assert that if base exception appeared, it will be processed correctly.

          """

          issue = {

              "id": 100,
@@ -151,11 +182,12 @@ 

              "project": {"fullname": pdc_unretire_packages.PROJECT_NAMESPACE},

              "issue": issue,

          }

-         msg = IssueNewV1(body=body)

          config = {

              "dist_git_url": "https://src.fedoraproject.org",

              "dist_git_token": "Private API Key",

+             "temp_folder": "/var/tmp",

          }

+         msg = IssueNewV1(body=body)

  

          mock_pagure_io = Mock()

          mock_pagure.return_value = mock_pagure_io
@@ -163,12 +195,10 @@ 

          with patch(

              "toddlers.plugins.pdc_unretire_packages.PDCUnretirePackages.process_ticket"

          ) as mock_process_ticket:

-             mock_process_ticket.side_effect = Exception("Exception")

+             mock_process_ticket.side_effect = Exception("test")

              toddler.process(config, msg)

              mock_process_ticket.assert_called_with(issue)

  

-         mock_pdc.assert_called_with(config)

-         mock_bugzilla.assert_called_with(config)

          mock_pagure_io.add_comment_to_issue.assert_called_once()

  

      @patch("toddlers.utils.pdc.pdc_client_for_config")
@@ -229,13 +259,16 @@ 

          Initialize toddler

          """

          self.toddler = pdc_unretire_packages.PDCUnretirePackages()

+         self.toddler.temp_dir = Mock()

          self.toddler.git_repo = MagicMock()

          self.toddler.pagure_io = Mock()

          self.toddler.koji_session = MagicMock()

  

-     def test_process_ticket_package_name_doesnt_have_required_prefix(self, caplog):

+     def test_process_ticket_issue_title_should_not_contain_more_than_two_words(

+         self, caplog

+     ):

          """

-         Assert that ticket will be closed if title doesn't contain required prefix.

+         Assert that toddler will close issue if title is more than two words.

          """

          caplog.set_level(logging.INFO)

          issue = {
@@ -248,10 +281,9 @@ 

              "id": 55,

          }

          close_msg = (

-             "Unretire can only apply to rpm packages, please add `{0}`"

-             " prefix before the package name"

-         ).format(pdc_unretire_packages.RPM_PREFIX)

- 

+             "The unretirement can be requested just for one package in time, title should not "

+             "contain more than two words."

+         )

          self.toddler.process_ticket(issue)

          assert caplog.records[-1].message == close_msg

          self.toddler.pagure_io.close_issue.assert_called_with(
@@ -261,13 +293,13 @@ 

              reason="Invalid",

          )

  

-     def test_process_ticket_two_packages_requested(self, caplog):

+     def test_process_ticket_package_name_doesnt_have_required_prefix(self, caplog):

          """

-         Assert that ticket will be closed if in single title two packages were requested.

+         Assert that ticket will be closed if title doesn't contain required prefix.

          """

          caplog.set_level(logging.INFO)

          issue = {

-             "title": "Unretire rpms/rubygem-logging lala rpms/pakca",

+             "title": "Unretire rubygem-logging",

              "content": "",

              "full_url": "",

              "user": {
@@ -276,9 +308,9 @@ 

              "id": 55,

          }

          close_msg = (

-             "Requester trying to request more than one package in single ticket "

-             "or made a mistake in namespace."

-         )

+             "Unretire can only apply to rpm packages, please add `{0}`"

+             " prefix before the package name"

+         ).format(pdc_unretire_packages.RPM_PREFIX)

  

          self.toddler.process_ticket(issue)

          assert caplog.records[-1].message == close_msg
@@ -352,7 +384,10 @@ 

          "toddlers.plugins.pdc_unretire_packages.PDCUnretirePackages._is_url_exist",

          return_value=True,

      )

-     def test_process_ticket_clone_repo_error(self, mock_is_url_exist, caplog):

+     @patch("tempfile.TemporaryDirectory")

+     def test_process_ticket_clone_repo_error(

+         self, mock_temp_dir, mock_is_url_exist, caplog

+     ):

          """

          Assert that remote repo doesn't exist or user don't have a permission.

          """
@@ -366,10 +401,7 @@ 

              },

              "id": 55,

          }

-         close_msg = (

-             "Remote repository doesn't exist or you do not have the appropriate "

-             "permissions to access it."

-         )

+         close_msg = "Something went wrong during cloning git repository."

          with patch(

              "toddlers.plugins.pdc_unretire_packages.git.clone_repo",

              side_effect=git.GitCommandError("some_command"),
@@ -392,8 +424,9 @@ 

          "toddlers.plugins.pdc_unretire_packages.PDCUnretirePackages._is_url_exist",

          return_value=True,

      )

+     @patch("tempfile.TemporaryDirectory")

      def test_process_ticket_package_not_ready_for_unretirement(

-         self, mock_is_url_exist, mock_get_tags, mock_git

+         self, mock_temp_dir, mock_is_url_exist, mock_get_tags, mock_git

      ):

          """

          Assert that processing will not continue if package will not be verified.
@@ -436,8 +469,10 @@ 

          "toddlers.plugins.pdc_unretire_packages.PDCUnretirePackages._is_url_exist",

          return_value=True,

      )

+     @patch("tempfile.TemporaryDirectory")

      def test_process_ticket(

          self,

+         mock_temp_dir,

          mock_is_url_exist,

          is_package_ready_mock,

          get_tags_mock,

@@ -126,21 +126,23 @@ 

              return

  

          issue_title = issue["title"]

- 

-         if UNRETIRE_KEYWORD not in issue_title.lower():

+         words_in_issue_title = issue_title.split()

+         if UNRETIRE_KEYWORD != words_in_issue_title[0].lower():

              _log.info(

-                 "The issue doesn't contain keyword '{0}' in the title '{1}'".format(

-                     UNRETIRE_KEYWORD, issue_title

-                 )

+                 "The issue doesn't contain keyword '{0}' in the title '{1}', or it's not a first "

+                 "word in title".format(UNRETIRE_KEYWORD, issue_title)

              )

              return

  

+         print("12")

+         _log.info("Getting temp_folder name from config.")

+         self.temp_dir = config.get("temp_folder", "")

+ 

          _log.info("Getting dist-git url from config.")

          self.dist_git_base = config.get("dist_git_url")

  

          _log.info("Setting up connection to PDC")

          self.pdc_client = pdc.pdc_client_for_config(config)

- 

          _log.info("Setting up connection to Pagure")

          self.pagure_io = pagure.set_pagure(config)

  
@@ -175,33 +177,32 @@ 

          issue_opener = issue["user"]["name"]

          issue_id = issue["id"]

  

-         _log.debug("Verifying that issue title has `{0}` prefix".format(RPM_PREFIX))

-         if RPM_PREFIX not in issue_title.lower():

+         _log.debug(

+             "Verifying that issue title contain just 'unretire' keyword and package name."

+         )

+         words_in_issue_title = issue_title.split()

+         if len(words_in_issue_title) > 2:

              msg = (

-                 "Unretire can only apply to rpm packages, please add `{0}`"

-                 " prefix before the package name"

-             ).format(RPM_PREFIX)

+                 "The unretirement can be requested just for one package in time, title should not "

+                 "contain more than two words."

+             )

              _log.info(msg)

              self.pagure_io.close_issue(

-                 issue_id,

-                 namespace=PROJECT_NAMESPACE,

-                 message=msg,

-                 reason="Invalid",

+                 issue_id, namespace=PROJECT_NAMESPACE, message=msg, reason="Invalid"

              )

              return

  

-         _log.debug("Verifying that just single package is requested in one ticket.")

-         words_in_issue_title = issue_title.split()

-         words_with_prefix = [

-             word.lower()

-             for word in words_in_issue_title

-             if word.lower().startswith(RPM_PREFIX)

-         ]

-         if len(words_with_prefix) != 1:

+         pkg_name_with_ns = words_in_issue_title[1].lower()

+ 

+         _log.debug(

+             "Verifying that package name in issue title"

+             " starts with `{0}` prefix".format(RPM_PREFIX)

+         )

+         if not pkg_name_with_ns.startswith(RPM_PREFIX):

              msg = (

-                 "Requester trying to request more than one package in single ticket "

-                 "or made a mistake in namespace."

-             )

+                 "Unretire can only apply to rpm packages, please add `{0}`"

+                 " prefix before the package name"

+             ).format(RPM_PREFIX)

              _log.info(msg)

              self.pagure_io.close_issue(

                  issue_id,
@@ -211,7 +212,6 @@ 

              )

              return

  

-         pkg_name_with_ns = words_with_prefix[0]

          pkg_name_and_ns = [word for word in pkg_name_with_ns.split("/") if word != ""]

  

          _log.debug("Verifying that package name with namespace are correct.")
@@ -247,24 +247,19 @@ 

              )

              return

  

-         folder_name_for_clonned_repo = "{0}_folder".format(package_name)

+         folder_name_for_clonned_repo = "{0}_repository_folder".format(package_name)

  

          _log.info("Creating temporary directory")

-         with tempfile.TemporaryDirectory():

+         with tempfile.TemporaryDirectory(dir=self.temp_dir) as tmp_dir:

              _log.info(

                  "Cloning repo into dir with name '{0}'".format(

                      folder_name_for_clonned_repo

                  )

              )

              try:

-                 self.git_repo = git.clone_repo(

-                     package_url, folder_name_for_clonned_repo

-                 )

+                 self.git_repo = git.clone_repo(package_url, tmp_dir)

              except GitCommandError:

-                 message = (

-                     "Remote repository doesn't exist or you do not have the appropriate "

-                     "permissions to access it."

-                 )

+                 message = "Something went wrong during cloning git repository."

                  _log.info(message)

                  self.pagure_io.close_issue(

                      issue_id,

Bug fixes for unretire toddler, related to processing issue title and creating temporary repository.

Signed-off-by: amedvede amedvede@redhat.com

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/6d10a8a515034fd19a71eb9bce5c5c74

1 new commit added

  • test: changed and added tests to cover fixes in plugin
2 months ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/a14a71677b814374bae79c233e78faf9

1 new commit added

  • format: format fixes for pdc_unretirement tests
2 months ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/2860b0eeb2344c77a5999c002d1e7cb3

1 new commit added

  • format: black formating of pdc_urentire plugin
2 months ago

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/1bd23092f44f4b7bbf687c504c8e0c1f

rebased onto 075d83d

2 months ago

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/11eb34bff5b44419a4ee31a17a9d0f9f

Build succeeded.
https://fedora.softwarefactory-project.io/zuul/buildset/79113dc1b858464bac32a32da9e732ec

Pull-Request has been merged by zlopez

2 months ago