KSM-1269 Run Java test matrix on release-branch PRs - #1119
Conversation
Add release/sdk/java/core/** to pull_request and push triggers in test.java.yml so CI fires on PRs targeting or pushing to the release branch.
mgallego-keeper
left a comment
mgallego-keeper
left a comment
There was a problem hiding this comment.
Approving. The trigger change is correct and self-verifying: all four matrix jobs (Java 8/11/17/21) ran and passed on this PR against base release/sdk/java/core/v17.4.0, which is exactly the case that previously got no signal.
Verified:
release/sdk/java/core/**matches all 17 existing Java core release branches, including the non-versionedgraph-links-update-v17.0.1.- Structure matches the existing
test.ruby.ymlprecedent exactly (master+ release glob on bothpull_requestandpush). - The
pathsfilter still covers both changed files. - Security posture unchanged:
contents: read,persist-credentials: false, actions pinned to SHAs, no secrets, nopull_request_target.
Two non-blocking notes.
1. The README changelog entry. Every other 17.4.0 entry describes a change visible to someone consuming the published Maven artifact (generatePassword, isEditable, HTTP timeouts, getFolders(), deleteFolder(), key ID validation, file permissions). A GitHub Actions trigger fix has no effect on the shipped JAR, so it reads as noise in the SDK's release notes. I would drop the README hunk; nothing parses that changelog (the Maven publish workflow does not read it, and no script in the repo references it), so removing it is risk-free. If it stays: append it after KSM-1262 rather than inserting above it, since the list is otherwise append-ordered, and keep it on one line like every other bullet.
2. push on the release glob will double runs. PR #1110 is open with head release/sdk/java/core/v17.4.0 and base master. Once this merges, every push to the release branch fires both push (branch matches the release glob) and pull_request: synchronize on #1110 (base matches master), so 8 matrix jobs per push instead of 4. Draft status does not suppress it, since drafts still emit synchronize.
Not worth blocking on for a 2-minute job, and test.ruby.yml already carries the identical duplication, so this is consistent with the house pattern. One thing worth knowing if it ever does get addressed: a concurrency: block will not dedupe it. The standard group key ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} puts the push run in Test-Java-refs/heads/release/... and the PR run in Test-Java-1110, so both still run. The only real options are dropping the release glob from push: (the release PR to master is open for most of the branch's life, so pull_request already covers it) or accepting the duplication.
Follow-up tickets for the remaining workflows with the same gap are being filed against the audit item in KSM-1269.
1b1e4dc
into
release/sdk/java/core/v17.4.0
…cle guard Brings in #1117 (Java API compatibility), #1119 (CI on release-branch PRs) and #1120 (17.4.0 docs). Two files conflicted; SecretsManager.kt merged cleanly and the cycle guard is unchanged. README.md: both sides appended a changelog entry at the same position. Kept both, with KSM-1269 ahead of KSM-1270 in landing order. SecretsManagerTest.kt: the conflict bundled the new cycle test with testSecretsManagerOptionsAcceptsCustomTimeouts, whose incoming side was a deletion. That test was not dropped by #1117, it was relocated to the new TimeoutTest.kt as timeoutOptions_acceptCustomValues with identical assertions. Resolving in favour of our side would have left the same assertions duplicated across two files, so the deletion is accepted here and only the cycle test is kept. Verified on Zulu JDK 8: 71 tests, 0 failures. That is 69 from the PR head, plus 3 from TimeoutTest.kt, minus the 1 relocated test. The net diff against the base branch is the KSM-1270 change alone.
…-531-java-proxy Resolves the three conflicts against the base commits that landed after the rebase (#1117, #1119, #1120): - SecretsManager.kt: keep the proxy-aware uploadFile call and private helper, which are supersets of base's timeout threading (proxyUrl, allowUnverifiedCertificate, connectTimeoutMillis, readTimeoutMillis all carried). Give proxyUrl a null default in the full postFunction form so TimeoutTest's postFunction(url, tk, payload, true, readTimeoutMillis = X) call binds; the explicit 4-arg overload is kept for the published Java descriptor. - SecretsManagerExceptions.kt: keep base's @jvmoverloads cause constructor and KDoc, and restore the serialVersionUID pin (5401507264959279624) so exceptions round-trip with jars built from released 16.6.6/17.2.0/17.3.0. Without the pin the computed SUID changes to 1054703023149159532 and cross-version deserialization throws InvalidClassException. - README.md: take base's 17.4.0 Breaking Changes block, corrected for this merge: KeeperRecord gained one constructor parameter (isEditable) and SecretsManagerOptions gained three (connectTimeoutMillis, readTimeoutMillis, proxyUrl). All eight base changelog entries and the KSM-531 entry survive. Merged tree verified on JDK 8: 95 tests, 0 failures; the released 4-arg postFunction descriptor and the pinned SUID are present in the built jar.
test.js.yml triggered only on pull_request into master, so every JS core fix that goes to a release branch first, which is all of them, merged with no test signal. The only checks on those PRs are the Socket Security scans, and a green tick there says nothing about whether the SDK still passes. Add release/sdk/javascript/core/** to the pull_request branch filter. The workflow already lists itself under paths, so this change gates itself. Deliberately not adding a push trigger on release/**: that is the duplicate-run pattern KSM-1302 tracks in test.ruby.yml and the four JavaScript KMS workflows, where the pull_request run has already tested the same commit. Follows PR #1119, which made the same change to test.java.yml for KSM-1269. Both are legs of KSM-1284, which tracks the remaining SDK core workflows.
test.js.yml triggered only on pull_request into master, so every JS core fix that goes to a release branch first, which is all of them, merged with no test signal. The only checks on those PRs are the Socket Security scans, and a green tick there says nothing about whether the SDK still passes. Add release/sdk/javascript/core/** to the pull_request branch filter. The workflow already lists itself under paths, so this change gates itself. Deliberately not adding a push trigger on release/**: that is the duplicate-run pattern KSM-1302 tracks in test.ruby.yml and the four JavaScript KMS workflows, where the pull_request run has already tested the same commit. Follows PR #1119, which made the same change to test.java.yml for KSM-1269. Both are legs of KSM-1284, which tracks the remaining SDK core workflows.
2 participants
Summary
release/sdk/java/core/**to thepull_requestandpushtriggers intest.java.yml. Previously, the Java CI matrix only fired on PRs targetingmaster; fix PRs developed against a release branch received no CI signal.push:as well covers direct commits to the release branch.Test plan