From 51a8b97cc1587265414862ae3c6af2c8d02af62f Mon Sep 17 00:00:00 2001 From: Toddy Mladenov Date: Thu, 29 Oct 2026 13:23:54 -0700 Subject: [PATCH] test: repair catalog and authentication baseline Make catalog pagination mocks update last_response, preserve existing result/cursor/error assertions, and add real-client contract cases. Verify the intended resource-to-v2 authentication probe sequence with no credential exchange. Add hosted test validation without changing production behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/tests.yml | 27 ++++ CONTRIBUTING.md | 13 ++ src/regshape/tests/test_catalog_operations.py | 135 +++++++++++++----- src/regshape/tests/test_transport_client.py | 21 ++- 4 files changed, 151 insertions(+), 45 deletions(-) create mode 100644 .github/workflows/tests.yml diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 0000000..47c8c93 --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,27 @@ +name: Tests + +on: + push: + branches: + - main + pull_request: + +permissions: + contents: read + +jobs: + pytest: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python-version: ["3.10", "3.11", "3.12"] + steps: + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6 + with: + python-version: ${{ matrix.python-version }} + - name: Install package and test dependencies + run: python -m pip install -e ".[dev]" + - name: Run tests + run: python -m pytest -q diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index eb96883..0ad5e73 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -8,6 +8,19 @@ Create an `.env` file in the root directory of the project and add the following PYTHONPATH=/src ``` +## Testing + +Install the package and its test dependencies, then run the test suite: + +```bash +python -m pip install -e ".[dev]" +python -m pytest -q +``` + +The [test workflow](.github/workflows/tests.yml) runs the suite on Python 3.10, +3.11, and 3.12 for pull requests and pushes to `main`. Tests include mocked +registry interactions; they do not require a live registry or Docker daemon. + ## Releasing ### Version Tagging Convention diff --git a/src/regshape/tests/test_catalog_operations.py b/src/regshape/tests/test_catalog_operations.py index 1976d73..baa9219 100644 --- a/src/regshape/tests/test_catalog_operations.py +++ b/src/regshape/tests/test_catalog_operations.py @@ -3,13 +3,13 @@ """ Tests for :mod:`regshape.libs.catalog.operations`. -Exercises the domain functions directly (without the CLI layer). The -:class:`~regshape.libs.transport.RegistryClient` and HTTP responses are -replaced with lightweight mocks so no network I/O is performed. +Exercises the domain functions with lightweight client mocks. Contract tests +also use the real :class:`~regshape.libs.transport.RegistryClient` with mocked +HTTP responses. No network I/O is performed. """ import json -from unittest.mock import MagicMock, call +from unittest.mock import MagicMock, call, patch import pytest @@ -21,7 +21,7 @@ ) from regshape.libs.errors import AuthError, CatalogError, CatalogNotSupportedError from regshape.libs.models.catalog import RepositoryCatalog - +from regshape.libs.transport import RegistryClient, TransportConfig # --------------------------------------------------------------------------- # Helpers @@ -30,10 +30,30 @@ REGISTRY = "acr.example.io" -def _make_client(registry: str = REGISTRY) -> MagicMock: - """Return a minimal mock RegistryClient.""" +def _make_client( + registry: str = REGISTRY, + *, + responses: list[MagicMock] | None = None, +) -> MagicMock: + """Build a mock whose configured response sequence mirrors client metadata. + + A real RegistryClient updates ``last_response`` before returning from + ``get``. Pagination fixtures must do the same so the operation can read + the continuation headers from the page it just fetched. + """ client = MagicMock() client.config.registry = registry + client.last_response = None + if responses is not None: + pages = iter(responses) + + def get(*_args: object, **_kwargs: object) -> MagicMock: + """Return the next queued response and publish its pagination metadata.""" + response = next(pages) + client.last_response = response + return response + + client.get.side_effect = get return client @@ -43,7 +63,7 @@ def _make_response( link: str | None = None, body: str | None = None, ) -> MagicMock: - """Return a mock HTTP response for a catalog endpoint.""" + """Return catalog response text, bytes, and headers for mock or real clients.""" response = MagicMock() response.status_code = status_code if body is not None: @@ -52,6 +72,7 @@ def _make_response( response.text = json.dumps({"repositories": repositories}) else: response.text = json.dumps({"repositories": []}) + response.content = response.text.encode("utf-8") headers = {} if link is not None: headers["Link"] = link @@ -188,51 +209,52 @@ def test_malformed_json_raises_catalog_error(self): # =========================================================================== class TestListCatalogAll: + """Verify traversal with faithful mocks and the real client metadata contract.""" - def test_single_page_no_link_header(self): + def test_single_page_no_link_header(self) -> None: + """Stop after a page whose actual response has no continuation link.""" repos = ["library/ubuntu", "myrepo/myimage"] - client = _make_client() - client.get.return_value = _make_response(repos) + client = _make_client(responses=[_make_response(repos)]) result = list_catalog_all(client) assert result.repositories == repos assert client.get.call_count == 1 - def test_two_pages_merged(self): + def test_two_pages_merged(self) -> None: + """Merge both pages after the client publishes the first continuation.""" page1_repos = ["a/one", "a/two"] page2_repos = ["b/three", "b/four"] - client = _make_client() - client.get.side_effect = [ + client = _make_client(responses=[ _make_response(page1_repos, link=_next_link("a/two")), _make_response(page2_repos), - ] + ]) result = list_catalog_all(client) assert result.repositories == page1_repos + page2_repos assert client.get.call_count == 2 - def test_three_pages_merged(self): + def test_three_pages_merged(self) -> None: + """Follow successive continuation headers until the final page.""" pages = [["a/one"], ["b/two"], ["c/three"]] - client = _make_client() - client.get.side_effect = [ + client = _make_client(responses=[ _make_response(pages[0], link=_next_link("a/one")), _make_response(pages[1], link=_next_link("b/two")), _make_response(pages[2]), - ] + ]) result = list_catalog_all(client) assert result.repositories == ["a/one", "b/two", "c/three"] assert client.get.call_count == 3 - def test_second_request_uses_cursor_from_link(self): - client = _make_client() - client.get.side_effect = [ + def test_second_request_uses_cursor_from_link(self) -> None: + """Keep the configured page size and send the cursor from the prior response.""" + client = _make_client(responses=[ _make_response(["a/one"], link=_next_link("a/one", n=1)), _make_response(["b/two"]), - ] + ]) list_catalog_all(client, page_size=1) @@ -240,46 +262,81 @@ def test_second_request_uses_cursor_from_link(self): assert calls[0] == call("/v2/_catalog", params={"n": 1}) assert calls[1] == call("/v2/_catalog", params={"n": 1, "last": "a/one"}) - def test_no_page_size_omits_n_param(self): - client = _make_client() - client.get.return_value = _make_response(["a/one"]) + def test_no_page_size_omits_n_param(self) -> None: + """Omit n when the caller leaves the page size unspecified.""" + client = _make_client(responses=[_make_response(["a/one"])]) list_catalog_all(client) client.get.assert_called_once_with("/v2/_catalog", params=None) - def test_returns_repository_catalog_instance(self): - client = _make_client() - client.get.return_value = _make_response(["a/b"]) + def test_returns_repository_catalog_instance(self) -> None: + """Return the public catalog model after traversal.""" + client = _make_client(responses=[_make_response(["a/b"])]) result = list_catalog_all(client) assert isinstance(result, RepositoryCatalog) - def test_empty_registry_returns_empty_catalog(self): - client = _make_client() - client.get.return_value = _make_response([]) + def test_empty_registry_returns_empty_catalog(self) -> None: + """Treat an empty final page as a successful empty catalog.""" + client = _make_client(responses=[_make_response([])]) result = list_catalog_all(client) assert result.repositories == [] - def test_404_propagates_as_catalog_not_supported(self): - client = _make_client() - client.get.return_value = _make_response(status_code=404) + def test_404_propagates_as_catalog_not_supported(self) -> None: + """Preserve the unsupported-catalog error rather than returning an empty list.""" + client = _make_client(responses=[_make_response(status_code=404)]) with pytest.raises(CatalogNotSupportedError): list_catalog_all(client) - def test_error_on_second_page_propagates(self): - client = _make_client() - client.get.side_effect = [ + def test_error_on_second_page_propagates(self) -> None: + """Reach the second page and propagate its registry error.""" + client = _make_client(responses=[ _make_response(["a/one"], link=_next_link("a/one")), _make_response(status_code=500), - ] + ]) with pytest.raises(CatalogError, match="Registry error"): list_catalog_all(client) + assert client.get.call_count == 2 + + @pytest.mark.parametrize("enable_middleware", [False, True]) + @pytest.mark.parametrize("second_status", [200, 500]) + def test_real_client_response_metadata_drives_pagination( + self, enable_middleware: bool, second_status: int + ) -> None: + """Verify the mock contract against both real transports, without network I/O.""" + responses = [ + _make_response(["a/one"], link=_next_link("a/one", n=1)), + _make_response(["b/two"], status_code=second_status), + ] + with ( + patch("regshape.libs.transport.client.resolve_credentials", return_value=(None, None)), + patch("regshape.libs.transport.client.http_request", side_effect=responses) as http, + ): + client = RegistryClient(TransportConfig( + registry=REGISTRY, enable_middleware=enable_middleware, + )) + if second_status == 500: + with pytest.raises(CatalogError, match="Registry error"): + list_catalog_all(client, page_size=1) + else: + result = list_catalog_all(client, page_size=1) + assert result.repositories == ["a/one", "b/two"] + + assert client.last_response is responses[1] + assert http.call_count == 2 + urls = [ + entry.kwargs["url"] if "url" in entry.kwargs else entry.args[0] + for entry in http.call_args_list + ] + assert urls == [f"https://{REGISTRY}/v2/_catalog"] * 2 + assert http.call_args_list[0].kwargs["params"] == {"n": 1} + assert http.call_args_list[1].kwargs["params"] == {"n": 1, "last": "a/one"} # =========================================================================== diff --git a/src/regshape/tests/test_transport_client.py b/src/regshape/tests/test_transport_client.py index 85f176b..88cc69d 100644 --- a/src/regshape/tests/test_transport_client.py +++ b/src/regshape/tests/test_transport_client.py @@ -2,15 +2,15 @@ """Tests for :mod:`regshape.libs.transport.client`.""" +from unittest.mock import MagicMock, call, patch + import pytest import requests -from unittest.mock import MagicMock, patch from regshape.libs.errors import AuthError from regshape.libs.transport.client import RegistryClient, TransportConfig from regshape.libs.transport.middleware import _normalize_www_authenticate - # --------------------------------------------------------------------------- # Constants # --------------------------------------------------------------------------- @@ -302,6 +302,7 @@ def test_basic_challenge_no_password_raises(self): # =========================================================================== class TestRegistryClientRequest401NoChallenge: + """Verify challenge discovery and terminal failure without credential exchange.""" def test_401_without_www_authenticate_raises_auth_error(self): challenge = _make_response(401) @@ -309,12 +310,20 @@ def test_401_without_www_authenticate_raises_auth_error(self): with pytest.raises(AuthError, match="without a WWW-Authenticate header"): _client().get(PATH) - def test_401_without_www_authenticate_only_one_call_made(self): + def test_401_without_www_authenticate_probes_v2_then_fails(self) -> None: + """Probe once for a missing challenge, then fail without exchanging credentials.""" challenge = _make_response(401) - with patch("regshape.libs.transport.client.http_request", return_value=challenge) as mock: - with pytest.raises(AuthError): + with ( + patch("regshape.libs.transport.client.http_request", return_value=challenge) as mock, + patch("regshape.libs.transport.client.registryauth.authenticate") as authenticate, + ): + with pytest.raises(AuthError, match="without a WWW-Authenticate header"): _client().get(PATH) - assert mock.call_count == 1 + assert mock.call_args_list == [ + call(f"https://{REGISTRY}{PATH}", "GET", headers={}, timeout=30), + call(f"https://{REGISTRY}/v2/", "GET", headers={}, timeout=30), + ] + authenticate.assert_not_called() # ===========================================================================