fix(ci): the release workflow could not pass its own test suite - #203
Merged
Conversation
Auto Release failed on main after six merges, so v1.11.4 was never cut. The failing job was "Verify CI passes", and the failure was tests/test_compose_image_pins.py: "no semver tags in this checkout — CI must fetch tags (fetch-tags: true), otherwise this drift test silently passes and the pin can rot again". The test was working exactly as designed. It compares the pinned compose image tag against the newest RELEASED version and fails loudly rather than skipping, because a silent skip is how the pin rotted to 1.4 and shipped issue #188. test.yml was given fetch-depth: 0 + fetch-tags: true when that test was written; release.yml was not, and nothing connected the two. release.yml now fetches tags, and a guard asserts every workflow job that runs the unfiltered suite checks out with them, so a third workflow cannot repeat it. The guard is narrowed to runs that would actually COLLECT the drift test. Its first version flagged collector-live-check.yml, which runs `pytest tests/ -m live` — no test carries that marker, so the drift test never executes there and requiring tags would have been a rule about a problem that workflow cannot have. A control pins that distinction rather than leaving the rule merely strict. Negative control: removing fetch-tags from release.yml fails the guard.
📝 WalkthroughWalkthroughChangesWorkflow tag validation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_compose_image_pins.py`:
- Around line 184-190: Update the checkout validation around the offenders loop
to require parsed fetch-tags to be boolean true and fetch-depth to be numeric
zero, rejecting string values such as "false" and nonzero depths. Add
negative-control test cases covering fetch-tags: "false" and a nonzero
fetch-depth while preserving acceptance of the required inputs.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 37ca27d4-83c5-418e-ac64-3c9d7f12e903
📒 Files selected for processing (2)
.github/workflows/release.ymltests/test_compose_image_pins.py
Comment on lines
+184
to
+190
| checkouts = [s for s in steps if str(s.get("uses", "")).startswith("actions/checkout")] | ||
| for step in checkouts: | ||
| if not (step.get("with") or {}).get("fetch-tags"): | ||
| offenders.append(f"{name}:{job_name}") | ||
| assert not offenders, ( | ||
| f"these run pytest without fetching tags, so the compose-pin drift test fails: {offenders}. " | ||
| "Add `fetch-depth: 0` and `fetch-tags: true` to the checkout." |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file context =="
sed -n '120,210p' tests/test_compose_image_pins.py || true
echo "== related tests =="
rg -n "TestEveryWorkflowThatRunsTheSuiteFetchesTags|fetch-tags|fetch-depth|actions/checkout|pytest" tests/test_compose_image_pins.py tests || true
echo "== python yaml/load semantics probe =="
python3 - <<'PY'
import yaml
samples = {
"null": None,
"boolean_true": True,
"string_true": "true",
"string_false": "false",
"quoted_false": '"false"',
"fetch_tags_1": {
"uses": "actions/checkout",
"with": {"fetch-tags": True, "fetch-depth": 1},
},
"fetch_tags_false": {
"uses": "actions/checkout",
"with": {"fetch-tags": "false", "fetch-depth": "0"},
},
"fetch_tags_true_depth_1": {
"uses": "actions/checkout",
"with": {"fetch-tags": True, "fetch-depth": "1"},
},
}
for name, step in samples.items():
if step is None:
value = None
elif isinstance(step, dict):
value = (step.get("with") or {}).get("fetch-tags")
else:
value = step
print(f"{name}: value={value!r} bool={bool(value)}")
PYRepository: GeiserX/CashPilot
Length of output: 43862
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
print("== workflows using pytest ==")
for path in sorted(Path(".github/workflows").glob("*.yml")):
text = path.read_text()
if "pytest" in text:
with_checkout = False
with fetch_tags_1 = False
with fetch_tags_false = False
for lineno, line in enumerate(text.splitlines(), 1):
if "uses:" in line and "actions/checkout" in line:
with_checkout = True
elif with_checkout:
if "fetch-tags:" in line:
with_fetch_tags_1 = True
elif 'fetch-tags: "false"' in line or "fetch-tags: false" in line:
with_fetch_tags_false = True
fetch_tags_found = any("fetch-tags:" in l for l in text.splitlines())
fetch_depth_found = any("fetch-depth:" in l for l in text.splitlines())
print(f"{path}: pytest={str(bool(text))}: checkout={with_checkout}; has fetch-tags={fetch_tags_found}; has fetch-depth={fetch_depth_found}; fetch-tags boolean-ish {with_fetch_tags_1}; fetch-tags false-ish {with_fetch_tags_false}")
print("\n== python parse samples from strings ==")
import ast, os
import glob
import subprocess
# read all workflow files
for path in Path(".github/workflows").glob("*.yml"):
text = path.read_text()
print(path)
for line in text.splitlines():
if "fetch-tags:" in line:
print(" " + line.strip())
PYRepository: GeiserX/CashPilot
Length of output: 262
Validate checkout inputs instead of present keys.
Line 186 only checks that fetch-tags is truthy. A string value such as fetch-tags: "false" currently passes, and the required fetch-depth: 0 input is not checked at all. Update the guard to compare the parsed values, or validate through a YAML parser, so these invalid workflow values are rejected.
Proposed fix
checkouts = [s for s in steps if str(s.get("uses", "")).startswith("actions/checkout")]
for step in checkouts:
- if not (step.get("with") or {}).get("fetch-tags"):
+ checkout_with = step.get("with") or {}
+ if (
+ str(checkout_with.get("fetch-tags")).lower() != "true"
+ or str(checkout_with.get("fetch-depth")) != "0"
+ ):
offenders.append(f"{name}:{job_name}")Add negative controls for fetch-tags: "false" and a nonzero fetch-depth.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| checkouts = [s for s in steps if str(s.get("uses", "")).startswith("actions/checkout")] | |
| for step in checkouts: | |
| if not (step.get("with") or {}).get("fetch-tags"): | |
| offenders.append(f"{name}:{job_name}") | |
| assert not offenders, ( | |
| f"these run pytest without fetching tags, so the compose-pin drift test fails: {offenders}. " | |
| "Add `fetch-depth: 0` and `fetch-tags: true` to the checkout." | |
| checkouts = [s for s in steps if str(s.get("uses", "")).startswith("actions/checkout")] | |
| for step in checkouts: | |
| checkout_with = step.get("with") or {} | |
| if ( | |
| str(checkout_with.get("fetch-tags")).lower() != "true" | |
| or str(checkout_with.get("fetch-depth")) != "0" | |
| ): | |
| offenders.append(f"{name}:{job_name}") | |
| assert not offenders, ( | |
| f"these run pytest without fetching tags, so the compose-pin drift test fails: {offenders}. " | |
| "Add `fetch-depth: 0` and `fetch-tags: true` to the checkout." |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_compose_image_pins.py` around lines 184 - 190, Update the checkout
validation around the offenders loop to require parsed fetch-tags to be boolean
true and fetch-depth to be numeric zero, rejecting string values such as "false"
and nonzero depths. Add negative-control test cases covering fetch-tags: "false"
and a nonzero fetch-depth while preserving acceptance of the required inputs.
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
1 participant
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.
What happened
Auto Release failed on
mainafter the six merges, so v1.11.4 was never cut.The failing job was
Verify CI passes, and the failure was:The test was working exactly as designed. It compares the pinned compose image tag against the newest released version, and fails loudly rather than skipping — because a silent skip is precisely how the pin rotted to
1.4and shipped issue #188.test.ymlwas givenfetch-depth: 0+fetch-tags: truewhen that test was written.release.ymlwas not, and nothing connected the two.The fix
release.ymlfetches tags, and a guard asserts every workflow job that runs the unfiltered suite checks out with them — so a third workflow can't repeat this.The guard is narrowed to invocations that would actually collect the drift test. Its first version flagged
collector-live-check.yml, which runspytest tests/ -m live; no test carries that marker, so the drift test never executes there and requiring tags would have been a rule about a problem that workflow cannot have. A control pins that distinction rather than leaving the rule merely strict.Evidence
ruff check+ruff format --checkclean, 95.17% coverage, 2706 passed.Negative control: removing
fetch-tagsfromrelease.ymlfails the guard; a marker-filtered invocation is asserted not to require it.Summary by CodeRabbit
Bug Fixes
Tests