refactor(ci): enforce unit-tests gate — workflow_file required (DISTR-456 follow-up)#1788
Draft
anurag-atlan wants to merge 8 commits into
Draft
refactor(ci): enforce unit-tests gate — workflow_file required (DISTR-456 follow-up)#1788anurag-atlan wants to merge 8 commits into
anurag-atlan wants to merge 8 commits into
Conversation
Block publish unless the consumer repo's Tests workflow has reported a successful `unit-tests` check (or whatever name is passed via `unit_tests_check_name`) for the SHA being published. The gate runs before any build minutes are spent so failing tests or missing-coverage checks surface at minute 0 rather than after the multi-arch build. Behaviour: - Check absent on SHA -> fail with onboarding instructions - Check still running -> poll every 30s, max 30 min - Check failed -> fail with the run's conclusion - Check succeeded -> proceed to prepare / build / publish Only runs when `publish: true`; build-only invocations are unaffected. Adds `checks: read` to the workflow permissions for the Checks API lookups. The wait action is pinned to its v1.2.0 SHA per the supply-chain rules in CLAUDE.md. Contract and reference template are documented in docs/standards/unit-tests-gate.md.
…tion The pre-flight check in unit-tests-gate was using the default per_page=30 on the /check-runs endpoint and a client-side jq filter. On commits with many Dependabot or other bot-created check runs, the target unit-tests check can fall to page 2+ and be invisible — causing the gate to abort with a spurious "check missing" error even when the check exists and passed. Switch to the API's server-side `check_name` query parameter, which filters before paginating and exposes the count directly via .total_count. Discovered while testing on atlan-clickhouse-app, where the SHA on main had 31 check-runs (15 of them Dependabot) and unit-tests sat at position 31.
…ck found Adds an optional `unit_tests_workflow_file` input. When set, the gate's pre-flight no longer fails on a missing check — instead it dispatches the named workflow on the publish ref and falls through to the wait step, which then picks up the newly-created check once GitHub registers the run. Mode summary: - Polling-only (default, when input unset): existing behaviour — gate fails fast if no check on SHA. Lower latency, requires the app's tests workflow to run on every publish commit independently (e.g. via push:main). - Hybrid (when input set): freshness on demand. Requires the target workflow to declare workflow_dispatch:. Limitation: gh workflow run accepts a branch or tag, never a bare SHA, so the dispatched run executes on the branch's current HEAD. For typical publish flows this matches the publish SHA exactly; edge cases (commit moves between resolve and dispatch) accept millisecond-scale drift.
Make `unit_tests_workflow_file` required and remove `unit_tests_check_name`. The gate now tracks tests by run ID end-to-end: - Look for an existing run of the named workflow on the publish SHA. - If none, dispatch on the publish ref and capture the new run's ID. - Poll that run by ID until completion; require conclusion=success. This removes the polling-only / hybrid mode split and the check-name plumbing. Apps no longer need to expose a check called `unit-tests` — the gate works regardless of how the test job is named. Tests workflow only needs to declare `workflow_dispatch:` and exit non-zero on failure. Replaces `fountainhead/action-wait-for-check` with an inline `gh run view` poll loop, dropping a third-party SHA pin in the process.
Apps that haven't onboarded to the gate yet keep working — the gate skips itself when the input is unset. Enforcement comes later via a one-line follow-up that flips required: false → true. Reasoning: the 72 consumer draft PRs are still draft (waiting for this SDK PR to land). Making the input required would break every existing caller the moment this merges, before consumer PRs can be merged in sequence. This change lets the SDK PR land safely, then consumers opt in at their pace.
…-456 follow-up) Flip the gate from opt-in (non-breaking) to mandatory. Every caller of build-and-publish-app.yaml MUST now set unit_tests_workflow_file; callers that don't are rejected by workflow_call at startup. This is the enforcement-flip follow-up to #1774. Sequence: 1. #1774 lands the gate as opt-in (zero breakage to existing callers). 2. The 72 consumer draft PRs are merged at each app team's pace, each setting unit_tests_workflow_file on its caller. 3. THIS PR flips the input to required, forcing any laggard apps to onboard before they can publish again. Do NOT merge this PR until consumer adoption is high enough. Any app that has not merged its consumer PR by the time this lands will fail to publish until they do.
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Contributor
📜 Docstring Coverage ReportRESULT: PASSED (minimum: 30.0%, actual: 77.9%) Detailed Coverage Report |
Contributor
📦 Trivy Vulnerability Scan Results
Report SummaryCould not generate summary table (data length mismatch: 9 vs 8). Scan Result Detailsrequirements.txtuv.lock |
Contributor
📦 Trivy Secret Scan Results
Report SummaryCould not generate summary table (data length mismatch: 9 vs 8). Scan Result Detailsrequirements.txtuv.lock |
Collaborator
🧪 SDR Integration Tests (testcontainer) — mysqlStatus: Failed ❌ What this validates
Re-runPush a new commit, or re-run from the Actions tab. |
Collaborator
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
Collaborator
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
Contributor
|
🛠 Full Test Coverage Report: https://k.atlan.dev/coverage/application-sdk/pr/1788 |
anurag-atlan
added a commit
that referenced
this pull request
May 19, 2026
While the gate is opt-in (unit_tests_workflow_file optional), make gate failure a warning rather than a hard block. Lets app teams merge their consumer PRs even when their current tests are broken — they get a visible warning but their publishes continue working. Once the enforcement-flip PR (#1788) lands, this reverts to exit 1 and gate failure blocks publish properly. Combined with workflow_file being optional, this gives a 2-step ramp: 1. Today: opt-in + warn-only (no breakage possible) 2. Enforcement flip: required + hard-block (laggards get forced)
Instead of requiring every caller to set the input explicitly, default to the conventional filename used by 49/60 already-onboarded apps. Apps with bespoke names (e.g. 'tests.yml', 'checks.yml') continue to override. Combined with the enforce-mode flip in this PR: - Apps using 'unit-tests.yml': gate auto-activates, no caller change needed - Apps with bespoke filenames: caller must explicitly override OR gate fails at dispatch (intended — forces correct configuration) - Apps that don't want the gate: opt out by setting workflow_file to '' Reduces the number of consumer PRs needed for full fleet onboarding.
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
3 participants
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.
Summary
Phase 2 / enforcement flip for the unit-tests gate from #1774. Three changes:
unit_tests_workflow_fileto"unit-tests.yml"— the conventional filename used by 49 of 60 already-onboarded apps. Apps with bespoke filenames continue to override. Reduces the number of consumer PRs needed for full fleet adoption.exit 1, propagates toprepare/build/publishbeing skipped, and the overall workflow conclusion isfailure.After this merges, the unit-tests gate is the contract it's intended to be: every caller MUST run their unit tests successfully before publish, and most apps don't need any caller-side configuration.
Sequence:
unit-tests.yml, flips warn → block. Any app that:Diff vs main (after #1774 already merged)
unit_tests_workflow_file.requiredfalsefalse(unchanged)unit_tests_workflow_file.default"""unit-tests.yml"unit-tests-gate.ifinputs.publish && inputs.unit_tests_workflow_file != ''inputs.publish::warning::, continues::error::,exit 1Effect after merge
unit_tests_workflow_fileunset, hasunit-tests.ymlunit_tests_workflow_fileunset, nounit-tests.ymlunit_tests_workflow_fileexplicitly set to""unit_tests_workflow_fileset + tests passunit_tests_workflow_fileset + tests failThe 12 apps still on draft consumer PRs
After this lands:
unit-tests.yml: auto-onboarded, no consumer PR merge neededtests.yml, mysqltests.yml, publishchecks.yml): consumer PR merge required, otherwise publish failsCoordination
Merge this when consumer adoption is sufficient and you're ready to enforce.