From 1a4fd7d023b9e75b5ab11d902bbced9f376e340d Mon Sep 17 00:00:00 2001 From: sdairs Date: Thu, 13 Aug 2026 19:11:45 +0100 Subject: [PATCH 1/2] Select affected Cloud integration suites --- .github/workflows/cloud-integration.yml | 156 +++++++- .github/workflows/test-cloud-api.yml | 1 + crates/clickhouse-cloud-api/README.md | 2 +- scripts/classify-cloud-integration.py | 309 ++++++++++++++++ .../tests/test_classify_cloud_integration.py | 337 ++++++++++++++++++ 5 files changed, 786 insertions(+), 19 deletions(-) create mode 100644 scripts/classify-cloud-integration.py create mode 100644 scripts/tests/test_classify_cloud_integration.py diff --git a/.github/workflows/cloud-integration.yml b/.github/workflows/cloud-integration.yml index e3af712..1484b06 100644 --- a/.github/workflows/cloud-integration.yml +++ b/.github/workflows/cloud-integration.yml @@ -30,8 +30,8 @@ concurrency: cancel-in-progress: false jobs: - cloud-integration: - name: Cloud integration tests + plan: + name: Plan Cloud integration tests if: >- github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' || @@ -40,6 +40,127 @@ jobs: github.event.pull_request.head.repo.full_name == github.repository && github.event.pull_request.user.login != 'dependabot[bot]') runs-on: ubuntu-latest + outputs: + service: ${{ steps.selection.outputs.service }} + postgres: ${{ steps.selection.outputs.postgres }} + organization: ${{ steps.selection.outputs.organization }} + clickpipes: ${{ steps.selection.outputs.clickpipes }} + test_sha: ${{ steps.selection.outputs.test_sha }} + requested_scope: ${{ steps.selection.outputs.requested_scope }} + selected_suites: ${{ steps.selection.outputs.selected_suites }} + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }} + fetch-depth: 0 + persist-credentials: false + + - name: Select suites and log plan + id: selection + env: + EVENT_NAME: ${{ github.event_name }} + EVENT_REF: ${{ github.ref }} + EVENT_SHA: ${{ github.sha }} + PR_BASE_SHA: ${{ github.event.pull_request.base.sha || 'n/a' }} + PR_HEAD_SHA: ${{ github.event.pull_request.head.sha || 'n/a' }} + DISPATCH_SCOPE: ${{ inputs.scope || '' }} + run: | + set -u + all_suites="service,postgres,organization,clickpipes" + test_sha="$EVENT_SHA" + + case "$EVENT_NAME" in + pull_request) + test_sha="$PR_HEAD_SHA" + requested_scope="affected (PR immediate diff)" + if selected="$(python3 scripts/classify-cloud-integration.py "$PR_BASE_SHA" "$PR_HEAD_SHA")"; then + : + else + echo "::warning::Classifier command failed; selecting all suites" + selected="$all_suites" + fi + ;; + workflow_dispatch) + requested_scope="$DISPATCH_SCOPE" + if [[ "$DISPATCH_SCOPE" == "all" ]]; then + selected="$all_suites" + else + selected="$DISPATCH_SCOPE" + fi + ;; + schedule) + requested_scope="all (schedule)" + selected="$all_suites" + ;; + *) + echo "::warning::Unexpected event; selecting all suites" + requested_scope="all (fallback)" + selected="$all_suites" + ;; + esac + + service=false + postgres=false + organization=false + clickpipes=false + [[ ",$selected," == *",service,"* ]] && service=true + [[ ",$selected," == *",postgres,"* ]] && postgres=true + [[ ",$selected," == *",organization,"* ]] && organization=true + [[ ",$selected," == *",clickpipes,"* ]] && clickpipes=true + + canonical="" + [[ "$service" == true ]] && canonical="service" + [[ "$postgres" == true ]] && canonical="${canonical:+$canonical,}postgres" + [[ "$organization" == true ]] && canonical="${canonical:+$canonical,}organization" + [[ "$clickpipes" == true ]] && canonical="${canonical:+$canonical,}clickpipes" + canonical="${canonical:-none}" + if [[ "$selected" != "$canonical" ]]; then + echo "::warning::Classifier emitted invalid suite output; selecting all suites" + selected="$all_suites" + service=true + postgres=true + organization=true + clickpipes=true + fi + + checked_out_sha="$(git rev-parse HEAD)" + echo "Event: $EVENT_NAME" + echo "Event SHA: $EVENT_SHA" + echo "PR base SHA: $PR_BASE_SHA" + echo "PR head SHA: $PR_HEAD_SHA" + echo "Selected SHA: $test_sha" + echo "Checked-out SHA: $checked_out_sha" + echo "Event ref: $EVENT_REF" + echo "Requested scope: $requested_scope" + echo "Selected suites: $selected" + + { + echo "service=$service" + echo "postgres=$postgres" + echo "organization=$organization" + echo "clickpipes=$clickpipes" + echo "test_sha=$test_sha" + echo "requested_scope=$requested_scope" + echo "selected_suites=$selected" + } >> "$GITHUB_OUTPUT" + + if [[ "$checked_out_sha" != "$test_sha" ]]; then + echo "::error::Checked-out SHA does not match selected test SHA" + exit 1 + fi + + cloud-integration: + name: Cloud integration tests + needs: plan + if: >- + ${{ + needs.plan.result == 'success' && + (needs.plan.outputs.service == 'true' || + needs.plan.outputs.postgres == 'true' || + needs.plan.outputs.organization == 'true' || + needs.plan.outputs.clickpipes == 'true') + }} + runs-on: ubuntu-latest environment: cloud-integration env: CLICKHOUSE_CLOUD_API_KEY: ${{ secrets.CLICKHOUSE_CLOUD_API_KEY }} @@ -61,25 +182,29 @@ jobs: steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: - ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }} + ref: ${{ needs.plan.outputs.test_sha }} persist-credentials: false - name: Log revisions env: + EVENT_NAME: ${{ github.event_name }} EVENT_SHA: ${{ github.sha }} PR_BASE_SHA: ${{ github.event.pull_request.base.sha || 'n/a' }} PR_HEAD_SHA: ${{ github.event.pull_request.head.sha || 'n/a' }} - SELECTED_REF: ${{ github.ref }} - SELECTED_SHA: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }} - REQUESTED_SCOPE: ${{ github.event_name == 'workflow_dispatch' && inputs.scope || 'all (non-dispatch)' }} + EVENT_REF: ${{ github.ref }} + SELECTED_SHA: ${{ needs.plan.outputs.test_sha }} + REQUESTED_SCOPE: ${{ needs.plan.outputs.requested_scope }} + SELECTED_SUITES: ${{ needs.plan.outputs.selected_suites }} run: | + echo "Event: $EVENT_NAME" echo "Event SHA: $EVENT_SHA" echo "PR base SHA: $PR_BASE_SHA" echo "PR head SHA: $PR_HEAD_SHA" - echo "Selected ref: $SELECTED_REF" echo "Selected SHA: $SELECTED_SHA" - echo "Requested scope: $REQUESTED_SCOPE" echo "Checked-out SHA: $(git rev-parse HEAD)" + echo "Event ref: $EVENT_REF" + echo "Requested scope: $REQUESTED_SCOPE" + echo "Selected suites: $SELECTED_SUITES" - name: Resolve test run label run: | @@ -101,29 +226,25 @@ jobs: - name: Run cloud integration suite if: >- ${{ !cancelled() && - (github.event_name != 'workflow_dispatch' || - inputs.scope == 'all' || inputs.scope == 'service') }} + needs.plan.outputs.service == 'true' }} run: cargo test -p clickhouse-cloud-api --test integration_test -- --ignored --nocapture - name: Run cloud Postgres integration suite if: >- ${{ !cancelled() && - (github.event_name != 'workflow_dispatch' || - inputs.scope == 'all' || inputs.scope == 'postgres') }} + needs.plan.outputs.postgres == 'true' }} run: cargo test -p clickhouse-cloud-api --test integration_postgres_test -- --ignored --nocapture - name: Run cloud Org integration suite if: >- ${{ !cancelled() && - (github.event_name != 'workflow_dispatch' || - inputs.scope == 'all' || inputs.scope == 'organization') }} + needs.plan.outputs.organization == 'true' }} run: cargo test -p clickhouse-cloud-api --test integration_org_test -- --ignored --nocapture - name: Run ClickPipe Postgres CDC integration test if: >- ${{ !cancelled() && - (github.event_name != 'workflow_dispatch' || - inputs.scope == 'all' || inputs.scope == 'clickpipes') }} + needs.plan.outputs.clickpipes == 'true' }} run: cargo test -p clickhouse-cloud-api --test clickpipe_postgres_cdc_test -- --ignored --nocapture # ClickPipe create smoke tests run against a long-lived ClickHouse Cloud @@ -134,7 +255,6 @@ jobs: - name: Run ClickPipe create smoke suite if: >- ${{ !cancelled() && - (github.event_name != 'workflow_dispatch' || - inputs.scope == 'all' || inputs.scope == 'clickpipes') && + needs.plan.outputs.clickpipes == 'true' && vars.CLICKHOUSE_CLOUD_TEST_CLICKPIPE_SERVICE_ID != '' }} run: cargo test -p clickhouse-cloud-api --test clickpipe_smoke_test -- --ignored --nocapture diff --git a/.github/workflows/test-cloud-api.yml b/.github/workflows/test-cloud-api.yml index 5abbda5..9e6ae10 100644 --- a/.github/workflows/test-cloud-api.yml +++ b/.github/workflows/test-cloud-api.yml @@ -7,6 +7,7 @@ on: - "crates/clickhouse-cloud-api/**" - "crates/clickhouse-openapi-analyzer/**" - "scripts/check-openapi-drift.py" + - "scripts/classify-cloud-integration.py" - "scripts/tests/**" - "Cargo.toml" - "Cargo.lock" diff --git a/crates/clickhouse-cloud-api/README.md b/crates/clickhouse-cloud-api/README.md index c22b3e8..64541d6 100644 --- a/crates/clickhouse-cloud-api/README.md +++ b/crates/clickhouse-cloud-api/README.md @@ -86,7 +86,7 @@ cargo test --test clickpipe_smoke_test -- --ignored --nocapture # creat All require `CLICKHOUSE_CLOUD_API_KEY`, `CLICKHOUSE_CLOUD_API_SECRET`, `CLICKHOUSE_CLOUD_TEST_ORG_ID`, `CLICKHOUSE_CLOUD_TEST_PROVIDER`, and `CLICKHOUSE_CLOUD_TEST_REGION` in the environment, and are wired into the scheduled `Cloud Integration` GitHub Actions workflow. The ClickPipes E2E suites additionally need AWS credentials and an `eu-west-1` region quota; `clickpipe_smoke_test` reads a pre-provisioned service ID from `CLICKHOUSE_CLOUD_TEST_CLICKPIPE_SERVICE_ID`. -Manual `Cloud Integration` dispatches accept `scope=all`, `service`, `postgres`, `organization`, or `clickpipes`. The focused scopes run only their corresponding suite; `clickpipes` runs Postgres CDC plus the fixture-gated smoke test, while `all` runs all four mandatory suites plus that optional smoke test. For full-stack validation, manually run `scope=all` against the top stack branch. +Applying `run-cloud-integration` to an eligible PR selects suites from that PR's immediate base-to-head diff; known changes without a live suite finish without entering the environment-bearing job, while unknown API source or test paths select all suites. Manual `Cloud Integration` dispatches accept `scope=all`, `service`, `postgres`, `organization`, or `clickpipes`. The focused scopes run only their corresponding suite; `clickpipes` runs Postgres CDC plus the fixture-gated smoke test, while `all` runs all four mandatory suites plus that optional smoke test. Because immediate stacked-PR diffs exclude inherited changes, manually run `scope=all` against the top stack branch for full-stack validation. `spec_coverage_test` sends the checked-in sources and snapshot through the private `clickhouse-openapi-analyzer` crate. The analyzer recursively traverses diff --git a/scripts/classify-cloud-integration.py b/scripts/classify-cloud-integration.py new file mode 100644 index 0000000..628c475 --- /dev/null +++ b/scripts/classify-cloud-integration.py @@ -0,0 +1,309 @@ +#!/usr/bin/env python3 +"""Select live Cloud integration suites for an exact Git revision diff.""" + +import argparse +import subprocess +import sys +from dataclasses import dataclass +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +SUITES = ("service", "postgres", "organization", "clickpipes") +ALL_SUITES = frozenset(SUITES) +NO_SUITES = frozenset() + +# Retain mappings when files are deleted or renamed so old diff paths stay classified. +SOURCE_PATH_SUITES = { + "crates/clickhouse-cloud-api/src/client.rs": ALL_SUITES, + "crates/clickhouse-cloud-api/src/client/activity.rs": frozenset({"organization"}), + "crates/clickhouse-cloud-api/src/client/api_keys.rs": frozenset( + {"service", "organization"} + ), + "crates/clickhouse-cloud-api/src/client/backups.rs": NO_SUITES, + "crates/clickhouse-cloud-api/src/client/clickpipes.rs": frozenset( + {"clickpipes"} + ), + "crates/clickhouse-cloud-api/src/client/clickstack.rs": NO_SUITES, + "crates/clickhouse-cloud-api/src/client/organizations.rs": frozenset( + {"service", "organization"} + ), + "crates/clickhouse-cloud-api/src/client/postgres.rs": frozenset( + {"postgres", "clickpipes"} + ), + "crates/clickhouse-cloud-api/src/client/services.rs": frozenset( + {"service", "clickpipes"} + ), + "crates/clickhouse-cloud-api/src/client/udfs.rs": NO_SUITES, + "crates/clickhouse-cloud-api/src/convert.rs": ALL_SUITES, + "crates/clickhouse-cloud-api/src/convert/clickstack.rs": NO_SUITES, + "crates/clickhouse-cloud-api/src/convert/postgres.rs": frozenset( + {"postgres", "clickpipes"} + ), + "crates/clickhouse-cloud-api/src/convert/service.rs": frozenset( + {"service", "clickpipes"} + ), + "crates/clickhouse-cloud-api/src/convert/shared.rs": ALL_SUITES, + "crates/clickhouse-cloud-api/src/error.rs": ALL_SUITES, + "crates/clickhouse-cloud-api/src/lib.rs": ALL_SUITES, + "crates/clickhouse-cloud-api/src/meta.rs": NO_SUITES, + "crates/clickhouse-cloud-api/src/models.rs": ALL_SUITES, + "crates/clickhouse-cloud-api/src/models/activity.rs": frozenset( + {"organization"} + ), + "crates/clickhouse-cloud-api/src/models/api_keys.rs": frozenset( + {"service", "organization"} + ), + "crates/clickhouse-cloud-api/src/models/backups.rs": NO_SUITES, + "crates/clickhouse-cloud-api/src/models/byoc.rs": frozenset( + {"service", "organization"} + ), + "crates/clickhouse-cloud-api/src/models/clickpipes.rs": frozenset( + {"clickpipes"} + ), + "crates/clickhouse-cloud-api/src/models/clickstack.rs": NO_SUITES, + "crates/clickhouse-cloud-api/src/models/clickstack_enums.rs": NO_SUITES, + "crates/clickhouse-cloud-api/src/models/invitations.rs": frozenset( + {"organization"} + ), + "crates/clickhouse-cloud-api/src/models/members.rs": frozenset( + {"organization"} + ), + "crates/clickhouse-cloud-api/src/models/organization_private_endpoints.rs": frozenset( + {"service", "organization"} + ), + "crates/clickhouse-cloud-api/src/models/organizations.rs": frozenset( + {"service", "organization"} + ), + "crates/clickhouse-cloud-api/src/models/postgres.rs": frozenset( + {"postgres", "clickpipes"} + ), + "crates/clickhouse-cloud-api/src/models/quotas.rs": NO_SUITES, + "crates/clickhouse-cloud-api/src/models/rbac.rs": frozenset({"organization"}), + "crates/clickhouse-cloud-api/src/models/scim.rs": NO_SUITES, + "crates/clickhouse-cloud-api/src/models/services.rs": frozenset( + {"service", "clickpipes"} + ), + "crates/clickhouse-cloud-api/src/models/shared.rs": ALL_SUITES, + "crates/clickhouse-cloud-api/src/models/udfs.rs": NO_SUITES, + "crates/clickhouse-cloud-api/src/serde_helpers.rs": ALL_SUITES, +} + +TEST_PATH_SUITES = { + "crates/clickhouse-cloud-api/tests/clickpipes/driver.rs": NO_SUITES, + "crates/clickhouse-cloud-api/tests/clickpipes/e2e_test.rs": NO_SUITES, + "crates/clickhouse-cloud-api/tests/clickpipes/kafka_test.rs": NO_SUITES, + "crates/clickhouse-cloud-api/tests/clickpipes/kinesis_test.rs": NO_SUITES, + "crates/clickhouse-cloud-api/tests/clickpipes/mongo_test.rs": NO_SUITES, + "crates/clickhouse-cloud-api/tests/clickpipes/mysql_test.rs": NO_SUITES, + "crates/clickhouse-cloud-api/tests/clickpipes/postgres_cdc_test.rs": frozenset( + {"clickpipes"} + ), + "crates/clickhouse-cloud-api/tests/clickpipes/postgres_ec2_test.rs": NO_SUITES, + "crates/clickhouse-cloud-api/tests/clickpipes/s3_test.rs": NO_SUITES, + "crates/clickhouse-cloud-api/tests/clickpipes/smoke_test.rs": frozenset( + {"clickpipes"} + ), + "crates/clickhouse-cloud-api/tests/clickpipes/stages/kafka.rs": NO_SUITES, + "crates/clickhouse-cloud-api/tests/clickpipes/stages/kinesis.rs": NO_SUITES, + "crates/clickhouse-cloud-api/tests/clickpipes/stages/mod.rs": NO_SUITES, + "crates/clickhouse-cloud-api/tests/clickpipes/stages/mongo.rs": NO_SUITES, + "crates/clickhouse-cloud-api/tests/clickpipes/stages/mongo_user_data.sh.template": NO_SUITES, + "crates/clickhouse-cloud-api/tests/clickpipes/stages/mysql.rs": NO_SUITES, + "crates/clickhouse-cloud-api/tests/clickpipes/stages/mysql_user_data.sh.template": NO_SUITES, + "crates/clickhouse-cloud-api/tests/clickpipes/stages/postgres.rs": NO_SUITES, + "crates/clickhouse-cloud-api/tests/clickpipes/stages/postgres_user_data.sh.template": NO_SUITES, + "crates/clickhouse-cloud-api/tests/clickpipes/stages/redpanda_user_data_mtls.sh.template": NO_SUITES, + "crates/clickhouse-cloud-api/tests/clickpipes/stages/redpanda_user_data_scram_tls.sh.template": NO_SUITES, + "crates/clickhouse-cloud-api/tests/clickpipes/stages/s3.rs": NO_SUITES, + "crates/clickhouse-cloud-api/tests/clickpipes/support.rs": frozenset( + {"clickpipes"} + ), + "crates/clickhouse-cloud-api/tests/client_test.rs": NO_SUITES, + "crates/clickhouse-cloud-api/tests/common/mod.rs": ALL_SUITES, + "crates/clickhouse-cloud-api/tests/common/support.rs": ALL_SUITES, + "crates/clickhouse-cloud-api/tests/integration_org_test.rs": frozenset( + {"organization"} + ), + "crates/clickhouse-cloud-api/tests/integration_postgres_test.rs": frozenset( + {"postgres"} + ), + "crates/clickhouse-cloud-api/tests/integration_test.rs": frozenset({"service"}), + "crates/clickhouse-cloud-api/tests/model_facade_test.rs": NO_SUITES, + "crates/clickhouse-cloud-api/tests/models_test.rs": NO_SUITES, + "crates/clickhouse-cloud-api/tests/run_query_test.rs": NO_SUITES, + "crates/clickhouse-cloud-api/tests/spec_coverage_test.rs": NO_SUITES, +} + +API_SOURCE_PREFIX = "crates/clickhouse-cloud-api/src/" +API_TEST_PREFIX = "crates/clickhouse-cloud-api/tests/" +API_CRATE_PREFIX = "crates/clickhouse-cloud-api/" +ALWAYS_ALL_PATHS = { + ".github/workflows/cloud-integration.yml", + "Cargo.lock", + "scripts/classify-cloud-integration.py", + "scripts/tests/test_classify_cloud_integration.py", +} +RUST_CONFIG_PATHS = { + ".cargo/config", + ".cargo/config.toml", + ".rustfmt.toml", + "clippy.toml", + "rust-toolchain", + "rust-toolchain.toml", + "rustfmt.toml", +} + + +class DiffFormatError(ValueError): + """The name-status stream cannot be classified safely.""" + + +@dataclass(frozen=True) +class Selection: + suites: tuple[str, ...] + failed_closed: bool = False + reason: str | None = None + + +def classify_path(path: str) -> frozenset[str] | None: + """Return mapped suites, an empty known mapping, or None for unknown API code.""" + if path in SOURCE_PATH_SUITES: + return SOURCE_PATH_SUITES[path] + if path in TEST_PATH_SUITES: + return TEST_PATH_SUITES[path] + if ( + path.startswith(API_SOURCE_PREFIX) + or path.startswith(API_TEST_PREFIX) + or (path.startswith(API_CRATE_PREFIX) and path.endswith(".rs")) + ): + return None + if ( + path in ALWAYS_ALL_PATHS + or path in RUST_CONFIG_PATHS + or path == "Cargo.toml" + or path.endswith("/Cargo.toml") + ): + return ALL_SUITES + return NO_SUITES + + +def parse_name_status(data: bytes) -> list[tuple[str, tuple[str, ...]]]: + """Parse `git diff --name-status -z`, rejecting anything unexpected.""" + if not data: + return [] + fields = data.split(b"\0") + if fields[-1] != b"": + raise DiffFormatError("name-status output is not NUL terminated") + fields.pop() + + records = [] + index = 0 + while index < len(fields): + try: + status = fields[index].decode("ascii") + except UnicodeDecodeError as error: + raise DiffFormatError("non-ASCII diff status") from error + index += 1 + + if status in {"A", "M", "D"}: + path_count = 1 + elif ( + len(status) > 1 + and status[0] in {"R", "C"} + and status[1:].isdigit() + and 0 <= int(status[1:]) <= 100 + ): + path_count = 2 + else: + raise DiffFormatError(f"unsupported diff status: {status!r}") + + if index + path_count > len(fields): + raise DiffFormatError(f"missing path for diff status {status!r}") + try: + paths = tuple( + field.decode("utf-8") for field in fields[index : index + path_count] + ) + except UnicodeDecodeError as error: + raise DiffFormatError("non-UTF-8 diff path") from error + if any(not path for path in paths): + raise DiffFormatError(f"empty path for diff status {status!r}") + records.append((status, paths)) + index += path_count + return records + + +def select_records(records: list[tuple[str, tuple[str, ...]]]) -> Selection: + selected = set() + for _status, paths in records: + for path in paths: + suites = classify_path(path) + if suites is None: + return Selection( + SUITES, + failed_closed=True, + reason=f"unknown Cloud API source/test path: {path}", + ) + unknown_suites = set(suites) - ALL_SUITES + if unknown_suites: + return Selection( + SUITES, + failed_closed=True, + reason=f"unknown suite mapping for {path}: {sorted(unknown_suites)}", + ) + selected.update(suites) + return Selection(tuple(suite for suite in SUITES if suite in selected)) + + +def select_name_status(data: bytes) -> Selection: + try: + return select_records(parse_name_status(data)) + except DiffFormatError as error: + return Selection(SUITES, failed_closed=True, reason=str(error)) + + +def select_revisions(base_sha: str, head_sha: str) -> Selection: + command = [ + "git", + "diff", + "--name-status", + "-z", + "--find-renames", + "--find-copies", + "--find-copies-harder", + base_sha, + head_sha, + "--", + ] + try: + result = subprocess.run(command, cwd=REPO_ROOT, capture_output=True) + except OSError as error: + return Selection(SUITES, failed_closed=True, reason=f"git diff failed: {error}") + if result.returncode != 0: + detail = result.stderr.decode("utf-8", errors="replace").strip() + return Selection( + SUITES, + failed_closed=True, + reason=f"git diff failed: {detail or f'exit {result.returncode}'}", + ) + return select_name_status(result.stdout) + + +def format_suites(suites: tuple[str, ...]) -> str: + return ",".join(suites) if suites else "none" + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("base_sha") + parser.add_argument("head_sha") + args = parser.parse_args() + + selection = select_revisions(args.base_sha, args.head_sha) + if selection.failed_closed: + print(f"warning: {selection.reason}; selecting all suites", file=sys.stderr) + print(format_suites(selection.suites)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/tests/test_classify_cloud_integration.py b/scripts/tests/test_classify_cloud_integration.py new file mode 100644 index 0000000..e2e80d5 --- /dev/null +++ b/scripts/tests/test_classify_cloud_integration.py @@ -0,0 +1,337 @@ +import importlib.util +import subprocess +import sys +import unittest +from pathlib import Path +from unittest import mock + +SCRIPT = Path(__file__).resolve().parents[1] / "classify-cloud-integration.py" +SPEC = importlib.util.spec_from_file_location("classify_cloud_integration", SCRIPT) +classifier = importlib.util.module_from_spec(SPEC) +sys.modules[SPEC.name] = classifier +SPEC.loader.exec_module(classifier) + + +class CloudIntegrationClassifierTests(unittest.TestCase): + def test_path_mapping(self): + cases = { + "crates/clickhouse-cloud-api/clickhouse_cloud_openapi.json": classifier.NO_SUITES, + "crates/clickhouse-cloud-api/README.md": classifier.NO_SUITES, + "crates/clickhousectl/src/cloud/client.rs": classifier.NO_SUITES, + "Cargo.lock": classifier.ALL_SUITES, + "crates/clickhousectl/Cargo.toml": classifier.ALL_SUITES, + ".cargo/config.toml": classifier.ALL_SUITES, + ".github/workflows/cloud-integration.yml": classifier.ALL_SUITES, + "scripts/classify-cloud-integration.py": classifier.ALL_SUITES, + "crates/clickhouse-cloud-api/src/new_domain.rs": None, + "crates/clickhouse-cloud-api/src/template.txt": None, + "crates/clickhouse-cloud-api/tests/new_live_test.rs": None, + "crates/clickhouse-cloud-api/build.rs": None, + } + for path, expected in cases.items(): + with self.subTest(path=path): + self.assertEqual(classifier.classify_path(path), expected) + + def test_every_cloud_api_source_file_has_an_explicit_mapping(self): + crate_root = classifier.REPO_ROOT / "crates" / "clickhouse-cloud-api" + actual = { + path.relative_to(classifier.REPO_ROOT).as_posix() + for path in crate_root.rglob("*.rs") + if path.relative_to(crate_root).parts[0] != "tests" + } + self.assertFalse(actual - set(classifier.SOURCE_PATH_SUITES)) + + def test_current_source_mapping_values(self): + expected = { + classifier.ALL_SUITES: { + "crates/clickhouse-cloud-api/src/client.rs", + "crates/clickhouse-cloud-api/src/convert.rs", + "crates/clickhouse-cloud-api/src/convert/shared.rs", + "crates/clickhouse-cloud-api/src/error.rs", + "crates/clickhouse-cloud-api/src/lib.rs", + "crates/clickhouse-cloud-api/src/models.rs", + "crates/clickhouse-cloud-api/src/models/shared.rs", + "crates/clickhouse-cloud-api/src/serde_helpers.rs", + }, + frozenset({"service", "clickpipes"}): { + "crates/clickhouse-cloud-api/src/client/services.rs", + "crates/clickhouse-cloud-api/src/convert/service.rs", + "crates/clickhouse-cloud-api/src/models/services.rs", + }, + frozenset({"postgres", "clickpipes"}): { + "crates/clickhouse-cloud-api/src/client/postgres.rs", + "crates/clickhouse-cloud-api/src/convert/postgres.rs", + "crates/clickhouse-cloud-api/src/models/postgres.rs", + }, + frozenset({"clickpipes"}): { + "crates/clickhouse-cloud-api/src/client/clickpipes.rs", + "crates/clickhouse-cloud-api/src/models/clickpipes.rs", + }, + frozenset({"service", "organization"}): { + "crates/clickhouse-cloud-api/src/client/api_keys.rs", + "crates/clickhouse-cloud-api/src/client/organizations.rs", + "crates/clickhouse-cloud-api/src/models/api_keys.rs", + "crates/clickhouse-cloud-api/src/models/byoc.rs", + "crates/clickhouse-cloud-api/src/models/organization_private_endpoints.rs", + "crates/clickhouse-cloud-api/src/models/organizations.rs", + }, + frozenset({"organization"}): { + "crates/clickhouse-cloud-api/src/client/activity.rs", + "crates/clickhouse-cloud-api/src/models/activity.rs", + "crates/clickhouse-cloud-api/src/models/invitations.rs", + "crates/clickhouse-cloud-api/src/models/members.rs", + "crates/clickhouse-cloud-api/src/models/rbac.rs", + }, + classifier.NO_SUITES: { + "crates/clickhouse-cloud-api/src/client/backups.rs", + "crates/clickhouse-cloud-api/src/client/clickstack.rs", + "crates/clickhouse-cloud-api/src/client/udfs.rs", + "crates/clickhouse-cloud-api/src/convert/clickstack.rs", + "crates/clickhouse-cloud-api/src/meta.rs", + "crates/clickhouse-cloud-api/src/models/backups.rs", + "crates/clickhouse-cloud-api/src/models/clickstack.rs", + "crates/clickhouse-cloud-api/src/models/clickstack_enums.rs", + "crates/clickhouse-cloud-api/src/models/quotas.rs", + "crates/clickhouse-cloud-api/src/models/scim.rs", + "crates/clickhouse-cloud-api/src/models/udfs.rs", + }, + } + source_root = ( + classifier.REPO_ROOT / "crates" / "clickhouse-cloud-api" / "src" + ) + actual = {} + for path in source_root.rglob("*.rs"): + relative = path.relative_to(classifier.REPO_ROOT).as_posix() + actual.setdefault(classifier.SOURCE_PATH_SUITES[relative], set()).add( + relative + ) + self.assertEqual(actual, expected) + + def test_every_current_cloud_api_test_file_has_an_explicit_mapping(self): + test_root = ( + classifier.REPO_ROOT / "crates" / "clickhouse-cloud-api" / "tests" + ) + actual = { + path.relative_to(classifier.REPO_ROOT).as_posix() + for path in test_root.rglob("*") + if path.is_file() + } + self.assertFalse(actual - set(classifier.TEST_PATH_SUITES)) + + def test_current_test_mapping_values(self): + expected = { + classifier.ALL_SUITES: { + "crates/clickhouse-cloud-api/tests/common/mod.rs", + "crates/clickhouse-cloud-api/tests/common/support.rs", + }, + frozenset({"service"}): { + "crates/clickhouse-cloud-api/tests/integration_test.rs" + }, + frozenset({"postgres"}): { + "crates/clickhouse-cloud-api/tests/integration_postgres_test.rs" + }, + frozenset({"organization"}): { + "crates/clickhouse-cloud-api/tests/integration_org_test.rs" + }, + frozenset({"clickpipes"}): { + "crates/clickhouse-cloud-api/tests/clickpipes/postgres_cdc_test.rs", + "crates/clickhouse-cloud-api/tests/clickpipes/smoke_test.rs", + "crates/clickhouse-cloud-api/tests/clickpipes/support.rs", + }, + classifier.NO_SUITES: { + "crates/clickhouse-cloud-api/tests/clickpipes/driver.rs", + "crates/clickhouse-cloud-api/tests/clickpipes/e2e_test.rs", + "crates/clickhouse-cloud-api/tests/clickpipes/kafka_test.rs", + "crates/clickhouse-cloud-api/tests/clickpipes/kinesis_test.rs", + "crates/clickhouse-cloud-api/tests/clickpipes/mongo_test.rs", + "crates/clickhouse-cloud-api/tests/clickpipes/mysql_test.rs", + "crates/clickhouse-cloud-api/tests/clickpipes/postgres_ec2_test.rs", + "crates/clickhouse-cloud-api/tests/clickpipes/s3_test.rs", + "crates/clickhouse-cloud-api/tests/clickpipes/stages/kafka.rs", + "crates/clickhouse-cloud-api/tests/clickpipes/stages/kinesis.rs", + "crates/clickhouse-cloud-api/tests/clickpipes/stages/mod.rs", + "crates/clickhouse-cloud-api/tests/clickpipes/stages/mongo.rs", + "crates/clickhouse-cloud-api/tests/clickpipes/stages/mongo_user_data.sh.template", + "crates/clickhouse-cloud-api/tests/clickpipes/stages/mysql.rs", + "crates/clickhouse-cloud-api/tests/clickpipes/stages/mysql_user_data.sh.template", + "crates/clickhouse-cloud-api/tests/clickpipes/stages/postgres.rs", + "crates/clickhouse-cloud-api/tests/clickpipes/stages/postgres_user_data.sh.template", + "crates/clickhouse-cloud-api/tests/clickpipes/stages/redpanda_user_data_mtls.sh.template", + "crates/clickhouse-cloud-api/tests/clickpipes/stages/redpanda_user_data_scram_tls.sh.template", + "crates/clickhouse-cloud-api/tests/clickpipes/stages/s3.rs", + "crates/clickhouse-cloud-api/tests/client_test.rs", + "crates/clickhouse-cloud-api/tests/model_facade_test.rs", + "crates/clickhouse-cloud-api/tests/models_test.rs", + "crates/clickhouse-cloud-api/tests/run_query_test.rs", + "crates/clickhouse-cloud-api/tests/spec_coverage_test.rs", + }, + } + test_root = ( + classifier.REPO_ROOT / "crates" / "clickhouse-cloud-api" / "tests" + ) + actual = {} + for path in test_root.rglob("*"): + if path.is_file(): + relative = path.relative_to(classifier.REPO_ROOT).as_posix() + actual.setdefault(classifier.TEST_PATH_SUITES[relative], set()).add( + relative + ) + self.assertEqual(actual, expected) + + def test_parses_add_modify_delete_rename_and_copy_records(self): + data = ( + b"A\0added\0M\0modified\0D\0deleted\0" + b"R100\0rename-old\0rename-new\0C75\0copy-old\0copy-new\0" + ) + self.assertEqual( + classifier.parse_name_status(data), + [ + ("A", ("added",)), + ("M", ("modified",)), + ("D", ("deleted",)), + ("R100", ("rename-old", "rename-new")), + ("C75", ("copy-old", "copy-new")), + ], + ) + + def test_rename_and_copy_union_both_paths_in_canonical_order(self): + cases = [ + ( + "R100", + "crates/clickhouse-cloud-api/src/client/services.rs", + "crates/clickhouse-cloud-api/src/models/activity.rs", + ("service", "organization", "clickpipes"), + ), + ( + "C100", + "crates/clickhouse-cloud-api/src/models/activity.rs", + "crates/clickhouse-cloud-api/src/models/postgres.rs", + ("postgres", "organization", "clickpipes"), + ), + ] + for status, old_path, new_path, expected in cases: + with self.subTest(status=status): + selection = classifier.select_records( + [(status, (old_path, new_path))] + ) + self.assertEqual(selection.suites, expected) + + def test_retained_known_none_deletion_and_rename_stay_none(self): + historical = ( + "crates/clickhouse-cloud-api/tests/integration_clickpipe_s3_test.rs" + ) + cases = [ + [("D", (historical,))], + [ + ( + "R100", + ( + historical, + "crates/clickhouse-cloud-api/tests/clickpipes/s3_test.rs", + ), + ) + ], + ] + with mock.patch.dict( + classifier.TEST_PATH_SUITES, + {historical: classifier.NO_SUITES}, + ): + for records in cases: + with self.subTest(records=records): + selection = classifier.select_records(records) + self.assertEqual(selection.suites, ()) + self.assertFalse(selection.failed_closed) + + def test_known_none_is_distinct_from_unknown(self): + known = classifier.select_records( + [("M", ("crates/clickhouse-cloud-api/src/meta.rs",))] + ) + unknown = classifier.select_records( + [("M", ("crates/clickhouse-cloud-api/src/new_domain.rs",))] + ) + self.assertEqual(known.suites, ()) + self.assertFalse(known.failed_closed) + self.assertEqual(unknown.suites, classifier.SUITES) + self.assertTrue(unknown.failed_closed) + + def test_unknown_suite_token_in_mapping_fails_closed(self): + path = "crates/clickhouse-cloud-api/src/meta.rs" + with mock.patch.dict( + classifier.SOURCE_PATH_SUITES, + {path: frozenset({"future-suite"})}, + ): + selection = classifier.select_records([("M", (path,))]) + self.assertEqual(selection.suites, classifier.SUITES) + self.assertTrue(selection.failed_closed) + self.assertIn("future-suite", selection.reason) + + def test_malformed_or_unsupported_records_fail_closed(self): + cases = { + "not NUL terminated": b"M\0path", + "rename missing destination": b"R100\0old\0", + "unsupported status": b"T\0path\0", + "malformed rename score": b"Rbad\0old\0new\0", + "empty path": b"A\0\0", + "non-UTF-8 path": b"M\0\xff\0", + } + for name, data in cases.items(): + with self.subTest(name=name): + selection = classifier.select_name_status(data) + self.assertEqual(selection.suites, classifier.SUITES) + self.assertTrue(selection.failed_closed) + + @mock.patch.object(classifier.subprocess, "run") + def test_diffs_the_exact_revisions_with_rename_and_copy_detection(self, run): + run.return_value = subprocess.CompletedProcess( + args=[], + returncode=0, + stdout=b"M\0crates/clickhouse-cloud-api/src/meta.rs\0", + stderr=b"", + ) + selection = classifier.select_revisions("base-sha", "head-sha") + self.assertEqual(selection.suites, ()) + run.assert_called_once_with( + [ + "git", + "diff", + "--name-status", + "-z", + "--find-renames", + "--find-copies", + "--find-copies-harder", + "base-sha", + "head-sha", + "--", + ], + cwd=classifier.REPO_ROOT, + capture_output=True, + ) + + def test_git_failures_fail_closed(self): + cases = [ + OSError("git unavailable"), + subprocess.CompletedProcess( + args=[], returncode=128, stdout=b"", stderr=b"bad revision" + ), + ] + for result in cases: + with self.subTest(result=result): + with mock.patch.object(classifier.subprocess, "run") as run: + if isinstance(result, Exception): + run.side_effect = result + else: + run.return_value = result + selection = classifier.select_revisions("base", "head") + self.assertEqual(selection.suites, classifier.SUITES) + self.assertTrue(selection.failed_closed) + + def test_formats_none_and_canonical_suites(self): + self.assertEqual(classifier.format_suites(()), "none") + self.assertEqual( + classifier.format_suites(classifier.SUITES), + "service,postgres,organization,clickpipes", + ) + + +if __name__ == "__main__": + unittest.main() From 5df2c1969b5c03eafc98b06dadf2f07002edad88 Mon Sep 17 00:00:00 2001 From: sdairs Date: Mon, 17 Aug 2026 12:11:07 +0100 Subject: [PATCH 2/2] Fail closed when selecting Cloud integration suites --- .github/workflows/cloud-integration.yml | 21 +++++++-- crates/clickhouse-cloud-api/README.md | 2 +- scripts/classify-cloud-integration.py | 3 +- .../tests/test_classify_cloud_integration.py | 45 +++++++++++++++++-- 4 files changed, 61 insertions(+), 10 deletions(-) diff --git a/.github/workflows/cloud-integration.yml b/.github/workflows/cloud-integration.yml index 1484b06..36db509 100644 --- a/.github/workflows/cloud-integration.yml +++ b/.github/workflows/cloud-integration.yml @@ -72,11 +72,24 @@ jobs: case "$EVENT_NAME" in pull_request) test_sha="$PR_HEAD_SHA" - requested_scope="affected (PR immediate diff)" - if selected="$(python3 scripts/classify-cloud-integration.py "$PR_BASE_SHA" "$PR_HEAD_SHA")"; then - : + requested_scope="affected (PR merge-base diff)" + if git diff --quiet "$PR_BASE_SHA...$PR_HEAD_SHA" -- \ + .github/workflows/cloud-integration.yml \ + scripts/classify-cloud-integration.py \ + scripts/tests/test_classify_cloud_integration.py; then + if selected="$(python3 scripts/classify-cloud-integration.py "$PR_BASE_SHA" "$PR_HEAD_SHA")"; then + : + else + echo "::warning::Classifier command failed; selecting all suites" + selected="$all_suites" + fi else - echo "::warning::Classifier command failed; selecting all suites" + guard_status=$? + if [[ "$guard_status" -eq 1 ]]; then + echo "::warning::Cloud integration selection logic changed; selecting all suites" + else + echo "::warning::Selection guard diff failed; selecting all suites" + fi selected="$all_suites" fi ;; diff --git a/crates/clickhouse-cloud-api/README.md b/crates/clickhouse-cloud-api/README.md index 64541d6..ba8ce1a 100644 --- a/crates/clickhouse-cloud-api/README.md +++ b/crates/clickhouse-cloud-api/README.md @@ -86,7 +86,7 @@ cargo test --test clickpipe_smoke_test -- --ignored --nocapture # creat All require `CLICKHOUSE_CLOUD_API_KEY`, `CLICKHOUSE_CLOUD_API_SECRET`, `CLICKHOUSE_CLOUD_TEST_ORG_ID`, `CLICKHOUSE_CLOUD_TEST_PROVIDER`, and `CLICKHOUSE_CLOUD_TEST_REGION` in the environment, and are wired into the scheduled `Cloud Integration` GitHub Actions workflow. The ClickPipes E2E suites additionally need AWS credentials and an `eu-west-1` region quota; `clickpipe_smoke_test` reads a pre-provisioned service ID from `CLICKHOUSE_CLOUD_TEST_CLICKPIPE_SERVICE_ID`. -Applying `run-cloud-integration` to an eligible PR selects suites from that PR's immediate base-to-head diff; known changes without a live suite finish without entering the environment-bearing job, while unknown API source or test paths select all suites. Manual `Cloud Integration` dispatches accept `scope=all`, `service`, `postgres`, `organization`, or `clickpipes`. The focused scopes run only their corresponding suite; `clickpipes` runs Postgres CDC plus the fixture-gated smoke test, while `all` runs all four mandatory suites plus that optional smoke test. Because immediate stacked-PR diffs exclude inherited changes, manually run `scope=all` against the top stack branch for full-stack validation. +Applying `run-cloud-integration` to an eligible PR selects suites from that PR's merge-base-to-head diff; known changes without a live suite finish without entering the environment-bearing job, while unknown API source or test paths select all suites. Changes to the classifier, its tests, or the workflow also select all suites through an independent workflow guard. Manual `Cloud Integration` dispatches accept `scope=all`, `service`, `postgres`, `organization`, or `clickpipes`. The focused scopes run only their corresponding suite; `clickpipes` runs Postgres CDC plus the fixture-gated smoke test, while `all` runs all four mandatory suites plus that optional smoke test. Because stacked-PR diffs exclude inherited changes, manually run `scope=all` against the top stack branch for full-stack validation. `spec_coverage_test` sends the checked-in sources and snapshot through the private `clickhouse-openapi-analyzer` crate. The analyzer recursively traverses diff --git a/scripts/classify-cloud-integration.py b/scripts/classify-cloud-integration.py index 628c475..fb2a1cc 100644 --- a/scripts/classify-cloud-integration.py +++ b/scripts/classify-cloud-integration.py @@ -270,8 +270,7 @@ def select_revisions(base_sha: str, head_sha: str) -> Selection: "--find-renames", "--find-copies", "--find-copies-harder", - base_sha, - head_sha, + f"{base_sha}...{head_sha}", "--", ] try: diff --git a/scripts/tests/test_classify_cloud_integration.py b/scripts/tests/test_classify_cloud_integration.py index e2e80d5..e88050a 100644 --- a/scripts/tests/test_classify_cloud_integration.py +++ b/scripts/tests/test_classify_cloud_integration.py @@ -1,6 +1,7 @@ import importlib.util import subprocess import sys +import tempfile import unittest from pathlib import Path from unittest import mock @@ -281,7 +282,7 @@ def test_malformed_or_unsupported_records_fail_closed(self): self.assertTrue(selection.failed_closed) @mock.patch.object(classifier.subprocess, "run") - def test_diffs_the_exact_revisions_with_rename_and_copy_detection(self, run): + def test_diffs_from_merge_base_with_rename_and_copy_detection(self, run): run.return_value = subprocess.CompletedProcess( args=[], returncode=0, @@ -299,14 +300,52 @@ def test_diffs_the_exact_revisions_with_rename_and_copy_detection(self, run): "--find-renames", "--find-copies", "--find-copies-harder", - "base-sha", - "head-sha", + "base-sha...head-sha", "--", ], cwd=classifier.REPO_ROOT, capture_output=True, ) + def test_stale_base_changes_are_excluded(self): + with tempfile.TemporaryDirectory() as temp_dir: + repo = Path(temp_dir) + + def git(*args): + return subprocess.run( + ["git", *args], + cwd=repo, + check=True, + capture_output=True, + text=True, + ).stdout.strip() + + git("init", "-b", "main") + git("config", "user.name", "Cloud Integration Test") + git("config", "user.email", "cloud-integration-test@example.com") + git("commit", "--allow-empty", "-m", "initial") + + git("checkout", "-b", "feature") + source_root = repo / "crates" / "clickhouse-cloud-api" / "src" + source_root.mkdir(parents=True) + (source_root / "meta.rs").write_text("feature\n") + git("add", ".") + git("commit", "-m", "feature") + head_sha = git("rev-parse", "HEAD") + + git("checkout", "main") + source_root.mkdir(parents=True) + (source_root / "services.rs").write_text("base advance\n") + git("add", ".") + git("commit", "-m", "advance base") + base_sha = git("rev-parse", "HEAD") + + with mock.patch.object(classifier, "REPO_ROOT", repo): + selection = classifier.select_revisions(base_sha, head_sha) + + self.assertEqual(selection.suites, ()) + self.assertFalse(selection.failed_closed) + def test_git_failures_fail_closed(self): cases = [ OSError("git unavailable"),