From d2c8ca51fc4331dc4e55569c324e8c2c18ac6061 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 25 Aug 2026 12:45:57 +0100 Subject: [PATCH 1/4] chore: optimize CI test coverage Preserve the full Python compatibility matrix while running live integration tests once and reducing PR binary builds to representative platforms. Remove test retry delays and avoid repeated coverage work to shorten feedback without weakening default-branch or release validation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/binaries.yml | 28 ++++++- .github/workflows/image-build.yml | 23 ++++-- .github/workflows/test.yml | 76 +++++++++++++++++-- .github/workflows/ty-check.yml | 8 ++ AGENTS.md | 2 +- .../cli/tests/commands/policy/test_deny.py | 1 + .../cli/tests/commands/policy/test_licence.py | 1 + .../commands/policy/test_vulnerability.py | 1 + .../cli/tests/commands/test_check.py | 10 ++- .../tests/commands/test_credential_helper.py | 6 ++ .../cli/tests/commands/test_entitlements.py | 1 + .../cli/tests/commands/test_login.py | 1 + .../tests/commands/test_package_commands.py | 2 + .../cli/tests/commands/test_repos.py | 1 + .../cli/tests/commands/test_tokens.py | 8 +- .../cli/tests/commands/test_upstream.py | 1 + cloudsmith_cli/cli/tests/conftest.py | 8 +- cloudsmith_cli/core/tests/test_download.py | 2 - cloudsmith_cli/core/tests/test_rest.py | 2 +- pyproject.toml | 5 +- 20 files changed, 159 insertions(+), 28 deletions(-) diff --git a/.github/workflows/binaries.yml b/.github/workflows/binaries.yml index 6ff56cb4..ba5ccbcb 100644 --- a/.github/workflows/binaries.yml +++ b/.github/workflows/binaries.yml @@ -18,6 +18,9 @@ on: CLOUDSMITH_API_KEY: description: Read-only API key for the online smoketests required: false + push: + branches: + - master pull_request: paths: - cloudsmith_cli/** @@ -77,12 +80,31 @@ jobs: persist-credentials: false - id: gen - run: echo "include=$(jq -c . .github/scripts/binaries-matrix.json)" >> "$GITHUB_OUTPUT" + env: + EVENT_NAME: ${{ github.event_name }} + run: | + if [ "${EVENT_NAME}" = "pull_request" ]; then + MATRIX=$( + jq -c '{ + include: [ + .include[] + | select( + .name == "linux-x86_64-gnu" + or .name == "macos-arm64" + or .name == "windows-x86_64" + ) + ] + }' .github/scripts/binaries-matrix.json + ) + else + MATRIX=$(jq -c . .github/scripts/binaries-matrix.json) + fi + echo "include=${MATRIX}" >> "$GITHUB_OUTPUT" build: needs: matrix strategy: - fail-fast: false + fail-fast: ${{ github.event_name == 'pull_request' }} matrix: ${{ fromJson(needs.matrix.outputs.include) }} runs-on: ${{ matrix.build_runner }} timeout-minutes: 45 @@ -207,7 +229,7 @@ jobs: test: needs: [matrix, build] strategy: - fail-fast: false + fail-fast: ${{ github.event_name == 'pull_request' }} matrix: ${{ fromJson(needs.matrix.outputs.include) }} runs-on: ${{ matrix.test_runner }} timeout-minutes: 25 diff --git a/.github/workflows/image-build.yml b/.github/workflows/image-build.yml index c6ad076b..f5e8aa2c 100644 --- a/.github/workflows/image-build.yml +++ b/.github/workflows/image-build.yml @@ -8,20 +8,33 @@ on: - "**" permissions: contents: read + +concurrency: + group: image-build-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: image-build: name: Build container image runs-on: ubuntu-24.04 + timeout-minutes: 30 steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: persist-credentials: false + + - uses: docker/setup-buildx-action@d7f5e7f509e45cec5c76c4d5afdd7de93d0b3df5 # v4.1.0 + - name: Build image run: | - docker build -t cloudsmith-io/cli-dev \ - --file dev.Dockerfile \ - --build-arg CLOUDSMITH_CLI_VERSION="$(cat VERSION)" \ - --build-arg VCS_REF="${GITHUB_SHA}" \ - . + docker buildx build \ + --load \ + --cache-from type=gha \ + --cache-to type=gha,mode=max \ + --tag cloudsmith-io/cli-dev \ + --file dev.Dockerfile \ + --build-arg CLOUDSMITH_CLI_VERSION="$(cat VERSION)" \ + --build-arg VCS_REF="${GITHUB_SHA}" \ + . - name: Test image run: docker run -t cloudsmith-io/cli-dev --help diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c82affd9..28bab59e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -17,10 +17,11 @@ concurrency: jobs: pytest: - name: Run tests (Python ${{ matrix.python-version }}) + name: Unit tests (Python ${{ matrix.python-version }}) runs-on: ubuntu-24.04 + timeout-minutes: 15 strategy: - fail-fast: false + fail-fast: ${{ github.event_name == 'pull_request' }} matrix: python-version: ['3.10', '3.11', '3.12', '3.13', '3.14'] @@ -42,17 +43,82 @@ jobs: - name: Install dependencies run: uv sync --locked --group dev --python ${{ matrix.python-version }} + - name: Run pytest with coverage + if: matrix.python-version == '3.12' + run: >- + uv run pytest + -m "not integration" + --cov=cloudsmith_cli + --junitxml=./reports/pytest.xml + - name: Run pytest + if: matrix.python-version != '3.12' + run: uv run pytest -m "not integration" --junitxml=./reports/pytest.xml + + - name: Upload test results + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: test-results-${{ matrix.python-version }} + path: ./reports/pytest.xml + retention-days: 30 + if-no-files-found: error + + integration: + name: Live integration tests (Python 3.12) + runs-on: ubuntu-24.04 + timeout-minutes: 45 + steps: + - name: Check integration credentials + id: credentials + env: + API_KEY: ${{ secrets.PYTEST_CLOUDSMITH_API_KEY }} + API_HOST: ${{ vars.PYTEST_CLOUDSMITH_API_HOST }} + ORGANIZATION: ${{ vars.PYTEST_CLOUDSMITH_ORGANIZATION }} + run: | + if [ -n "${API_KEY}" ] && [ -n "${API_HOST}" ] && [ -n "${ORGANIZATION}" ]; then + echo "available=true" >> "$GITHUB_OUTPUT" + else + echo "available=false" >> "$GITHUB_OUTPUT" + echo "Skipping live integration tests because credentials are unavailable." + fi + + - if: steps.credentials.outputs.available == 'true' + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + persist-credentials: false + + - name: Set up Python 3.12 + if: steps.credentials.outputs.available == 'true' + uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 + with: + python-version: "3.12" + + - name: Set up uv + if: steps.credentials.outputs.available == 'true' + uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39 # v8.2.0 + with: + enable-cache: true + + - name: Install dependencies + if: steps.credentials.outputs.available == 'true' + run: uv sync --locked --group dev --python 3.12 + + - name: Run live integration tests + if: steps.credentials.outputs.available == 'true' env: PYTEST_CLOUDSMITH_API_KEY: ${{ secrets.PYTEST_CLOUDSMITH_API_KEY }} PYTEST_CLOUDSMITH_API_HOST: ${{ vars.PYTEST_CLOUDSMITH_API_HOST }} PYTEST_CLOUDSMITH_ORGANIZATION: ${{ vars.PYTEST_CLOUDSMITH_ORGANIZATION }} - run: uv run pytest --junitxml=./reports/pytest.xml + PYTEST_CLOUDSMITH_USERNAME: ${{ secrets.PYTEST_CLOUDSMITH_USERNAME }} + PYTEST_CLOUDSMITH_PASSWORD: ${{ secrets.PYTEST_CLOUDSMITH_PASSWORD }} + run: uv run pytest -m integration --junitxml=./reports/pytest.xml - name: Upload test results - if: always() + if: always() && steps.credentials.outputs.available == 'true' uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: - name: test-results-${{ matrix.python-version }} + name: test-results-integration-3.12 path: ./reports/pytest.xml retention-days: 30 + if-no-files-found: error diff --git a/.github/workflows/ty-check.yml b/.github/workflows/ty-check.yml index 93ab43e2..9b33527e 100644 --- a/.github/workflows/ty-check.yml +++ b/.github/workflows/ty-check.yml @@ -1,10 +1,18 @@ name: Type check on: + push: + branches: + - master pull_request: branches: - "**" permissions: contents: read + +concurrency: + group: ty-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: ty: runs-on: ubuntu-24.04 diff --git a/AGENTS.md b/AGENTS.md index 38a67757..25d11afe 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,7 +18,7 @@ Python `>=3.10` is required (CI tests 3.10–3.14). ## Common commands - Run the CLI locally: `cloudsmith ...` (console_script) or `python -m cloudsmith_cli ...`. -- Run tests: `pytest` (configured via `pyproject.toml` `[tool.pytest.ini_options]` — adds `--cov=cloudsmith_cli`). +- Run tests: `pytest`. Run with coverage: `pytest --cov=cloudsmith_cli`. - Run a single test: `pytest cloudsmith_cli/cli/tests/test_push.py::TestClass::test_name` or by node id / `-k `. - Lint/format (all run via pre-commit): `pre-commit run --all-files`. Individual tools: `black .`, `isort .`, `flake8 --config=.flake8`, `pylint --rcfile=.pylintrc `, `pyupgrade --py310-plus `. - Release: `bumpversion ` then `git push origin `. The `VERSION` symlink in repo root points at `cloudsmith_cli/data/VERSION`. diff --git a/cloudsmith_cli/cli/tests/commands/policy/test_deny.py b/cloudsmith_cli/cli/tests/commands/policy/test_deny.py index 17b7fa26..5670961a 100644 --- a/cloudsmith_cli/cli/tests/commands/policy/test_deny.py +++ b/cloudsmith_cli/cli/tests/commands/policy/test_deny.py @@ -74,6 +74,7 @@ def assert_output_matches_policy_config(output, config_file_path): @pytest.mark.usefixtures("set_api_key_env_var", "set_api_host_env_var") +@pytest.mark.integration def test_deny_policy_commands(runner, organization, tmp_path): """Test CRUD operations for deny policies.""" # Generate the deny policy configuration file. diff --git a/cloudsmith_cli/cli/tests/commands/policy/test_licence.py b/cloudsmith_cli/cli/tests/commands/policy/test_licence.py index 32cc438a..12efce4b 100644 --- a/cloudsmith_cli/cli/tests/commands/policy/test_licence.py +++ b/cloudsmith_cli/cli/tests/commands/policy/test_licence.py @@ -86,6 +86,7 @@ def assert_output_matches_policy_config(output, config_file_path): @pytest.mark.usefixtures("set_api_key_env_var", "set_api_host_env_var") +@pytest.mark.integration def test_license_policy_commands(runner, organization, tmp_path): """Test CRUD operations for license policies.""" diff --git a/cloudsmith_cli/cli/tests/commands/policy/test_vulnerability.py b/cloudsmith_cli/cli/tests/commands/policy/test_vulnerability.py index d37146b9..03ee7c54 100644 --- a/cloudsmith_cli/cli/tests/commands/policy/test_vulnerability.py +++ b/cloudsmith_cli/cli/tests/commands/policy/test_vulnerability.py @@ -88,6 +88,7 @@ def assert_output_matches_policy_config(output, config_file_path): @pytest.mark.usefixtures("set_api_key_env_var", "set_api_host_env_var") +@pytest.mark.integration def test_vulnerability_policy_commands(runner, organization, tmp_path): """Test CRUD operations for vulnerability policies.""" diff --git a/cloudsmith_cli/cli/tests/commands/test_check.py b/cloudsmith_cli/cli/tests/commands/test_check.py index 5ebe02e8..0302b5c8 100644 --- a/cloudsmith_cli/cli/tests/commands/test_check.py +++ b/cloudsmith_cli/cli/tests/commands/test_check.py @@ -5,8 +5,9 @@ from ....cli.commands.check import check from ....cli.tests.utils import random_str +FAKE_API_HOST = "https://api.example.com" + -@pytest.mark.usefixtures("set_api_host_env_var") class TestCheckServiceCommand: @pytest.mark.parametrize( "service_version,api_binding_version", @@ -17,9 +18,10 @@ class TestCheckServiceCommand: ], ) def test_check_service_command_output( - self, runner, api_host, service_version, api_binding_version + self, runner, monkeypatch, service_version, api_binding_version ): """Unit test the command output given different combinations of service/binding version.""" + monkeypatch.setenv("CLOUDSMITH_API_HOST", FAKE_API_HOST) service_status = random_str() with ( @@ -39,7 +41,7 @@ def test_check_service_command_output( assert output[0] == "Retrieving service status ... OK" assert output[1] == "" - assert output[2] == f"The service endpoint is: {api_host}" + assert output[2] == f"The service endpoint is: {FAKE_API_HOST}" assert output[3] == f"The service status is: {service_status}" assert ( output[4] @@ -52,6 +54,8 @@ def test_check_service_command_output( else "The API library used by this CLI tool seems to be up-to-date." ) + @pytest.mark.integration + @pytest.mark.usefixtures("set_api_host_env_var") def test_check_service_command(self, runner, api_host): """Integration test the `cloudsmith check service` command (actually hit the API).""" result = runner.invoke(check, args="service", catch_exceptions=False) diff --git a/cloudsmith_cli/cli/tests/commands/test_credential_helper.py b/cloudsmith_cli/cli/tests/commands/test_credential_helper.py index bd944649..5af2594f 100644 --- a/cloudsmith_cli/cli/tests/commands/test_credential_helper.py +++ b/cloudsmith_cli/cli/tests/commands/test_credential_helper.py @@ -254,6 +254,9 @@ def test_get_custom_domains_status_matrix( tmp_path, monkeypatch, status, expect_domains, expect_cached ): """get_custom_domains() caches or not based on HTTP status.""" + if status == 500: + monkeypatch.setattr("cloudsmith_cli.core.rest.time.sleep", lambda _: None) + # redirect per-test (autouse fixture already set module-level path) monkeypatch.setattr( "cloudsmith_cli.credential_helpers.custom_domains.get_default_config_path", @@ -322,6 +325,9 @@ def test_get_custom_domains_strict_raises_on_failure( Consumers presenting results to a user (`cloudsmith domains list`) must not render a typo'd org or an unreachable API as "no custom domains". """ + if status == 500: + monkeypatch.setattr("cloudsmith_cli.core.rest.time.sleep", lambda _: None) + monkeypatch.setattr( "cloudsmith_cli.credential_helpers.custom_domains.get_default_config_path", lambda: str(tmp_path), diff --git a/cloudsmith_cli/cli/tests/commands/test_entitlements.py b/cloudsmith_cli/cli/tests/commands/test_entitlements.py index e9e17d2b..3e61748a 100644 --- a/cloudsmith_cli/cli/tests/commands/test_entitlements.py +++ b/cloudsmith_cli/cli/tests/commands/test_entitlements.py @@ -7,6 +7,7 @@ @pytest.mark.usefixtures("set_api_key_env_var", "set_api_host_env_var") +@pytest.mark.integration def test_entitlements_list_with_show_all(runner, organization, tmp_repository): """Test listing entitlements with --show-all flag.""" org_repo = f"{organization}/{tmp_repository['slug']}" diff --git a/cloudsmith_cli/cli/tests/commands/test_login.py b/cloudsmith_cli/cli/tests/commands/test_login.py index b29be4c7..724fa002 100644 --- a/cloudsmith_cli/cli/tests/commands/test_login.py +++ b/cloudsmith_cli/cli/tests/commands/test_login.py @@ -4,6 +4,7 @@ @pytest.mark.usefixtures("set_api_host_env_var") +@pytest.mark.integration class TestLoginCommand: def test_login_via_prompt(self, runner, username, password, api_key): """Test that a user can `cloudsmith login` with interactive prompts.""" diff --git a/cloudsmith_cli/cli/tests/commands/test_package_commands.py b/cloudsmith_cli/cli/tests/commands/test_package_commands.py index 79c65f01..5140dffe 100644 --- a/cloudsmith_cli/cli/tests/commands/test_package_commands.py +++ b/cloudsmith_cli/cli/tests/commands/test_package_commands.py @@ -12,6 +12,7 @@ @pytest.mark.usefixtures("set_api_key_env_var", "set_api_host_env_var") +@pytest.mark.integration @pytest.mark.parametrize( "filesize", [ @@ -88,6 +89,7 @@ def test_push_and_delete_raw_package( @pytest.mark.usefixtures("set_api_key_env_var", "set_api_host_env_var") +@pytest.mark.integration def test_list_packages_with_sort(runner, organization, tmp_repository, tmp_path): """Test listing packages with different sort options.""" org_repo = f"{organization}/{tmp_repository['slug']}" diff --git a/cloudsmith_cli/cli/tests/commands/test_repos.py b/cloudsmith_cli/cli/tests/commands/test_repos.py index 3f6c990b..4d159167 100644 --- a/cloudsmith_cli/cli/tests/commands/test_repos.py +++ b/cloudsmith_cli/cli/tests/commands/test_repos.py @@ -74,6 +74,7 @@ def assert_output_is_equal_to_repo_config(output, organisation, repo_config_file @pytest.mark.usefixtures("set_api_key_env_var", "set_api_host_env_var") +@pytest.mark.integration def test_repos_commands(runner, organization, tmp_path): """Test CRUD operations for repositories.""" diff --git a/cloudsmith_cli/cli/tests/commands/test_tokens.py b/cloudsmith_cli/cli/tests/commands/test_tokens.py index 293274ca..3cf3e35d 100644 --- a/cloudsmith_cli/cli/tests/commands/test_tokens.py +++ b/cloudsmith_cli/cli/tests/commands/test_tokens.py @@ -9,7 +9,11 @@ from .conftest import MockToken -@pytest.mark.usefixtures("set_api_host_env_var") +@pytest.fixture(autouse=True) +def set_fake_api_host(monkeypatch): + monkeypatch.setenv("CLOUDSMITH_API_HOST", "https://api.example.com") + + class TestListTokensCommand: def test_list_tokens_success(self, runner): """Test successful listing of tokens.""" @@ -53,7 +57,6 @@ def test_list_tokens_error(self, runner): ) -@pytest.mark.usefixtures("set_api_host_env_var") class TestRefreshTokenCommand: """Test suite for the 'tokens refresh' command.""" @@ -113,7 +116,6 @@ def test_refresh_token_list_error(self, runner): ) -@pytest.mark.usefixtures("set_api_host_env_var") class TestRequestApiKeyFunction: """Test suite for the request_api_key helper function.""" diff --git a/cloudsmith_cli/cli/tests/commands/test_upstream.py b/cloudsmith_cli/cli/tests/commands/test_upstream.py index 42651b83..99dea9e0 100644 --- a/cloudsmith_cli/cli/tests/commands/test_upstream.py +++ b/cloudsmith_cli/cli/tests/commands/test_upstream.py @@ -7,6 +7,7 @@ @pytest.mark.usefixtures("set_api_key_env_var", "set_api_host_env_var") +@pytest.mark.integration @pytest.mark.parametrize("upstream_format", UPSTREAM_FORMATS) def test_upstream_commands( runner, organization, upstream_format, tmp_repository, tmp_path diff --git a/cloudsmith_cli/cli/tests/conftest.py b/cloudsmith_cli/cli/tests/conftest.py index 20254d20..6ba3fe94 100644 --- a/cloudsmith_cli/cli/tests/conftest.py +++ b/cloudsmith_cli/cli/tests/conftest.py @@ -67,12 +67,12 @@ def api_host(): @pytest.fixture() -def set_api_host_env_var(api_host): +def set_api_host_env_var(api_host, monkeypatch): """Set the CLOUDSMITH_API_HOST environment variable.""" - os.environ["CLOUDSMITH_API_HOST"] = api_host + monkeypatch.setenv("CLOUDSMITH_API_HOST", api_host) @pytest.fixture() -def set_api_key_env_var(api_key): +def set_api_key_env_var(api_key, monkeypatch): """Set the CLOUDSMITH_API_KEY environment variable.""" - os.environ["CLOUDSMITH_API_KEY"] = api_key + monkeypatch.setenv("CLOUDSMITH_API_KEY", api_key) diff --git a/cloudsmith_cli/core/tests/test_download.py b/cloudsmith_cli/core/tests/test_download.py index 8d3dfb7b..cf218a72 100644 --- a/cloudsmith_cli/core/tests/test_download.py +++ b/cloudsmith_cli/core/tests/test_download.py @@ -2,7 +2,6 @@ # pylint: disable=protected-access # Testing private functions is acceptable in tests -import tempfile import unittest from unittest.mock import Mock, patch @@ -324,7 +323,6 @@ class TestStreamDownload(unittest.TestCase): def setUp(self): self.session = Mock() - self.temp_dir = tempfile.mkdtemp() @patch("os.path.exists") def test_stream_download_file_exists_no_overwrite(self, mock_exists): diff --git a/cloudsmith_cli/core/tests/test_rest.py b/cloudsmith_cli/core/tests/test_rest.py index d2a6e6d8..af8ebac9 100644 --- a/cloudsmith_cli/core/tests/test_rest.py +++ b/cloudsmith_cli/core/tests/test_rest.py @@ -29,7 +29,7 @@ def test_implicit_retry_for_status_codes(self): # happened elsewhere. But just in case this test is ever run in isolation... initialise_api() - client = RestClient() + client = RestClient(backoff_factor=0) method = "GET" url = "https://test.site" diff --git a/pyproject.toml b/pyproject.toml index 36895dd7..1d2f3711 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -110,8 +110,11 @@ namespaces = false cloudsmith_cli = ["data/*", "templates/*"] [tool.pytest.ini_options] -addopts = "--cov=cloudsmith_cli" norecursedirs = ["bin", ".git", ".venv"] +testpaths = ["cloudsmith_cli"] +markers = [ + "integration: tests that call the live Cloudsmith service", +] [tool.coverage.run] branch = true From e1a46c279fd2626250ef99b72e2763bc52cdd985 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 25 Aug 2026 12:59:58 +0100 Subject: [PATCH 2/4] fix: patch time.sleep directly in retry tests The retry sleep moved from core.rest to core.session on master. Patch the stdlib time.sleep so the test does not depend on the module that holds the retry class. Co-Authored-By: Claude Fable 5 --- cloudsmith_cli/cli/tests/commands/test_credential_helper.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cloudsmith_cli/cli/tests/commands/test_credential_helper.py b/cloudsmith_cli/cli/tests/commands/test_credential_helper.py index 5af2594f..ae77e44c 100644 --- a/cloudsmith_cli/cli/tests/commands/test_credential_helper.py +++ b/cloudsmith_cli/cli/tests/commands/test_credential_helper.py @@ -255,7 +255,7 @@ def test_get_custom_domains_status_matrix( ): """get_custom_domains() caches or not based on HTTP status.""" if status == 500: - monkeypatch.setattr("cloudsmith_cli.core.rest.time.sleep", lambda _: None) + monkeypatch.setattr("time.sleep", lambda _: None) # redirect per-test (autouse fixture already set module-level path) monkeypatch.setattr( @@ -326,7 +326,7 @@ def test_get_custom_domains_strict_raises_on_failure( not render a typo'd org or an unreachable API as "no custom domains". """ if status == 500: - monkeypatch.setattr("cloudsmith_cli.core.rest.time.sleep", lambda _: None) + monkeypatch.setattr("time.sleep", lambda _: None) monkeypatch.setattr( "cloudsmith_cli.credential_helpers.custom_domains.get_default_config_path", From f43df9be447ef8f39d510b4583c8e5f901251c33 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 25 Aug 2026 13:07:39 +0100 Subject: [PATCH 3/4] chore: apply code-review fixes to the CI changes - Expose the Actions runtime so the buildx gha cache engages. - Gate the integration job on all five credentials; show it as skipped, not green, when they are absent. - Move the PR binary subset into binaries-matrix.json and fail on an empty matrix. - Add a paths filter to the push trigger in binaries.yml. - Merge the duplicate pytest steps and guard the artifact uploads. - Enable --strict-markers and document the integration marker. - Share the fake API host fixture; replace the conditional sleep patches with one autouse fixture. Co-Authored-By: Claude Fable 5 --- .github/scripts/binaries-matrix.json | 3 ++ .github/workflows/binaries.yml | 31 +++++++------ .github/workflows/image-build.yml | 2 + .github/workflows/test.yml | 45 ++++++++++--------- AGENTS.md | 5 ++- cloudsmith_cli/cli/tests/commands/conftest.py | 8 ++++ .../cli/tests/commands/test_check.py | 7 ++- .../tests/commands/test_credential_helper.py | 11 +++-- pyproject.toml | 1 + 9 files changed, 63 insertions(+), 50 deletions(-) diff --git a/.github/scripts/binaries-matrix.json b/.github/scripts/binaries-matrix.json index a9697fb2..b3560490 100644 --- a/.github/scripts/binaries-matrix.json +++ b/.github/scripts/binaries-matrix.json @@ -2,6 +2,7 @@ "include": [ { "name": "linux-x86_64-gnu", + "pr": true, "build_runner": "ubuntu-24.04", "mode": "glibc", "build_image": "almalinux:8@sha256:4a87d2615a770506e204c27d6248ac97f4df67f4e41e2e9c47c81f0ed0be98cb", @@ -42,6 +43,7 @@ }, { "name": "macos-arm64", + "pr": true, "build_runner": "macos-14", "mode": "native", "build_image": "", @@ -62,6 +64,7 @@ }, { "name": "windows-x86_64", + "pr": true, "build_runner": "windows-2025", "mode": "native", "build_image": "", diff --git a/.github/workflows/binaries.yml b/.github/workflows/binaries.yml index ba5ccbcb..c573ca9e 100644 --- a/.github/workflows/binaries.yml +++ b/.github/workflows/binaries.yml @@ -21,6 +21,15 @@ on: push: branches: - master + paths: + - cloudsmith_cli/** + - packaging/** + - Dockerfile + - pyproject.toml + - uv.lock + - .github/scripts/** + - .github/workflows/binaries.yml + - .github/workflows/release.yml pull_request: paths: - cloudsmith_cli/** @@ -83,22 +92,12 @@ jobs: env: EVENT_NAME: ${{ github.event_name }} run: | - if [ "${EVENT_NAME}" = "pull_request" ]; then - MATRIX=$( - jq -c '{ - include: [ - .include[] - | select( - .name == "linux-x86_64-gnu" - or .name == "macos-arm64" - or .name == "windows-x86_64" - ) - ] - }' .github/scripts/binaries-matrix.json - ) - else - MATRIX=$(jq -c . .github/scripts/binaries-matrix.json) - fi + MATRIX=$( + jq -c --arg event_name "${EVENT_NAME}" \ + '{include: [.include[] | select($event_name != "pull_request" or .pr)]}' \ + .github/scripts/binaries-matrix.json + ) + test "$(echo "${MATRIX}" | jq '.include | length')" -gt 0 echo "include=${MATRIX}" >> "$GITHUB_OUTPUT" build: diff --git a/.github/workflows/image-build.yml b/.github/workflows/image-build.yml index f5e8aa2c..992fc438 100644 --- a/.github/workflows/image-build.yml +++ b/.github/workflows/image-build.yml @@ -25,6 +25,8 @@ jobs: - uses: docker/setup-buildx-action@d7f5e7f509e45cec5c76c4d5afdd7de93d0b3df5 # v4.1.0 + - uses: crazy-max/ghaction-github-runtime@04d248b84655b509d8c44dc1d6f990c879747487 # v4.0.0 + - name: Build image run: | docker buildx build \ diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 28bab59e..d2d5283b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -43,20 +43,14 @@ jobs: - name: Install dependencies run: uv sync --locked --group dev --python ${{ matrix.python-version }} - - name: Run pytest with coverage - if: matrix.python-version == '3.12' - run: >- - uv run pytest - -m "not integration" - --cov=cloudsmith_cli - --junitxml=./reports/pytest.xml - - name: Run pytest - if: matrix.python-version != '3.12' - run: uv run pytest -m "not integration" --junitxml=./reports/pytest.xml + id: pytest + env: + COVERAGE_ARGS: ${{ matrix.python-version == '3.12' && '--cov=cloudsmith_cli' || '' }} + run: uv run pytest -m "not integration" ${COVERAGE_ARGS} --junitxml=./reports/pytest.xml - name: Upload test results - if: always() + if: ${{ !cancelled() && steps.pytest.outcome != 'skipped' }} uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: test-results-${{ matrix.python-version }} @@ -64,10 +58,12 @@ jobs: retention-days: 30 if-no-files-found: error - integration: - name: Live integration tests (Python 3.12) + integration-gate: + name: Check integration credentials runs-on: ubuntu-24.04 - timeout-minutes: 45 + timeout-minutes: 5 + outputs: + available: ${{ steps.credentials.outputs.available }} steps: - name: Check integration credentials id: credentials @@ -75,37 +71,42 @@ jobs: API_KEY: ${{ secrets.PYTEST_CLOUDSMITH_API_KEY }} API_HOST: ${{ vars.PYTEST_CLOUDSMITH_API_HOST }} ORGANIZATION: ${{ vars.PYTEST_CLOUDSMITH_ORGANIZATION }} + USER_NAME: ${{ secrets.PYTEST_CLOUDSMITH_USERNAME }} + PASSWORD: ${{ secrets.PYTEST_CLOUDSMITH_PASSWORD }} run: | - if [ -n "${API_KEY}" ] && [ -n "${API_HOST}" ] && [ -n "${ORGANIZATION}" ]; then + if [ -n "${API_KEY}" ] && [ -n "${API_HOST}" ] && [ -n "${ORGANIZATION}" ] && [ -n "${USER_NAME}" ] && [ -n "${PASSWORD}" ]; then echo "available=true" >> "$GITHUB_OUTPUT" else echo "available=false" >> "$GITHUB_OUTPUT" echo "Skipping live integration tests because credentials are unavailable." fi - - if: steps.credentials.outputs.available == 'true' - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + integration: + name: Live integration tests (Python 3.12) + needs: integration-gate + if: needs.integration-gate.outputs.available == 'true' + runs-on: ubuntu-24.04 + timeout-minutes: 45 + steps: + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: persist-credentials: false - name: Set up Python 3.12 - if: steps.credentials.outputs.available == 'true' uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 with: python-version: "3.12" - name: Set up uv - if: steps.credentials.outputs.available == 'true' uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39 # v8.2.0 with: enable-cache: true - name: Install dependencies - if: steps.credentials.outputs.available == 'true' run: uv sync --locked --group dev --python 3.12 - name: Run live integration tests - if: steps.credentials.outputs.available == 'true' + id: pytest env: PYTEST_CLOUDSMITH_API_KEY: ${{ secrets.PYTEST_CLOUDSMITH_API_KEY }} PYTEST_CLOUDSMITH_API_HOST: ${{ vars.PYTEST_CLOUDSMITH_API_HOST }} @@ -115,7 +116,7 @@ jobs: run: uv run pytest -m integration --junitxml=./reports/pytest.xml - name: Upload test results - if: always() && steps.credentials.outputs.available == 'true' + if: ${{ !cancelled() && steps.pytest.outcome != 'skipped' }} uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: test-results-integration-3.12 diff --git a/AGENTS.md b/AGENTS.md index 25d11afe..d27a3f3d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,7 +18,8 @@ Python `>=3.10` is required (CI tests 3.10–3.14). ## Common commands - Run the CLI locally: `cloudsmith ...` (console_script) or `python -m cloudsmith_cli ...`. -- Run tests: `pytest`. Run with coverage: `pytest --cov=cloudsmith_cli`. +- Run the unit tests: `pytest -m "not integration"`. Run with coverage: `pytest --cov=cloudsmith_cli`. +- Run the live-service tests: `pytest -m integration` (requires the `PYTEST_CLOUDSMITH_*` environment variables). Mark each test that calls the live Cloudsmith service with `@pytest.mark.integration`. - Run a single test: `pytest cloudsmith_cli/cli/tests/test_push.py::TestClass::test_name` or by node id / `-k `. - Lint/format (all run via pre-commit): `pre-commit run --all-files`. Individual tools: `black .`, `isort .`, `flake8 --config=.flake8`, `pylint --rcfile=.pylintrc `, `pyupgrade --py310-plus `. - Release: `bumpversion ` then `git push origin `. The `VERSION` symlink in repo root points at `cloudsmith_cli/data/VERSION`. @@ -59,7 +60,7 @@ Three auth paths feed `core.api.init.initialise_api`: ## Tests -Tests live alongside code: `cloudsmith_cli/cli/tests/` and `cloudsmith_cli/core/tests/`. The CLI tests use Click's `CliRunner`; API tests stub HTTP with `httpretty` and freeze time with `freezegun`. `bin/` and `.venv/` are excluded from pytest discovery (`norecursedirs` in `setup.cfg`). +Tests live alongside code: `cloudsmith_cli/cli/tests/` and `cloudsmith_cli/core/tests/`. The CLI tests use Click's `CliRunner`; API tests stub HTTP with `httpretty` and freeze time with `freezegun`. `bin/` and `.venv/` are excluded from pytest discovery (`norecursedirs` in `pyproject.toml`). Tests that call the live Cloudsmith service carry the `integration` marker. ## Style notes specific to this repo diff --git a/cloudsmith_cli/cli/tests/commands/conftest.py b/cloudsmith_cli/cli/tests/commands/conftest.py index 0bdeb58d..85317d16 100644 --- a/cloudsmith_cli/cli/tests/commands/conftest.py +++ b/cloudsmith_cli/cli/tests/commands/conftest.py @@ -1,5 +1,13 @@ import pytest +FAKE_API_HOST = "https://api.example.com" + + +@pytest.fixture() +def set_fake_api_host_env_var(monkeypatch): + """Set CLOUDSMITH_API_HOST to a host that no test can reach.""" + monkeypatch.setenv("CLOUDSMITH_API_HOST", FAKE_API_HOST) + class MockToken: """Mock Token object with the properties needed for testing.""" diff --git a/cloudsmith_cli/cli/tests/commands/test_check.py b/cloudsmith_cli/cli/tests/commands/test_check.py index 0302b5c8..9972e6d1 100644 --- a/cloudsmith_cli/cli/tests/commands/test_check.py +++ b/cloudsmith_cli/cli/tests/commands/test_check.py @@ -4,8 +4,7 @@ from ....cli.commands.check import check from ....cli.tests.utils import random_str - -FAKE_API_HOST = "https://api.example.com" +from .conftest import FAKE_API_HOST class TestCheckServiceCommand: @@ -17,11 +16,11 @@ class TestCheckServiceCommand: ("1.0.0", "2.0.0"), ], ) + @pytest.mark.usefixtures("set_fake_api_host_env_var") def test_check_service_command_output( - self, runner, monkeypatch, service_version, api_binding_version + self, runner, service_version, api_binding_version ): """Unit test the command output given different combinations of service/binding version.""" - monkeypatch.setenv("CLOUDSMITH_API_HOST", FAKE_API_HOST) service_status = random_str() with ( diff --git a/cloudsmith_cli/cli/tests/commands/test_credential_helper.py b/cloudsmith_cli/cli/tests/commands/test_credential_helper.py index ae77e44c..839f39ce 100644 --- a/cloudsmith_cli/cli/tests/commands/test_credential_helper.py +++ b/cloudsmith_cli/cli/tests/commands/test_credential_helper.py @@ -234,6 +234,11 @@ def _cache_dir(tmp_path, monkeypatch): ) +@pytest.fixture(autouse=True) +def _no_retry_sleep(monkeypatch): + monkeypatch.setattr("time.sleep", lambda _: None) + + @pytest.mark.parametrize( "status,expect_domains,expect_cached", [ @@ -254,9 +259,6 @@ def test_get_custom_domains_status_matrix( tmp_path, monkeypatch, status, expect_domains, expect_cached ): """get_custom_domains() caches or not based on HTTP status.""" - if status == 500: - monkeypatch.setattr("time.sleep", lambda _: None) - # redirect per-test (autouse fixture already set module-level path) monkeypatch.setattr( "cloudsmith_cli.credential_helpers.custom_domains.get_default_config_path", @@ -325,9 +327,6 @@ def test_get_custom_domains_strict_raises_on_failure( Consumers presenting results to a user (`cloudsmith domains list`) must not render a typo'd org or an unreachable API as "no custom domains". """ - if status == 500: - monkeypatch.setattr("time.sleep", lambda _: None) - monkeypatch.setattr( "cloudsmith_cli.credential_helpers.custom_domains.get_default_config_path", lambda: str(tmp_path), diff --git a/pyproject.toml b/pyproject.toml index 1d2f3711..748aafd2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -110,6 +110,7 @@ namespaces = false cloudsmith_cli = ["data/*", "templates/*"] [tool.pytest.ini_options] +addopts = "--strict-markers" norecursedirs = ["bin", ".git", ".venv"] testpaths = ["cloudsmith_cli"] markers = [ From 43e4575dc80132163302ac4168b5dc9b9d9acb59 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 25 Aug 2026 13:40:28 +0100 Subject: [PATCH 4/4] fix: run live tests with the original credential gate Gate the integration job on the three values it requires. The login tests skip themselves when the username and password are absent. The five-value gate stopped all live tests instead. Also drop the pr key from the binaries matrix so it does not appear in job names. Co-Authored-By: Claude Fable 5 --- .github/workflows/binaries.yml | 2 +- .github/workflows/test.yml | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/binaries.yml b/.github/workflows/binaries.yml index c573ca9e..c4b98793 100644 --- a/.github/workflows/binaries.yml +++ b/.github/workflows/binaries.yml @@ -94,7 +94,7 @@ jobs: run: | MATRIX=$( jq -c --arg event_name "${EVENT_NAME}" \ - '{include: [.include[] | select($event_name != "pull_request" or .pr)]}' \ + '{include: [.include[] | select($event_name != "pull_request" or .pr) | del(.pr)]}' \ .github/scripts/binaries-matrix.json ) test "$(echo "${MATRIX}" | jq '.include | length')" -gt 0 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d2d5283b..3bb6cd0b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -71,10 +71,8 @@ jobs: API_KEY: ${{ secrets.PYTEST_CLOUDSMITH_API_KEY }} API_HOST: ${{ vars.PYTEST_CLOUDSMITH_API_HOST }} ORGANIZATION: ${{ vars.PYTEST_CLOUDSMITH_ORGANIZATION }} - USER_NAME: ${{ secrets.PYTEST_CLOUDSMITH_USERNAME }} - PASSWORD: ${{ secrets.PYTEST_CLOUDSMITH_PASSWORD }} run: | - if [ -n "${API_KEY}" ] && [ -n "${API_HOST}" ] && [ -n "${ORGANIZATION}" ] && [ -n "${USER_NAME}" ] && [ -n "${PASSWORD}" ]; then + if [ -n "${API_KEY}" ] && [ -n "${API_HOST}" ] && [ -n "${ORGANIZATION}" ]; then echo "available=true" >> "$GITHUB_OUTPUT" else echo "available=false" >> "$GITHUB_OUTPUT"