ci: gate integration tests on unit-test success - #1283
Conversation
Deploying infrahub-sdk-python with
|
| Latest commit: |
4cac0c5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9431be37.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://pog-em-pipeline-fail-fast-z0.infrahub-sdk-python.pages.dev |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/ci.yml">
<violation number="1" location=".github/workflows/ci.yml:320">
P2: Adding `unit-tests` to this job's `needs` now gates `integration-tests-latest-infrahub` (40-min huge-runner job) not just on test failures but on every step inside the `unit-tests` matrix leg, including the `Upload coverage to Codecov` and `Report coverage for pytest-plugin` steps. Those upload `CODECOV_TOKEN` to an external service, so a transient Codecov outage or upload failure marks the leg `failure` and silently skips integration coverage even when the actual tests are green. Previously integration ran independently of unit-tests/Codecov health. Consider making the coverage-upload steps conditional on test success or not part of the aggregate gate, so a non-test failure does not block the expensive integration job.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| !contains(needs.*.result, 'cancelled') && | ||
| needs.files-changed.outputs.python == 'true' | ||
| needs: ["prepare-environment", "files-changed", "yaml-lint", "python-lint"] | ||
| needs: ["prepare-environment", "files-changed", "yaml-lint", "python-lint", "unit-tests"] |
There was a problem hiding this comment.
P2: Adding unit-tests to this job's needs now gates integration-tests-latest-infrahub (40-min huge-runner job) not just on test failures but on every step inside the unit-tests matrix leg, including the Upload coverage to Codecov and Report coverage for pytest-plugin steps. Those upload CODECOV_TOKEN to an external service, so a transient Codecov outage or upload failure marks the leg failure and silently skips integration coverage even when the actual tests are green. Previously integration ran independently of unit-tests/Codecov health. Consider making the coverage-upload steps conditional on test success or not part of the aggregate gate, so a non-test failure does not block the expensive integration job.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ci.yml, line 320:
<comment>Adding `unit-tests` to this job's `needs` now gates `integration-tests-latest-infrahub` (40-min huge-runner job) not just on test failures but on every step inside the `unit-tests` matrix leg, including the `Upload coverage to Codecov` and `Report coverage for pytest-plugin` steps. Those upload `CODECOV_TOKEN` to an external service, so a transient Codecov outage or upload failure marks the leg `failure` and silently skips integration coverage even when the actual tests are green. Previously integration ran independently of unit-tests/Codecov health. Consider making the coverage-upload steps conditional on test success or not part of the aggregate gate, so a non-test failure does not block the expensive integration job.</comment>
<file context>
@@ -317,7 +317,7 @@ jobs:
!contains(needs.*.result, 'cancelled') &&
needs.files-changed.outputs.python == 'true'
- needs: ["prepare-environment", "files-changed", "yaml-lint", "python-lint"]
+ needs: ["prepare-environment", "files-changed", "yaml-lint", "python-lint", "unit-tests"]
runs-on:
group: "huge-runners"
</file context>
There was a problem hiding this comment.
Verified against this repo's setup - this specific risk doesn't apply here, so no change is needed.
The concern assumes a failed Codecov upload marks the unit-tests leg as failure. But the repo pins the legacy codecov==2.1.13 uploader, whose upload path is non-fatal by default. From codecov/__init__.py:
except Exception as e:
write("Error: " + str(e))
...
sys.exit(1 if codecov.required else 0) # exits 0 unless --required is passedThe steps invoke uv run codecov --flags ... with no --required/-Z, so any upload error (transient 5xx outage, network failure, 4xx) is caught and exits 0. The leg stays success and integration is not spuriously skipped.
The reasoning is sound in the general case (codecov-cli, or this uploader with --required, both fail on upload error) - just not for the current config. Noting for the record: if we ever migrate to codecov-cli or add --required, the clean fix is continue-on-error: true on the coverage-upload steps.
Add unit-tests to the needs list of integration-tests-latest-infrahub so the expensive huge-runner integration job is skipped when any unit-test matrix leg fails, instead of running in parallel with failing unit tests.
2b0ba2b to
4cac0c5
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## stable #1283 +/- ##
==========================================
- Coverage 84.24% 84.23% -0.01%
==========================================
Files 147 147
Lines 13066 13045 -21
Branches 1940 1930 -10
==========================================
- Hits 11007 10989 -18
+ Misses 1494 1493 -1
+ Partials 565 563 -2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
fatih-acar
left a comment
fatih-acar
left a comment
There was a problem hiding this comment.
Good catch, thank you for the help on this!
2 participants
What
Gate the integration-test job on unit-test success so CI fails fast.
integration-tests-latest-infrahub(which runs on thehuge-runnersgroup with a 40-minute timeout) previously listed only:Its guard
!contains(needs.*.result, 'failure')therefore never inspected theunit-testsmatrix, so the expensive integration job started in parallel with unit tests and kept running even when unit tests were red.Change
Add
unit-teststo thatneedslist. The existing!contains(needs.*.result, 'failure')/'cancelled')guards then gate the integration job on unit-test success automatically - a matrix job aggregates tofailureif any Python-version leg fails, so a single failing leg skips integration. Thealways()prefix keeps the guard evaluating whenunit-testsis skipped (no Python changes), where thepython == 'true'clause already skips integration.The commented-out
integration-tests-local-infrahubjob'sneedsis updated to match so it stays correct if re-enabled.Tradeoff (deliberate and accepted)
This serializes what used to run in parallel: integration tests now start only after every unit-test matrix leg (Python 3.10-3.14) has finished, rather than concurrently with them.
The added latency on passing runs is a known, accepted cost. The goal is to stop spending scarce
huge-runnerscapacity on branches whose unit tests are already failing, where the integration result would not change the outcome.