Pin setup-uv references and stabilise action tests#272
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
OverviewThis pull request hardens the GitHub Actions workflow infrastructure by pinning all Key ChangesSetup-uv PinningAll Workflow Test Harness Stabilisation
Generate-Coverage Script RefactoringCommon helpers (
Release-to-PyPI-uv Script Hardening
Documentation Updates
Build Integration
Code QualityRemoved and fixed:
Testing and ValidationThe changes include:
Follow-up ItemsProperty-based testing and snapshot testing enhancements are tracked as future improvements (cross-referenced in PR objectives). WalkthroughUpdate astral-sh/setup-uv pins across actions/workflows; centralise environment parsing for generate-coverage scripts and refactor CLIs; run action scripts under sys.executable in tests; add act runtime probing in the workflow test harness; update Makefile and docs; add tests and snapshots. ChangesWorkflow tooling and test harness enhancements
Sequence Diagram(s) Possibly related issues
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Contributor
Reviewer's GuidePins all astral-sh/setup-uv usages to a single commit, hardens the workflow test harness to accurately detect and configure the container runtime used by act (including Podman Docker-API health probing and ACT command selection), and stabilises Typer-based action scripts and their tests by routing through the project interpreter and making env-var handling more resilient and explicit. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Owner
Author
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Comment on lines +141 to +144 if (
shutil.which("docker")
and _command_succeeds("docker", "info")
and _command_succeeds("docker", "ps", "-a")❌ New issue: Complex Conditional |
Owner
Author
|
@coderabbitai Please investigate the cause of the following issue using codegraph exploration and research, identify a fix and provide an AI coding agent prompt for the fix: |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
leynos
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Complex Method.github/actions/generate-coverage/scripts/run_rust.py: main What lead to degradation?main has a cyclomatic complexity of 15, threshold = 9 Why does this problem occur?A Complex Method has a high cyclomatic complexity. The recommended threshold for the Python language is a cyclomatic complexity lower than 9. How to fix it?There are many reasons for Complex Method. Sometimes, another design approach is beneficial such as a) modeling state using an explicit state machine rather than conditionals, or b) using table lookup rather than long chains of logic. In other scenarios, the function can be split using EXTRACT FUNCTION. Just make sure you extract natural and cohesive functions. Complex Methods can also be addressed by identifying complex conditional expressions and then using the DECOMPOSE CONDITIONAL refactoring. Helpful refactoring examplesTo get a general understanding of what this code health issue looks like - and how it might be addressed - we have prepared some diffs for illustrative purposes. SAMPLE# complex_method.js
function postItem(item) {
if (!item.id) {
- if (item.x != null && item.y != null) {
- post(item);
- } else {
- throw Error("Item must have x and y");
- }
+ // extract a separate function for creating new item
+ postNew(item);
} else {
- if (item.x < 10 && item.y > 25) {
- put(item);
- } else {
- throw Error("Item must have an x and y value between 10 and 25");
- }
+ // and one for updating existing items
+ updateItem(item);
}
}
+
+function postNew(item) {
+ validateNew(item);
+ post(item);
+}
+
+function updateItem(item) {
+ validateUpdate(item);
+ put(item);
+}
+ |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. .github/actions/generate-coverage/scripts/run_rust.py Comment on lines +396 to +399 output_path: typ.Annotated[
Path | None, typer.Option(envvar="INPUT_OUTPUT_PATH")
] = None,
features: typ.Annotated[str, typer.Option(envvar="INPUT_FEATURES")] = "",❌ New issue: Complex Method |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph. If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced. Complex Method.github/actions/generate-coverage/scripts/run_rust.py: main What lead to degradation?main has a cyclomatic complexity of 10, threshold = 9 Why does this problem occur?A Complex Method has a high cyclomatic complexity. The recommended threshold for the Python language is a cyclomatic complexity lower than 9. How to fix it?There are many reasons for Complex Method. Sometimes, another design approach is beneficial such as a) modeling state using an explicit state machine rather than conditionals, or b) using table lookup rather than long chains of logic. In other scenarios, the function can be split using EXTRACT FUNCTION. Just make sure you extract natural and cohesive functions. Complex Methods can also be addressed by identifying complex conditional expressions and then using the DECOMPOSE CONDITIONAL refactoring. Helpful refactoring examplesTo get a general understanding of what this code health issue looks like - and how it might be addressed - we have prepared some diffs for illustrative purposes. SAMPLE# complex_method.js
function postItem(item) {
if (!item.id) {
- if (item.x != null && item.y != null) {
- post(item);
- } else {
- throw Error("Item must have x and y");
- }
+ // extract a separate function for creating new item
+ postNew(item);
} else {
- if (item.x < 10 && item.y > 25) {
- put(item);
- } else {
- throw Error("Item must have an x and y value between 10 and 25");
- }
+ // and one for updating existing items
+ updateItem(item);
}
}
+
+function postNew(item) {
+ validateNew(item);
+ post(item);
+}
+
+function updateItem(item) {
+ validateUpdate(item);
+ put(item);
+}
+ |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Fetch the current run_rust.py from the PR branch
gh api repos/leynos/shared-actions/contents/.github/actions/generate-coverage/scripts/run_rust.py?ref=chore/bump-setup-uv-sha \
--jq '.content' | base64 -dLength of output: 16831
Further extraction would be deleterious. Below is a suppression message for the CodeScene UI. CodeScene suppression messageThe residual cyclomatic complexity in All logically cohesive sub-concerns have already been extracted into dedicated private helpers ( |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b95aa3def
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 @.github/actions/generate-coverage/scripts/run_rust.py:
- Around line 421-423: The current logic overwrites an explicitly passed
manifest_path with the DETECTED_CARGO_MANIFEST env var; change it so
manifest_path is only replaced by the environment value when no explicit value
was supplied (e.g., when manifest_path equals the default Path("Cargo.toml") or
is None). Locate the manifest_path parameter and the code that assigns from
DETECTED_CARGO_MANIFEST (the assignment around the existing use of
manifest_path) and wrap that override in a conditional that checks manifest_path
is the default before setting it from os.getenv("DETECTED_CARGO_MANIFEST"),
thereby preserving any explicit argument passed to the function.
In `@docs/local-validation-of-github-actions-with-act-and-pytest.md`:
- Around line 30-33: The guidance uses first/second-person pronouns ("your
sandbox", "use ... to") so update the sentences to an impersonal voice: rephrase
the lines that mention the socket and the command (referencing
ACT_WORKFLOW_TESTS and sudo -E make test) to remove "you/your" and "use", e.g.
state that commands that access the socket must run with permission and that, in
this repository, the workflow harness can be exercised by running the indicated
command when the sandbox requires elevated runtime access; apply the same
impersonal rewrite pattern to the other occurrence noted around lines 190-191.
- Around line 25-27: Remove the comma before "because" in the sentence that
begins "The pytest harness checks `GET /v1.41/containers/json?all=true`, because
this is the path `act` uses before starting a job." — edit the fragment in the
docs so it reads without the comma (i.e., "...checks `GET
/v1.41/containers/json?all=true` because this is the path `act` uses before
starting a job."), preserving the backtick-enclosed GET path and surrounding
punctuation.
In `@tests/workflows/conftest.py`:
- Around line 79-85: The parsing code should guard against empty/malformed
Docker API responses so import-time runtime discovery doesn't raise; update the
block that builds response -> head via response.partition and then status_line =
head.splitlines()[0].decode(...) to first ensure head.splitlines() is non-empty
and that parts = status_line.split(maxsplit=2) has at least 2 elements, and wrap
the int(parts[1]) conversion in a try/except (ValueError) to return a
probe-failure tuple (e.g., (0, response.decode(...))) when parsing fails instead
of letting IndexError/ValueError propagate; change checks around
head.splitlines(), status_line, parts and int(parts[1]) accordingly so callers
like _ACT_RUNTIME_STATUS initialization get a safe fallback.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7cf8f678-6de6-4b8a-b66e-7540f06c0b7c
📒 Files selected for processing (26)
.github/actions/determine-release-modes/action.yml.github/actions/ensure-cargo-version/action.yml.github/actions/export-cargo-metadata/action.yml.github/actions/generate-coverage/action.yml.github/actions/generate-coverage/scripts/merge_cobertura.py.github/actions/generate-coverage/scripts/run_python.py.github/actions/generate-coverage/scripts/run_rust.py.github/actions/generate-coverage/tests/test_scripts.py.github/actions/linux-packages/action.yml.github/actions/macos-package/action.yml.github/actions/ratchet-coverage/action.yml.github/actions/release-to-pypi-uv/action.yml.github/actions/release-to-pypi-uv/scripts/publish_release.py.github/actions/rust-build-release/action.yml.github/actions/setup-rust/action.yml.github/actions/stage-release-artefacts/action.yml.github/actions/upload-release-assets/action.yml.github/actions/validate-linux-packages/action.yml.github/workflows/ci.yml.github/workflows/dependabot-automerge.yml.github/workflows/rust-toy-app.yml.github/workflows/test-rust-build-release-root-discovery.ymlMakefiledocs/local-validation-of-github-actions-with-act-and-pytest.mdtests/workflows/conftest.pytests/workflows/test_conftest_runtime.py
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/actions/generate-coverage/scripts/run_python.py (1)
409-486:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSplit
mainand unblock the failing quality gate.Reduce this method to orchestration only. Extract input resolution and
GITHUB_OUTPUTemission into helpers, then call them frommain. The current shape still triggers the failing Large Method gate.♻️ Proposed refactor
+def _resolve_main_inputs( + output_path: Path | None, + lang: str | None, + fmt: str | None, + github_output: Path | None, +) -> tuple[Path, str, str, Path]: + resolved_output = output_path or Path(_required_env("INPUT_OUTPUT_PATH")) + resolved_lang = lang or _required_env("DETECTED_LANG") + resolved_fmt = fmt or _required_env("DETECTED_FMT") + resolved_gh_out = github_output or Path(_required_env("GITHUB_OUTPUT")) + return resolved_output, resolved_lang, resolved_fmt, resolved_gh_out + + +def _write_github_output(github_output: Path, out: Path, percent: str) -> None: + with github_output.open("a") as fh: + fh.write(f"file={out}\n") + fh.write(f"percent={percent}\n") + def main( @@ ) -> None: @@ - output_path = output_path or Path(_required_env("INPUT_OUTPUT_PATH")) - lang = lang or _required_env("DETECTED_LANG") - fmt = fmt or _required_env("DETECTED_FMT") - github_output = github_output or Path(_required_env("GITHUB_OUTPUT")) + output_path, lang, fmt, github_output = _resolve_main_inputs( + output_path, lang, fmt, github_output + ) @@ - with github_output.open("a") as fh: - fh.write(f"file={out}\n") - fh.write(f"percent={percent}\n") + _write_github_output(github_output, out, percent)As per coding guidelines: "Keep C90 / mccabe complexity ≤ 9" and "Follow single responsibility and CQRS (command/query segregation)".
🤖 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 @.github/actions/generate-coverage/scripts/run_python.py around lines 409 - 486, The main function is too large and should be reduced to orchestration only: extract the input resolution logic (resolving output_path, lang, fmt, github_output via _required_env and calling _resolve_output_path) into a helper like resolve_inputs(), and extract the GitHub Actions emission (opening github_output and writing "file=" and "percent=") into a helper like emit_github_output(path, percent); then update main to call resolve_inputs(), call _run_coverage(fmt, out) and read_previous_coverage(baseline_file) and finally call emit_github_output(out, percent). Keep existing calls to _resolve_output_path, _run_coverage, and read_previous_coverage unchanged and ensure out.parent.mkdir remains near orchestration where output_path is available.
🤖 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/workflows/conftest.py`:
- Around line 185-197: Replace the mutable global _ACT_RUNTIME_STATUS and its
manual lazy-init in _get_act_runtime_status with a cached no-arg function using
functools.cache: remove the _ACT_RUNTIME_STATUS global and change
_get_act_runtime_status to be a zero-argument function decorated with
functools.cache that calls _probe_act_runtime() on first invocation and returns
the result; keep the same function name (_get_act_runtime_status) and call sites
so behavior is unchanged while avoiding global mutation and Ruff PLW0603.
In `@tests/workflows/test_conftest_runtime.py`:
- Around line 119-154: The two tests use inconsistent environ argument styles
for conftest._probe_act_runtime: test_probe_reports_missing_docker_host_socket
calls _probe_act_runtime() with no args while
test_probe_reports_unreachable_docker_host_socket passes os.environ.copy();
since monkeypatch.setenv has already modified os.environ, change
test_probe_reports_unreachable_docker_host_socket to call
conftest._probe_act_runtime() with no arguments (keep monkeypatch.setenv and the
monkeypatched _read_unix_http), ensuring both tests consistently rely on the
global os.environ behavior.
- Around line 81-116: Hoist the repeated local imports by adding a single
module-level "import os" with the other imports at the top of the file, and
remove the per-test "import os" statements inside
test_docker_host_unix_socket_missing, test_docker_host_unix_socket_unreachable
(and the other test at the noted location); this keeps a single import for the
symbol used by conftest._docker_host_usable and related tests and eliminates
redundant imports.
---
Outside diff comments:
In @.github/actions/generate-coverage/scripts/run_python.py:
- Around line 409-486: The main function is too large and should be reduced to
orchestration only: extract the input resolution logic (resolving output_path,
lang, fmt, github_output via _required_env and calling _resolve_output_path)
into a helper like resolve_inputs(), and extract the GitHub Actions emission
(opening github_output and writing "file=" and "percent=") into a helper like
emit_github_output(path, percent); then update main to call resolve_inputs(),
call _run_coverage(fmt, out) and read_previous_coverage(baseline_file) and
finally call emit_github_output(out, percent). Keep existing calls to
_resolve_output_path, _run_coverage, and read_previous_coverage unchanged and
ensure out.parent.mkdir remains near orchestration where output_path is
available.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0b6a97f8-e29a-461a-b55f-3aef67c67ce4
📒 Files selected for processing (8)
.github/actions/generate-coverage/scripts/common.py.github/actions/generate-coverage/scripts/merge_cobertura.py.github/actions/generate-coverage/scripts/run_python.py.github/actions/generate-coverage/scripts/run_rust.py.github/actions/release-to-pypi-uv/scripts/publish_release.pydocs/local-validation-of-github-actions-with-act-and-pytest.mdtests/workflows/conftest.pytests/workflows/test_conftest_runtime.py
Update all `astral-sh/setup-uv` action and workflow references to the requested commit SHA. Remove stale adjacent version comments so the pin is not contradicted by old tag notes.
Owner
Author
|
@coderabbitai Have the following now been resolved? Annul any requirements that violate the en-GB-oxendict spelling (-ize / -yse / -our) conventions (for example a request to replace "normalize" with "normalise" or "artefact" with "artifact"), or where the requirement unnecessarily increases cyclomatic complexity. |
Remove an invalid dead_code expectation from rust-toy-app's build script now that the included CLI module is used for manpage generation. Replace the broad generate-coverage GITHUB_OUTPUT key checks with a syrupy snapshot that records the exact file and percent output format.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Add direct tests for the workflow runtime command availability and command success helpers using real local process boundaries, complementing the existing act runtime orchestration tests.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/workflows/test_conftest_runtime.py (1)
113-180: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winParametrize the DOCKER_HOST failure-path coverage.
Fold these negative-path tests into parametrized helper/probe matrices. The current layout repeats the same monkeypatch wiring and makes new socket cases harder to keep aligned across
_docker_host_usable()and_probe_act_runtime().As per coding guidelines, "Replace duplicate tests with
@pytest.mark.parametrize" and "Use pytest idioms. Prefer fixtures to setup/teardown methods. Parametrize broadly."🤖 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/workflows/test_conftest_runtime.py` around lines 113 - 180, The tests duplicate failure-path wiring for DOCKER_HOST; consolidate them into parametrized tests that run both _docker_host_usable and _probe_act_runtime scenarios for the same failure cases (missing socket and unreachable socket). Create a pytest.mark.parametrize over cases like ("unix:///nonexistent/path.sock", "missing") and (f"unix://{socket_path}", "unreachable") and in the test body set the same monkeypatches: setenv for DOCKER_HOST, monkeypatch _command_available when probing, and monkeypatch _read_unix_http to raise OSError for the unreachable case; then assert expected usable/available flags and that reason contains "Docker API socket does not exist:" or "Docker API socket is not reachable:" for _docker_host_usable and _probe_act_runtime respectively, reusing the same parameter matrix to keep coverage aligned.
🤖 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/workflows/test_conftest_runtime.py`:
- Around line 30-44: Replace the three near-duplicate tests
(test_command_succeeds_reports_successful_command,
test_command_succeeds_reports_failed_command,
test_command_succeeds_reports_missing_command) with a single
pytest.mark.parametrize test that calls conftest._command_succeeds with each
case; parametrize the cases: (sys.executable, "-c", "pass") -> True,
(sys.executable, "-c", "import sys; sys.exit(3)") -> False, and
("definitely-not-a-real-command-for-tests",) -> False, and assert the expected
boolean for each row (add clear ids for readability).
---
Outside diff comments:
In `@tests/workflows/test_conftest_runtime.py`:
- Around line 113-180: The tests duplicate failure-path wiring for DOCKER_HOST;
consolidate them into parametrized tests that run both _docker_host_usable and
_probe_act_runtime scenarios for the same failure cases (missing socket and
unreachable socket). Create a pytest.mark.parametrize over cases like
("unix:///nonexistent/path.sock", "missing") and (f"unix://{socket_path}",
"unreachable") and in the test body set the same monkeypatches: setenv for
DOCKER_HOST, monkeypatch _command_available when probing, and monkeypatch
_read_unix_http to raise OSError for the unreachable case; then assert expected
usable/available flags and that reason contains "Docker API socket does not
exist:" or "Docker API socket is not reachable:" for _docker_host_usable and
_probe_act_runtime respectively, reusing the same parameter matrix to keep
coverage aligned.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 47596995-aa4e-4b2d-a15c-9915be8b6e28
📒 Files selected for processing (1)
tests/workflows/test_conftest_runtime.py
Collapse duplicate command success probes into a single parametrized test and share the Docker host failure matrix across helper and act runtime probe checks.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
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/workflows/test_conftest_runtime.py`:
- Around line 48-54: The inline noqa on the parameter `expected: bool # noqa:
FBT001` in test_command_succeeds_reports_command_status lacks a justification;
update the suppression to include a short reason (e.g., "# noqa: FBT001 - test
uses a boolean literal for clarity in parametrized cases") so it documents why
the FBT001 rule is intentionally bypassed; edit the test in
tests/workflows/test_conftest_runtime.py next to the `expected` parameter and
keep the scope narrow to that token while preserving the existing signature and
call to conftest._command_succeeds.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5adb80c8-9e88-43dd-a8e2-a815dacb4d6d
📒 Files selected for processing (1)
tests/workflows/test_conftest_runtime.py
Document why the parametrized command status test intentionally uses boolean literal cases while suppressing FBT001.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
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/workflows/test_conftest_runtime.py`:
- Around line 53-54: The parametrized assertion in
tests/workflows/test_conftest_runtime.py uses a bare assert for
conftest._command_succeeds(command, *args) is expected; update this to include a
descriptive failure message that embeds the command, args and expected value so
test failures show context (e.g., include command, args, and expected in the
message) — change the assert to use the comma form with a clear message
referencing conftest._command_succeeds, command and args.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d251190e-d511-40e8-b41e-242074579cf4
📒 Files selected for processing (1)
tests/workflows/test_conftest_runtime.py
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
The parametrized assert in test_command_succeeds_reports_command_status used a bare `assert ... is expected` with no failure message, making failures opaque. Embed command, args, and expected value in the message.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Add spaces around table column separators (MD060), wrap over-length table rows in disable/enable directives (MD013), and deduplicate "Observability" and "Public API" headings (MD024) across developers-guide.md and generate-coverage-design.md.
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.
Summary
This branch pins every
astral-sh/setup-uvreference used by the repository's action and workflow definitions to08807647e7069bb48b6ef5acd8ec9567f424441b. It also hardens the local act workflow harness so Podman Docker API failures are detected before workflow execution, and stabilises Typer-based action script tests under the project test runner.No issue, roadmap task, or execplan is linked to this branch.
Review walkthrough
ACTcommand handling.AnnotatedTyper option andINPUT_UV_INDEXfallback.Validation
uv run --with typer --with packaging --with plumbum --with pyyaml --with pytest-xdist pytest .github/actions/release-to-pypi-uv/tests/test_publish_release.py::test_cli_runner_respects_env_index .github/actions/release-to-pypi-uv/tests/test_publish_release.py::test_cli_proxies_to_main -v: passed,2 passed.uv run pytest .github/actions/generate-coverage/tests/ -v: passed,115 passed.make check-fmt: passed.make typecheck: passed.make lint: passed.make test: passed,728 passed, 93 skipped.ACT=$HOME/go/bin/act ACT_WORKFLOW_TESTS=1 ./.venv/bin/pytest tests/workflows -v -rs: passed,15 passed in 330.52s.Notes
The Podman investigation found that
podman ps -acould print containers while the Docker-compatible API endpointGET /v1.41/containers/json?all=truereturned HTTP 500 withcontainer not known. Since act uses that API path before starting jobs, the harness now verifies the same path and reports the exact runtime problem before invoking act.The generate-coverage scripts keep their PEP 723 metadata for direct action execution. Test execution now uses the project interpreter so
cmd-moxstubs run in the same environment as pytest; the script dependencies required by the inline metadata are already present inpyproject.tomlanduv.lock.Summary by Sourcery
Pin uv setup actions across workflows, harden the local act workflow harness, and stabilise Typer-based action scripts and their tests for more reliable CI and local validation.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests: