polarion_approve_automate() should compare commit hashes to get the git diff. (#153)

* mark automated can't work with branch with the branch being deleted after PR merge

* address comments

* updates based on review

Author: dbasunag

apps/polarion/polarion_set_automated.py

@@ -20,12 +20,22 @@ def approve_tests(polarion_project_id: str, added_ids: list[str]) -> dict[str, l
 
 
 def remove_approved_tests(
-    polarion_project_id: str, branch: str, added_ids: list[str] | None = None
+    polarion_project_id: str,
+    branch: str | None = None,
+    previous_commit: str | None = None,
+    current_commit: str | None = None,
+    added_ids: list[str] | None = None,
 ) -> dict[str, list[str]]:
     removed_polarions = {}
     added_ids = added_ids or []
     if removed_ids := set(
-        find_polarion_ids(polarion_project_id=polarion_project_id, string_to_match="removed", branch=branch)
+        find_polarion_ids(
+            polarion_project_id=polarion_project_id,
+            string_to_match="removed",
+            branch=branch,
+            previous_commit=previous_commit,
+            current_commit=current_commit,
+        )
     ) - set(added_ids):
         LOGGER.info(f"Following polarion ids were removed: {removed_ids}")
         removed_polarions = update_polarion_ids(
@@ -43,9 +53,12 @@ def remove_approved_tests(
     default=os.path.expanduser("~/.config/python-utility-scripts/config.yaml"),
 )
 @click.option("--project-id", "-p", help="Provide the polarion project id")
-@click.option("--branch", "-b", help="Provide the github remote branch to run against", default="origin/main")
+@click.option("--previous-commit", "-p", help="Provide previous-commit to compare against", required=True)
+@click.option("--current-commit", "-c", help="Provide current-commit to compare with", required=True)
 @click.option("--verbose", default=False, is_flag=True)
-def polarion_approve_automate(config_file_path: str, project_id: str, branch: str, verbose: bool) -> None:
+def polarion_approve_automate(
+    config_file_path: str, project_id: str, previous_commit: str, current_commit: str, verbose: bool
+) -> None:
     if verbose:
         LOGGER.setLevel(logging.DEBUG)
         # since the utilities are in apps.polarion.polarion_utils, we need to change log level
@@ -56,12 +69,21 @@ def polarion_approve_automate(config_file_path: str, project_id: str, branch: st
         config_file_path=config_file_path, util_name="pyutils-polarion-set-automated"
     )
     added_polarions = {}
-    if added_ids := find_polarion_ids(polarion_project_id=polarion_project_id, string_to_match="added", branch=branch):
+    if added_ids := find_polarion_ids(
+        polarion_project_id=polarion_project_id,
+        string_to_match="added",
+        branch=None,
+        previous_commit=previous_commit,
+        current_commit=current_commit,
+    ):
         added_polarions = approve_tests(polarion_project_id=polarion_project_id, added_ids=added_ids)
         LOGGER.debug(f"Following polarion ids were marked automated and approved: {added_polarions.get('updated')}")
 
     removed_polarions = remove_approved_tests(
-        polarion_project_id=polarion_project_id, added_ids=added_ids, branch=branch
+        polarion_project_id=polarion_project_id,
+        added_ids=added_ids,
+        previous_commit=previous_commit,
+        current_commit=current_commit,
     )
     if removed_polarions.get("failed") or added_polarions.get("failed"):
         error = "Following polarion ids updates failed."

apps/polarion/polarion_utils.py

@@ -15,14 +15,28 @@
 APPROVED = "approved"
 
 
-def git_diff(branch: str) -> str:
-    data = subprocess.check_output(shlex.split(f"git diff {branch} HEAD"))
+def git_diff(branch: str | None = None, current_commit: str | None = None, previous_commit: str | None = None) -> str:
+    if branch and (previous_commit or current_commit):
+        LOGGER.error("Branch and Previous or current commit are mutually exclusive command line options.")
+        sys.exit(1)
+
+    # Sanitize inputs to prevent command injection
+    if branch:
+        sanitized_branch = shlex.quote(branch)
+        diff_command = f"git diff {sanitized_branch} HEAD"
+    else:
+        sanitized_previous = shlex.quote(previous_commit) if previous_commit else ""
+        sanitized_current = shlex.quote(current_commit) if current_commit else ""
+        diff_command = f"git diff {sanitized_previous} {sanitized_current}"
+    data = subprocess.check_output(shlex.split(diff_command))
     return data.decode()
 
 
-def git_diff_lines(branch: str) -> dict[str, list[str]]:
+def git_diff_lines(
+    branch: str | None = None, previous_commit: str | None = None, current_commit: str | None = None
+) -> dict[str, list[str]]:
     diff: dict[str, list[str]] = {}
-    for line in git_diff(branch=branch).splitlines():
+    for line in git_diff(branch=branch, current_commit=current_commit, previous_commit=previous_commit).splitlines():
         LOGGER.debug(line)
         if line.startswith("+"):
             diff.setdefault("added", []).append(line)
@@ -43,7 +57,12 @@ def validate_polarion_requirements(
         for _id in polarion_test_ids:
             has_req = False
             LOGGER.debug(f"Checking if {_id} verifies any requirement")
-            tc = TestCase(project_id=polarion_project_id, work_item_id=_id)
+            try:
+                tc = TestCase(project_id=polarion_project_id, work_item_id=_id)
+            except PyleroLibException:
+                LOGGER.error(f"{_id}: Test case not found.")
+                tests_with_missing_requirements.append(_id)
+                continue
             for link in tc.linked_work_items:
                 try:
                     Requirement(project_id=polarion_project_id, work_item_id=link.work_item_id)
@@ -53,15 +72,25 @@ def validate_polarion_requirements(
                     continue
 
             if not has_req:
-                LOGGER.error(f"{_id}: Is missing requirement")
+                LOGGER.error(f"{_id}: does not have associated requirement.")
                 tests_with_missing_requirements.append(_id)
     return tests_with_missing_requirements
 
 
-def find_polarion_ids(polarion_project_id: str, string_to_match: str, branch: str) -> list[str]:
+def find_polarion_ids(
+    polarion_project_id: str,
+    string_to_match: str,
+    branch: str | None = None,
+    previous_commit: str | None = None,
+    current_commit: str | None = None,
+) -> list[str]:
     return re.findall(
         rf"pytest.mark.polarion.*({polarion_project_id}-[0-9]+)",
-        "\n".join(git_diff_lines(branch=branch).get(string_to_match, [])),
+        "\n".join(
+            git_diff_lines(branch=branch, previous_commit=previous_commit, current_commit=current_commit).get(
+                string_to_match, []
+            )
+        ),
         re.MULTILINE | re.IGNORECASE,
     )
 

tests/polarion/test_polarion_automated.py

@@ -6,13 +6,25 @@
 BASE_COMMAND = "poetry run python apps/polarion/polarion_set_automated.py --verbose"
 
 
-def test_missing_project_id_set_automated():
+def test_missing_required_params_set_automated():
     rc, _, err = run_command(
         command=shlex.split(BASE_COMMAND),
         verify_stderr=False,
         check=False,
         capture_output=False,
         stderr=subprocess.PIPE,
     )
+    assert "Missing option" in err
+    assert not rc
+
+
+def test_missing_project_id_set_automated():
+    rc, _, err = run_command(
+        command=shlex.split(f"{BASE_COMMAND} --previous-commit commit1 --current-commit commit2"),
+        verify_stderr=False,
+        check=False,
+        capture_output=False,
+        stderr=subprocess.PIPE,
+    )
     assert "Polarion project id must be passed via config file or command line" in err
     assert not rc