ci: run integration tests when upstream publishes a release - #190
Merged
Conversation
nikagra
force-pushed
the
ci/patch-apply-check
branch
2 times, most recently
from
August 25, 2026 09:23
0d6373d to
db928b8
Compare
version_folder's fallback branch logged driver_version_dir_path -- the exact path that does not exist -- so master #2101's log contradicted itself on two adjacent lines: The full directory for '3.11.5.18' tag is .../versions/scylla/3.11.5.18 Show patch's statistics for file '.../versions/scylla/3.11.5.16/patch' Log the directory actually returned, and say it is a fallback. Also fix the adjacent raise, which passed the tag as a second argument to ValueError instead of formatting it, so the tag never appeared in the message. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
nikagra
force-pushed
the
ci/patch-apply-check
branch
from
August 25, 2026 12:38
db928b8 to
67de26c
Compare
Collaborator
|
@nikagra , this should be done differentely, it should be done in release cicd of the driver |
nikagra
force-pushed
the
ci/patch-apply-check
branch
from
August 26, 2026 06:40
67de26c to
0eef63f
Compare
There was a problem hiding this comment.
Pull request overview
Adds early detection of stale driver patches and improves fallback diagnostics.
Changes:
- Corrects fallback logging and error formatting.
- Adds patch-only validation for selected driver tags.
- Adds scheduled Apache upstream-release integration checks.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
run.py |
Reports the actual fallback directory and improves the missing-directory error. |
.github/workflows/version-patch-check.yml |
Validates selected Scylla and Apache patches on PRs or manual runs. |
.github/workflows/upstream-release-watch.yml |
Detects new Apache releases and invokes integration tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A new apache/cassandra-java-driver tag is the one case nothing here reaches. A new Scylla driver tag is caught by scylladb/java-driver's release workflow, and an edited patch is caught by changed-driver-version-tests, which runs a full integration leg for every version directory a pull request touches -- but upstream is tagged by the ASF, so neither a ScyllaDB release nor a matrix pull request happens at that moment. The tag appears, Run.version_folder starts falling back to the newest version directory below it, and the first sign of trouble is the nightly matrix dying in _apply_patch_files() before it runs a single test. The fallback does not survive an apache release in practice. Applying versions/apache/4.19.2/patch to tag 4.19.3 fails on PreparedStatementCancellationIT.java and test-infra/revapi.json; that directory exists only because someone hand-migrated it the day after the tag (5c89f80). Consecutive 4.19.x patches differ by 176, 37 and 92 lines. So detect the tag and run the real integration workflow against it rather than checking patches in isolation: a green run means the version is genuinely supported, not merely that a patch applied. The trigger is a missing version directory -- the exact condition Run.version_folder tests before it starts falling back -- which needs no stored state, keeps firing until someone onboards the tag, and stops on its own once they do. A workflow_dispatch that names a driver_ref always runs, onboarded or not, so an existing release can be re-tested on demand. Only the newest release tag is checked, while the nightly selects the newest two. Widening that would duplicate a line of defence which already exists: a second upstream release landing before the first is onboarded is a discrepancy the nightly is there to surface. Everything reused rather than reinvented: get-version with source github-tag as integration-tests.yml already resolves driver refs, its component-regex filter syntax to exclude the 71 upstream tags that are not clean releases, the input-wins-else-resolved shape of its Normalize inputs step, and a concrete driver_ref passed into integration-tests.yml exactly as changed-driver-version-tests does. This cannot be a webhook: GitHub only delivers those for repositories we own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
nikagra
force-pushed
the
ci/patch-apply-check
branch
from
August 26, 2026 12:20
0eef63f to
fede20f
Compare
nikagra
changed the title
dkropachev
approved these changes
Aug 27, 2026
A missing versions/apache/<tag>/ directory kept should_run true on every scheduled run, so an un-onboarded tag re-ran the multi-hour integration workflow every morning until someone added the directory. A red run is the signal itself and is already reported; repeating it daily buries it. mark-tested now records the tag in an Actions cache once the run reaches a conclusion, success or failure, and the next run finds the marker and stops. Cancelled and skipped runs record nothing, so an infra abort is retried the next day. A workflow_dispatch naming a driver_ref still bypasses the marker. The decision moves out of the workflow heredoc into scripts/upstream_release_watch.py, alongside scripts/pr_integration_changes.py and invoked the same way, because a pull request run only ever sees the newest upstream tag as already onboarded: the missing-directory, already-tested and forced branches never execute there. tests/test_upstream_release_watch.py covers them, and pr-integration-tests.yml gains a unit-tests job so tests/ finally runs in CI at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A follow-up self-review after merge approval surfaced a few gaps worth closing before merge: - integration now explicitly excludes github.event_name == 'pull_request', so a PR touching this workflow/module can never start the multi-hour run even if a not-yet-onboarded tag makes should_run true, instead of relying on that never happening. - GITHUB_OUTPUT writes go through a new write_outputs() using the delimited form, so an embedded newline in a workflow_dispatch driver_ref can't forge extra output lines. - pick() raises a clean SystemExit on malformed JSON instead of an unhandled JSONDecodeError. - summarize() now derives should_run from decide() instead of re-deriving the same split by hand, so the two can't drift apart. - Fixed an unescaped '.' in Run.version_folder's version_pattern regex, and added regression tests for the fallback and ValueError branches this PR already edits (previously untested).
Collaborator
|
@nikagra , great idea to track them in gh cache, when you done feel free to merge it |
- Add a concurrency group so a second schedule/workflow_dispatch trigger waits for an in-flight run instead of racing it into a duplicate multi-hour integration run for the same tag. - Make the step summary aware of pull_request runs: the integration job is unconditionally skipped there, so the summary no longer claims it's "running the integration workflow" when it actually won't. Follow-up from a second review pass on top of 0b9065d.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #189.
apache/cassandra-java-driveris tagged by the ASF, so when a release lands there is no ScyllaDB release and no matrix pull request — nothing fires.Run.version_foldersilently starts patching the new tag with the newest directory below it, and the first sign of trouble is the nightly Jenkins matrix dying in_apply_patch_files()after ~3.5h with 0 test failures (master #2098–#2105).The fallback does not survive an apache release:
versions/apache/4.19.2/patchdoes not apply to tag4.19.3(PreparedStatementCancellationIT.java,test-infra/revapi.json), and that directory exists only because someone hand-migrated it the day after the tag (5c89f80). Consecutive 4.19.x patches differ by 176/37/92 lines.What
.github/workflows/upstream-release-watch.yml— polls apache for the newest release tag and, when it has noversions/apache/<tag>/directory of its own, callsintegration-tests.ymlagainst that exact tag. A green run means the version is genuinely supported, not merely that a patch applied.A missing directory starts the run; a cache marker stops it repeating.
mark-testedrecords the tag once the run reaches a conclusion, success or failure, so each tag gets exactly one multi-hour run — a red one is the signal itself and is already reported, and repeating it every morning until someone onboards the tag would bury it. Cancelled and skipped runs record nothing, so an infra abort is retried the next day, and an evicted marker costs one extra run at worst. Onboarding the directory makes the whole workflow a no-op again, so no marker needs cleaning up. Aworkflow_dispatchnaming adriver_refbypasses the marker, so an existing release can be re-tested on demand.scripts/upstream_release_watch.py— the scheduling decision, out of the workflow heredoc. A pull request run only ever sees the newest upstream tag as already onboarded, so the missing-directory, already-tested and forced branches never execute in CI here;tests/test_upstream_release_watch.pycovers them instead. Same split asscripts/pr_integration_changes.py— a stdlib-only module, invoked from the workflow exactly aspr-integration-tests.ymlinvokes that one..github/workflows/pr-integration-tests.yml— aunit-testsjob. Nothing in this repo ranpytestbefore, sotests/had no teeth; all 30 existing tests pass.Reuses what is already here rather than adding a mechanism —
get-versionwithsource: github-tagasintegration-tests.ymlalready resolves driver refs, its component-regex filter syntax (71 of the 179 upstream tags are not clean releases), the input-wins-else-resolved shape of itsNormalize inputsstep, and a concretedriver_refpassed intointegration-tests.ymlexactly aschanged-driver-version-testsdoes. The marker reuses theactions/cacherestore/save pin already inintegration-tests.yml.run.py— the fallback branch logged the directory that does not exist, so master #2101 contradicted itself on adjacent lines:Log the directory actually returned and say it is a fallback. Also fixes the adjacent
raise ValueError("...'%s'", self._tag), which passed the tag as a second argument toValueErrorinstead of formatting it, so the tag never reached the message.Scope
Apache is the only gap this fills. Scylla tags are gated in scylladb/java-driver's release workflow (scylladb/java-driver#899, scylladb/java-driver#898) between
release:prepareandrelease:perform, so a release aborts before publishing; patch edits are covered by the integration tests every matrix PR already runs; and the nightly stays the backstop for anything either misses.For the same reason only the newest release tag is checked, though the nightly selects the newest two. Widening that would add a line of defence which already exists.
An earlier revision of this PR added a scheduled and PR-triggered patch-apply check. Dropped after review, for the reasons above.
This cannot be a webhook — GitHub only delivers those for repositories we own — so it is a daily poll. Note
scheduleonly fires from the default branch, so it does nothing until this merges.🤖 Generated with Claude Code