Repository: py-cov-action/python-coverage-comment-action
Pull request #453 (2024-08-04 13:53:56)

add pagination for artifacts retrieval
Hello @ewjoachim,

following the discussion we had some weeks ago with @AlessandroMiola [here](https://github.com/py-cov-action/python-coverage-comment-action/discussions/438), I'm proposing you a small fix in order to handle the pagination for artifacts.

I'm looping page by page and, if the result of page n+1 is empty, I'm breaking out of the loop.
The rationale behind is that If we ask a page without artifact we get an empty response (see for example https://api.github.com/repos/pydantic/pydantic/actions/runs/9815232181/artifacts?page=3)


To be honest, I'm not very happy with this solution, probably the best one would be to migrate from the https://github.com/michaelliao/githubpy client (that I noticed it's not maintained anymore) to something like https://github.com/PyGithub/PyGithub, but the changes would be substantial I guess.

I also took the occasion to introduce a Makefile to ease local development, let me know what you think about it ;) 

Closes #438


Patch:
diff --git a/.gitignore b/.gitignore
index d1b1e7f..95d0cfe 100644
--- a/.gitignore
+++ b/.gitignore
@@ -3,3 +3,4 @@
 .coverage
 dev-env-vars
 .coverage*
+__pycache__
diff --git a/Makefile b/Makefile
new file mode 100644
index 0000000..e7ee4bd
--- /dev/null
+++ b/Makefile
@@ -0,0 +1,12 @@
+.PHONY: install
+install: ## install dependencies
+	poetry install --with dev
+	pre-commit install
+
+.PHONY: lint
+lint: ## lint code
+	pre-commit
+
+.PHONY: test
+test: ## run all tests
+	poetry run pytest tests
diff --git a/coverage_comment/github.py b/coverage_comment/github.py
index 4ca3591..9a06931 100644
--- a/coverage_comment/github.py
+++ b/coverage_comment/github.py
@@ -54,15 +54,15 @@ def download_artifact(
     filename: pathlib.Path,
 ) -> str:
     repo_path = github.repos(repository)
-    artifacts = repo_path.actions.runs(run_id).artifacts.get().artifacts
+
     try:
         artifact = next(
-            iter(artifact for artifact in artifacts if artifact.name == artifact_name),
+            artifact
+            for artifact in _fetch_artifacts(repo_path, run_id)
+            if artifact.name == artifact_name
         )
     except StopIteration:
-        raise NoArtifact(
-            f"Not artifact found with name {artifact_name} in run {run_id}"
-        )
+        raise NoArtifact(f"No artifact found with name {artifact_name} in run {run_id}")
 
     zip_bytes = io.BytesIO(repo_path.actions.artifacts(artifact.id).zip.get(bytes=True))
     zipf = zipfile.ZipFile(zip_bytes)
@@ -73,6 +73,24 @@ def download_artifact(
         raise NoArtifact(f"File named {filename} not found in artifact {artifact_name}")
 
 
+def _fetch_artifacts(repo_path, run_id):
+    page = 1
+    total_fetched = 0
+
+    while True:
+        result = repo_path.actions.runs(run_id).artifacts.get(page=str(page))
+        if not result or not result.artifacts:
+            break
+
+        yield from result.artifacts
+
+        total_fetched += len(result.artifacts)
+        if total_fetched >= result.total_count:
+            break
+
+        page += 1
+
+
 def get_branch_from_workflow_run(
     github: github_client.GitHub, repository: str, run_id: int
 ) -> tuple[str, str]:


Test patch:
diff --git a/tests/integration/test_github.py b/tests/integration/test_github.py
index ca2d5ba..a5b99ec 100644
--- a/tests/integration/test_github.py
+++ b/tests/integration/test_github.py
@@ -52,7 +52,7 @@ def test_download_artifact(gh, session, zip_bytes):
         {"name": "foo", "id": 789},
     ]
     session.register("GET", "/repos/foo/bar/actions/runs/123/artifacts")(
-        json={"artifacts": artifacts}
+        json={"artifacts": artifacts, "total_count": 2}
     )
 
     session.register("GET", "/repos/foo/bar/actions/artifacts/789/zip")(
@@ -70,12 +70,44 @@ def test_download_artifact(gh, session, zip_bytes):
     assert result == "bar"
 
 
+def test_download_artifact_from_page_2(gh, session, zip_bytes):
+    artifacts_page_1 = [
+        {"name": "test", "id": 000},
+    ]
+    artifacts_page_2 = [
+        {"name": "bar", "id": 456},
+        {"name": "foo", "id": 789},
+    ]
+    session.register("GET", "/repos/foo/bar/actions/runs/123/artifacts")(
+        json={"artifacts": artifacts_page_1, "total_count": 3}
+    )
+    session.register(
+        "GET",
+        "/repos/foo/bar/actions/runs/123/artifacts",
+        params={"page": "2"},
+    )(json={"artifacts": artifacts_page_2, "total_count": 3})
+
+    session.register("GET", "/repos/foo/bar/actions/artifacts/789/zip")(
+        content=zip_bytes(filename="foo.txt", content="bar")
+    )
+
+    result = github.download_artifact(
+        github=gh,
+        repository="foo/bar",
+        artifact_name="foo",
+        run_id=123,
+        filename=pathlib.Path("foo.txt"),
+    )
+
+    assert result == "bar"
+
+
 def test_download_artifact__no_artifact(gh, session):
     artifacts = [
         {"name": "bar", "id": 456},
     ]
     session.register("GET", "/repos/foo/bar/actions/runs/123/artifacts")(
-        json={"artifacts": artifacts}
+        json={"artifacts": artifacts, "total_count": 1}
     )
 
     with pytest.raises(github.NoArtifact):
@@ -89,12 +121,15 @@ def test_download_artifact__no_artifact(gh, session):
 
 
 def test_download_artifact__no_file(gh, session, zip_bytes):
-    artifacts = [
-        {"name": "foo", "id": 789},
-    ]
+    artifacts = [{"name": "foo", "id": 789}]
     session.register("GET", "/repos/foo/bar/actions/runs/123/artifacts")(
         json={"artifacts": artifacts}
     )
+    session.register(
+        "GET",
+        "/repos/foo/bar/actions/runs/123/artifacts",
+        params={"page": "2"},
+    )(json={})
 
     session.register("GET", "/repos/foo/bar/actions/artifacts/789/zip")(
         content=zip_bytes(filename="foo.txt", content="bar")
@@ -109,6 +144,59 @@ def test_download_artifact__no_file(gh, session, zip_bytes):
         )
 
 
+def test_fetch_artifacts_empty_response(gh, session):
+    session.register("GET", "/repos/foo/bar/actions/runs/123/artifacts")(
+        json={"artifacts": [], "total_count": 0}
+    )
+
+    repo_path = gh.repos("foo/bar")
+
+    result = github._fetch_artifacts(
+        repo_path=repo_path,
+        run_id=123,
+    )
+
+    assert not list(result)
+
+
+def test_fetch_artifacts_single_page(gh, session):
+    artifacts = [{"name": "bar", "id": 456}]
+
+    session.register("GET", "/repos/foo/bar/actions/runs/123/artifacts")(
+        json={"artifacts": artifacts, "total_count": 1}
+    )
+
+    repo_path = gh.repos("foo/bar")
+
+    result = github._fetch_artifacts(
+        repo_path=repo_path,
+        run_id=123,
+    )
+
+    assert list(result) == artifacts
+
+
+def test_fetch_artifacts_multiple_pages(gh, session):
+    artifacts_page_1 = [{"name": "bar", "id": 456}]
+    artifacts_page_2 = [{"name": "bar", "id": 789}]
+
+    session.register("GET", "/repos/foo/bar/actions/runs/123/artifacts")(
+        json={"artifacts": artifacts_page_1, "total_count": 2}
+    )
+    session.register(
+        "GET", "/repos/foo/bar/actions/runs/123/artifacts", params={"page": "2"}
+    )(json={"artifacts": artifacts_page_2, "total_count": 2})
+
+    repo_path = gh.repos("foo/bar")
+
+    result = github._fetch_artifacts(
+        repo_path=repo_path,
+        run_id=123,
+    )
+
+    assert list(result) == artifacts_page_1 + artifacts_page_2
+
+
 def test_get_branch_from_workflow_run(gh, session):
     json = {
         "head_branch": "other",
diff --git a/tests/integration/test_main.py b/tests/integration/test_main.py
index 10e563f..f925e43 100644
--- a/tests/integration/test_main.py
+++ b/tests/integration/test_main.py
@@ -811,7 +811,7 @@ def test_action__workflow_run__no_artifact(
     session.register(
         "GET",
         "/repos/py-cov-action/foobar/actions/runs/123/artifacts",
-    )(json={"artifacts": [{"name": "wrong_name"}]})
+    )(json={"artifacts": [{"name": "wrong_name"}], "total_count": 1})
 
     result = main.action(
         config=workflow_run_config(),
@@ -852,7 +852,12 @@ def test_action__workflow_run__post_comment(
     session.register(
         "GET",
         "/repos/py-cov-action/foobar/actions/runs/123/artifacts",
-    )(json={"artifacts": [{"name": "python-coverage-comment-action", "id": 789}]})
+    )(
+        json={
+            "artifacts": [{"name": "python-coverage-comment-action", "id": 789}],
+            "total_count": 1,
+        }
+    )
 
     session.register(
         "GET",