Merge pull request #453 from fabiogallotti/add-pagination-for-artifacts

Author: ewjoachim

.gitignore

@@ -3,3 +3,4 @@
 .coverage
 dev-env-vars
 .coverage*
+__pycache__

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

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

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",

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",