Work with full workflow artifact data object from GitHub API

The GitHub API is used to discover the sketches report workflow artifacts. The artifact archive is then downloaded from
the URL provided by the API response. Previously, the artifact discovery methods only returned the download URL.

The API response also contains the somewhat useful information of the artifact name. Previously this information was
discarded because the name is already available indirectly from the sketches-reports-source input value. However, that
will no longer be the case once pattern matching support for the sketches-reports-source input is added. The code is
refactored to pass the full data object received from the API for the artifact between the functions. This gives direct
access to the exact artifact name.

Author: per1234

reportsizedeltas/reportsizedeltas.py

@@ -140,15 +140,15 @@ def report_size_deltas_from_workflow_artifacts(self) -> None:
                     print("::debug::Report already exists")
                     continue
 
-                artifact_download_url = self.get_artifact_download_url_for_sha(
+                artifact_data = self.get_artifact_data_for_sha(
                     pr_user_login=pr_data["user"]["login"], pr_head_ref=pr_data["head"]["ref"], pr_head_sha=pr_head_sha
                 )
-                if artifact_download_url is None:
+                if artifact_data is None:
                     # Go on to the next PR
                     print("::debug::No sketches report artifact found")
                     continue
 
-                artifact_folder_object = self.get_artifact(artifact_download_url=artifact_download_url)
+                artifact_folder_object = self.get_artifact(artifact_data=artifact_data)
 
                 sketches_reports = self.get_sketches_reports(artifact_folder_object=artifact_folder_object)
 
@@ -195,8 +195,8 @@ def report_exists(self, pr_number: int, pr_head_sha: str) -> bool:
         # No reports found for the PR's head SHA
         return False
 
-    def get_artifact_download_url_for_sha(self, pr_user_login: str, pr_head_ref: str, pr_head_sha: str) -> str | None:
-        """Return the report artifact download URL associated with the given head commit hash.
+    def get_artifact_data_for_sha(self, pr_user_login: str, pr_head_ref: str, pr_head_sha: str) -> str | None:
+        """Return the data for the report artifact associated with the given head commit hash.
 
         Keyword arguments:
         pr_user_login -- user name of the PR author (used to reduce number of GitHub API requests)
@@ -222,18 +222,18 @@ def get_artifact_download_url_for_sha(self, pr_user_login: str, pr_head_ref: str
             for run_data in runs_data["workflow_runs"]:
                 if run_data["head_sha"] == pr_head_sha:
                     # Check if this run has the artifact we're looking for
-                    artifact_download_url = self.get_artifact_download_url_for_run(run_id=run_data["id"])
-                    if artifact_download_url is not None:
-                        return artifact_download_url
+                    artifact_data = self.get_artifact_data_for_run(run_id=run_data["id"])
+                    if artifact_data is not None:
+                        return artifact_data
 
             page_number += 1
             page_count = api_data["page_count"]
 
         # No matching artifact found
         return None
 
-    def get_artifact_download_url_for_run(self, run_id: str) -> str | None:
-        """Return the report artifact download URL associated with the given GitHub Actions workflow run.
+    def get_artifact_data_for_run(self, run_id: str):
+        """Return the data for the report artifact associated with the given GitHub Actions workflow run.
 
         Keyword arguments:
         run_id -- GitHub Actions workflow run ID
@@ -251,27 +251,27 @@ def get_artifact_download_url_for_run(self, run_id: str) -> str | None:
             for artifact_data in artifacts_data["artifacts"]:
                 # The artifact is identified by a specific name
                 if not artifact_data["expired"] and artifact_data["name"] == self.sketches_reports_source:
-                    return artifact_data["archive_download_url"]
+                    return artifact_data
 
             page_number += 1
             page_count = api_data["page_count"]
 
         # No matching artifact found
         return None
 
-    def get_artifact(self, artifact_download_url: str):
+    def get_artifact(self, artifact_data):
         """Download and unzip the artifact and return an object for the temporary directory containing it.
 
         Keyword arguments:
-        artifact_download_url -- URL to download the artifact from GitHub
+        artifact_data -- data object for the artifact
         """
         # Create temporary folder
         artifact_folder_object = tempfile.TemporaryDirectory(prefix="reportsizedeltas-")
         try:
-            artifact_zip_file = artifact_folder_object.name + "/" + self.sketches_reports_source + ".zip"
+            artifact_zip_file = artifact_folder_object.name + "/" + artifact_data["name"] + ".zip"
             # Download artifact
             with open(file=artifact_zip_file, mode="wb") as out_file:
-                with self.raw_http_request(url=artifact_download_url) as fp:
+                with self.raw_http_request(url=artifact_data["archive_download_url"]) as fp:
                     out_file.write(fp.read())
 
             # Unzip artifact

reportsizedeltas/tests/test_reportsizedeltas.py

@@ -239,7 +239,7 @@ def test_report_size_deltas_from_local_reports(mocker, monkeypatch):
 
 
 def test_report_size_deltas_from_workflow_artifacts(mocker):
-    artifact_download_url = "test_artifact_download_url"
+    artifact_data = unittest.mock.sentinel.artifact_data
     artifact_folder_object = "test_artifact_folder_object"
     pr_head_sha = "pr-head-sha"
     sketches_reports = [{reportsizedeltas.ReportSizeDeltas.ReportKeys.commit_hash: pr_head_sha}]
@@ -258,9 +258,9 @@ def test_report_size_deltas_from_workflow_artifacts(mocker):
     )
     mocker.patch("reportsizedeltas.ReportSizeDeltas.report_exists", autospec=True, return_value=False)
     mocker.patch(
-        "reportsizedeltas.ReportSizeDeltas.get_artifact_download_url_for_sha",
+        "reportsizedeltas.ReportSizeDeltas.get_artifact_data_for_sha",
         autospec=True,
-        return_value=artifact_download_url,
+        return_value=artifact_data,
     )
     mocker.patch("reportsizedeltas.ReportSizeDeltas.get_artifact", autospec=True, return_value=artifact_folder_object)
     mocker.patch("reportsizedeltas.ReportSizeDeltas.get_sketches_reports", autospec=True, return_value=sketches_reports)
@@ -286,15 +286,15 @@ def test_report_size_deltas_from_workflow_artifacts(mocker):
 
     # Test handling of no report artifact
     reportsizedeltas.ReportSizeDeltas.report_exists.return_value = False
-    reportsizedeltas.ReportSizeDeltas.get_artifact_download_url_for_sha.return_value = None
+    reportsizedeltas.ReportSizeDeltas.get_artifact_data_for_sha.return_value = None
     mocker.resetall()
 
     report_size_deltas.report_size_deltas_from_workflow_artifacts()
 
     report_size_deltas.comment_report.assert_not_called()
 
     # Test handling of old sketches report artifacts
-    reportsizedeltas.ReportSizeDeltas.get_artifact_download_url_for_sha.return_value = artifact_download_url
+    reportsizedeltas.ReportSizeDeltas.get_artifact_data_for_sha.return_value = artifact_data
     reportsizedeltas.ReportSizeDeltas.get_sketches_reports.return_value = None
     mocker.resetall()
 
@@ -321,15 +321,15 @@ def test_report_size_deltas_from_workflow_artifacts(mocker):
     report_size_deltas.report_size_deltas_from_workflow_artifacts()
 
     report_exists_calls = []
-    get_artifact_download_url_for_sha_calls = []
+    get_artifact_data_for_sha_calls = []
     get_sketches_reports_calls = []
     generate_report_calls = []
     comment_report_calls = []
     for pr_data in json_data:
         report_exists_calls.append(
             unittest.mock.call(report_size_deltas, pr_number=pr_data["number"], pr_head_sha=json_data[0]["head"]["sha"])
         )
-        get_artifact_download_url_for_sha_calls.append(
+        get_artifact_data_for_sha_calls.append(
             unittest.mock.call(
                 report_size_deltas,
                 pr_user_login=pr_data["user"]["login"],
@@ -345,8 +345,8 @@ def test_report_size_deltas_from_workflow_artifacts(mocker):
             unittest.mock.call(report_size_deltas, pr_number=pr_data["number"], report_markdown=report)
         )
     report_size_deltas.report_exists.assert_has_calls(calls=report_exists_calls)
-    report_size_deltas.get_artifact_download_url_for_sha.assert_has_calls(calls=get_artifact_download_url_for_sha_calls)
-    report_size_deltas.get_artifact.assert_called_with(report_size_deltas, artifact_download_url=artifact_download_url)
+    report_size_deltas.get_artifact_data_for_sha.assert_has_calls(calls=get_artifact_data_for_sha_calls)
+    report_size_deltas.get_artifact.assert_called_with(report_size_deltas, artifact_data=artifact_data)
     report_size_deltas.get_sketches_reports.assert_has_calls(calls=get_sketches_reports_calls)
     report_size_deltas.generate_report.assert_has_calls(calls=generate_report_calls)
     report_size_deltas.comment_report.assert_has_calls(calls=comment_report_calls)
@@ -373,12 +373,12 @@ def test_report_exists():
     assert not report_size_deltas.report_exists(pr_number=pr_number, pr_head_sha="asdf")
 
 
-def test_get_artifact_download_url_for_sha():
+def test_get_artifact_data_for_sha():
     repository_name = "test_name/test_repo"
     pr_user_login = "test_pr_user_login"
     pr_head_ref = "test_pr_head_ref"
     pr_head_sha = "bar123"
-    test_artifact_url = "test_artifact_url"
+    test_artifact_data = unittest.mock.sentinel.artifact_data
     run_id = "4567"
 
     report_size_deltas = get_reportsizedeltas_object(repository_name=repository_name)
@@ -387,11 +387,11 @@ def test_get_artifact_download_url_for_sha():
     report_size_deltas.api_request = unittest.mock.MagicMock(
         return_value={"json_data": json_data, "additional_pages": True, "page_count": 3}
     )
-    report_size_deltas.get_artifact_download_url_for_run = unittest.mock.MagicMock(return_value=None)
+    report_size_deltas.get_artifact_data_for_run = unittest.mock.MagicMock(return_value=None)
 
     # No SHA match
     assert (
-        report_size_deltas.get_artifact_download_url_for_sha(
+        report_size_deltas.get_artifact_data_for_sha(
             pr_user_login=pr_user_login, pr_head_ref=pr_head_ref, pr_head_sha="foosha"
         )
         is None
@@ -409,30 +409,30 @@ def test_get_artifact_download_url_for_sha():
 
     # SHA match, but no artifact for run
     assert (
-        report_size_deltas.get_artifact_download_url_for_sha(
+        report_size_deltas.get_artifact_data_for_sha(
             pr_user_login=pr_user_login, pr_head_ref=pr_head_ref, pr_head_sha=pr_head_sha
         )
         is None
     )
 
-    report_size_deltas.get_artifact_download_url_for_run = unittest.mock.MagicMock(return_value=test_artifact_url)
+    report_size_deltas.get_artifact_data_for_run = unittest.mock.MagicMock(return_value=test_artifact_data)
 
     # SHA match, artifact match
-    assert test_artifact_url == (
-        report_size_deltas.get_artifact_download_url_for_sha(
+    assert test_artifact_data == (
+        report_size_deltas.get_artifact_data_for_sha(
             pr_user_login=pr_user_login, pr_head_ref=pr_head_ref, pr_head_sha=pr_head_sha
         )
     )
 
-    report_size_deltas.get_artifact_download_url_for_run.assert_called_once_with(run_id=run_id)
+    report_size_deltas.get_artifact_data_for_run.assert_called_once_with(run_id=run_id)
 
 
 @pytest.mark.parametrize("expired", [True, False])
 @pytest.mark.parametrize("artifact_name", ["test_sketches_reports_source", "incorrect-artifact-name"])
-def test_get_artifact_download_url_for_run(expired, artifact_name):
+def test_get_artifact_data_for_run(expired, artifact_name):
     repository_name = "test_name/test_repo"
     sketches_reports_source = "test_sketches_reports_source"
-    archive_download_url = "archive_download_url"
+    artifact_data = {"name": artifact_name, "archive_download_url": "archive_download_url", "expired": expired}
     run_id = "1234"
 
     report_size_deltas = get_reportsizedeltas_object(
@@ -441,19 +441,19 @@ def test_get_artifact_download_url_for_run(expired, artifact_name):
 
     json_data = {
         "artifacts": [
-            {"name": artifact_name, "archive_download_url": archive_download_url, "expired": expired},
+            artifact_data,
             {"name": "bar123", "archive_download_url": "wrong_artifact_url", "expired": False},
         ]
     }
     report_size_deltas.api_request = unittest.mock.MagicMock(
         return_value={"json_data": json_data, "additional_pages": False, "page_count": 1}
     )
 
-    download_url = report_size_deltas.get_artifact_download_url_for_run(run_id=run_id)
+    data = report_size_deltas.get_artifact_data_for_run(run_id=run_id)
     if not expired and artifact_name == sketches_reports_source:
-        assert download_url == archive_download_url
+        assert data == artifact_data
     else:
-        assert download_url is None
+        assert data is None
 
     report_size_deltas.api_request.assert_called_once_with(
         request="repos/" + repository_name + "/actions/runs/" + str(run_id) + "/artifacts", page_number=1
@@ -472,7 +472,10 @@ def test_get_artifact(tmp_path, test_artifact_name, expected_success):
     real_artifact_name = "correct-artifact-name"
     artifact_archive_destination_path = artifact_destination_path.joinpath(real_artifact_name + ".zip")
 
-    artifact_download_url = artifact_destination_path.joinpath(test_artifact_name + ".zip").as_uri()
+    artifact_data = {
+        "archive_download_url": artifact_destination_path.joinpath(test_artifact_name + ".zip").as_uri(),
+        "name": "artifact_name",
+    }
 
     # Create an archive file
     with zipfile.ZipFile(file=artifact_archive_destination_path, mode="a") as zip_ref:
@@ -482,14 +485,14 @@ def test_get_artifact(tmp_path, test_artifact_name, expected_success):
     report_size_deltas = get_reportsizedeltas_object()
 
     if expected_success:
-        artifact_folder_object = report_size_deltas.get_artifact(artifact_download_url=artifact_download_url)
+        artifact_folder_object = report_size_deltas.get_artifact(artifact_data=artifact_data)
 
         with artifact_folder_object as artifact_folder:
             # Verify that the artifact matches the source
             assert directories_are_same(left_directory=artifact_source_path, right_directory=artifact_folder)
     else:
         with pytest.raises(expected_exception=urllib.error.URLError):
-            report_size_deltas.get_artifact(artifact_download_url=artifact_download_url)
+            report_size_deltas.get_artifact(artifact_data=artifact_data)
 
 
 @pytest.mark.parametrize(