Select affected Cloud integration suites - #413
Conversation
|
bugbot review |
There was a problem hiding this comment.
Pull request overview
Introduces an “affected-suites” planner for the Cloud Integration GitHub Actions workflow so labeled PR runs execute only the relevant live ClickHouse Cloud integration suites based on the PR’s immediate base-to-head diff (failing closed to all suites when classification is unsafe).
Changes:
- Added
scripts/classify-cloud-integration.pyand a comprehensive unit test suite to classify changed paths (including add/modify/delete/rename/copy) into affected live suites, failing closed to “all” on unknowns/malformed diffs. - Refactored
.github/workflows/cloud-integration.ymlinto a secretless planning job that selects suites + tested SHA, and a secret-bearing job that runs only the selected suites (or skips entirely when “none”). - Updated
crates/clickhouse-cloud-api/README.mdand the Cloud API test workflow trigger paths to document and validate the new classifier behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
scripts/tests/test_classify_cloud_integration.py |
Adds table-driven tests and inventory enforcement for source/test path→suite mappings and diff parsing. |
scripts/classify-cloud-integration.py |
Implements suite selection from git diff --name-status -z with rename/copy support and fail-closed behavior. |
crates/clickhouse-cloud-api/README.md |
Documents label-based affected-suite selection and when full scope=all is still needed. |
.github/workflows/test-cloud-api.yml |
Ensures changes to the new classifier script trigger the Cloud API library CI workflow. |
.github/workflows/cloud-integration.yml |
Adds a planning job that selects suites/test SHA and gates the secret-bearing integration job accordingly. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| 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 | ||
| ;; |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| "--find-copies-harder", | ||
| base_sha, | ||
| head_sha, | ||
| "--", |
There was a problem hiding this comment.
Stale base over-selects suites
Medium Severity
select_revisions runs a two-dot git diff of pull_request.base.sha against head. When the base tip has moved past the branch point, that diff includes unrelated base-branch paths, so suite selection no longer matches the PR’s own changes. No-suite PRs can enter the environment job, and new unmapped API paths on the base can fail closed to all suites.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a8b3c71. Configure here.
a8b3c71 to
1a4fd7d
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1a4fd7d. Configure here.
| else | ||
| echo "::warning::Classifier command failed; selecting all suites" | ||
| selected="$all_suites" | ||
| fi |
There was a problem hiding this comment.
Suite selection trusts PR classifier
Medium Severity
The plan job runs classify-cloud-integration.py from the PR head to decide which live suites run, including whether to skip the environment job entirely. ALWAYS_ALL_PATHS is meant to force all suites when that script changes, but that rule lives inside the same PR-controlled script, so a head revision that still emits a valid canonical string such as none can skip cloud integration while the workflow stays green.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 1a4fd7d. Configure here.
Closes #406
Summary
Verification
Rebased onto main after the API and CLI modularization stack merged.