Make Whitaker fallback tests hermetic (#53) - #54
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Summary
Validation
WalkthroughAdd a shared helper for hermetic subprocess environments. Verify filtering, overrides, copying, and mutation isolation. Update Whitaker fallback tests to isolate tool resolution and report captured output. Document the helper. ChangesSubprocess environment isolation
Suggested labels: Suggested reviewers: Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (19 passed)
📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
7584444 to
b062f30
Compare
![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: b062f30234
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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_template/test_lint_targets.py`:
- Around line 96-103: Update the test assertions around the generated lint
target execution to verify that result.stdout contains “Whitaker binary:
{expected_whitaker}” for cases where WHITAKER is present, using the resolved
expected path or home-directory value. Keep the existing return-code and marker
assertions, but ensure the test distinguishes the intended WHITAKER resolution
from the bare whitaker fallback.
- Around line 71-75: Update the PATH setup in the test targeting logic to use an
isolated temporary tool directory for the home and missing cases instead of
/usr/bin:/bin, while retaining path_bin for the path case. Ensure any required
test utilities are available in that sandbox or rely on shell built-ins so
host-installed whitaker cannot affect the intended branch.
🪄 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: 18c89b8e-1163-42a7-862e-ee029f3b1fe8
📒 Files selected for processing (2)
tests/helpers/subprocess_env.pytests/test_template/test_lint_targets.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/whitaker(auto-detected)
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
5e6d7c1 to
a78fc9f
Compare
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat documentation and validation coverage as in scope). ❌ Failed checks (1 error, 1 warning)
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== PR 54 changed files and diff summary ==='
git diff --stat origin/main...HEAD
git diff --name-status origin/main...HEAD
echo '=== Relevant source outlines ==='
ast-grep outline tests/test_template/test_lint_targets.py --items all || true
ast-grep outline tests/helpers/subprocess_env.py --items all || true
echo '=== Fallback test and environment helper ==='
sed -n '/def test_makefile_resolves_whitaker_fallback/,/^def /p' tests/test_template/test_lint_targets.py
printf '\n--- tests/helpers/subprocess_env.py ---\n'
cat -n tests/helpers/subprocess_env.py
echo '=== Candidate documentation and references ==='
fd -t f -i 'developer*guide*|adr|execplan|design|readme' . \
-E .git -E target -E node_modules | sort
rg -n -i -C2 'generated_project_env|subprocess[_ -]?env|hermetic|WHITAKER|MAKEFLAGS|MFLAGS|MAKELEVEL' \
-g '!target/**' -g '!node_modules/**' .
echo '=== PR metadata, checks, and issue `#53` ==='
gh pr view 54 --repo leynos/agent-template-rust --json url,headRefName,baseRefName,statusCheckRollup,body
gh issue view 53 --repo leynos/agent-template-rust --json url,title,body,stateLength of output: 1574 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Local repository state ==='
git status --short --branch
git branch -a
printf '\n=== Available relevant files ===\n'
fd -t f . tests docs | sort | grep -E '(^tests/(test_template/test_lint_targets\.py|helpers/subprocess_env\.py)$|(^docs/)|README)' || true
echo '=== Test module structure ==='
ast-grep outline tests/test_template/test_lint_targets.py --items all || true
printf '\n=== Fallback test implementation ===\n'
sed -n '/^def test_makefile_resolves_whitaker_fallback/,/^def /p' tests/test_template/test_lint_targets.py || true
printf '\n=== Environment helper ===\n'
cat -n tests/helpers/subprocess_env.py || true
echo '=== Documentation files and relevant terminology ==='
fd -t f . docs -E .git 2>/dev/null | sort || true
rg -n -i -C3 'generated_project_env|subprocess[_ -]?env|hermetic|WHITAKER|MAKEFLAGS|MFLAGS|MAKELEVEL' \
README.md docs tests -g '!target/**' 2>/dev/null || true
echo '=== GitHub PR 54 files, checks, and issue 53 ==='
gh api 'repos/leynos/agent-template-rust/pulls/54/files?per_page=100' \
--jq '.[] | [.filename, .status, (.patch // "")] | `@tsv`'
gh pr view 54 --repo leynos/agent-template-rust \
--json url,headRefName,baseRefName,statusCheckRollup,body
gh issue view 53 --repo leynos/agent-template-rust --json url,title,body,stateLength of output: 42423
GitHub currently reports |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/helpers/subprocess_env.py`:
- Line 1: Expand the module docstring in subprocess_env.py to document
generated_project_env: state that it copies the parent environment, removes
Make/Whitaker control variables, applies explicit overrides, and is passed to
subprocess calls. Include a brief usage example in the docstring.
In `@tests/test_template/test_lint_targets.py`:
- Line 64: Update the generated shell script in the test fixture to quote the
marker path using shlex.quote(str(marker)) while preserving the shell built-in
redirection and existing script behavior.
🪄 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: 0c1eb493-2c1e-41b3-a8fc-72059d57c913
📒 Files selected for processing (3)
docs/developers-guide.mdtests/helpers/subprocess_env.pytests/test_template/test_lint_targets.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/whitaker(auto-detected)
b12f321 to
c2b76ac
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat documentation and validation coverage as in scope). ❌ Failed checks (1 error, 1 warning, 2 inconclusive)
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Changed files ==='
git diff --name-status origin/main...HEAD
echo '=== Relevant outlines ==='
ast-grep outline tests/helpers/subprocess_env.py --items all || true
fd -t f 'test*helper*' tests -x sh -c 'echo "--- $1"; ast-grep outline "$1" --items all || true' sh {} \;
echo '=== Environment helper ==='
cat -n tests/helpers/subprocess_env.py
echo '=== Helper tests and references ==='
rg -n -C4 '\bgenerated_project_env\b|MAKE_RESOLUTION_VARIABLES|subprocess_env' tests \
-g '*.py' || true
echo '=== Relevant PR diff ==='
git diff --find-renames origin/main...HEAD -- tests/helpers/subprocess_env.py tests/test_helpers.py tests
echo '=== GitHub PR checks ==='
gh pr view 54 --repo leynos/agent-template-rust \
--json url,headRefName,statusCheckRollup \
--jq '{url, headRefName, checks: [.statusCheckRollup[] | {name, status: .status, conclusion: .conclusion}]}'Length of output: 1325 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Candidate test files ==='
fd -t f -e py . tests | sort
echo '=== Environment helper ==='
if [ -f tests/helpers/subprocess_env.py ]; then
cat -n tests/helpers/subprocess_env.py
else
echo 'tests/helpers/subprocess_env.py is absent'
fi
echo '=== Helper implementation references ==='
rg -n -C4 '\bgenerated_project_env\b|MAKE_RESOLUTION_VARIABLES|subprocess_env' \
tests -g '*.py' || true
echo '=== Test function declarations ==='
ast-grep outline tests --items all --type function --match 'test|generated_project_env|subprocess' || true
echo '=== PR changed files ==='
gh api 'repos/leynos/agent-template-rust/pulls/54/files?per_page=100' \
--jq '.[] | [.filename, .status, .additions, .deletions] | `@tsv`'
echo '=== PR checks ==='
gh pr view 54 --repo leynos/agent-template-rust \
--json url,headRefName,statusCheckRollup \
--jq '{url, headRefName, checks: [.statusCheckRollup[] | {name, status, conclusion}]}'Length of output: 15625
GitHub reports successful You are interacting with an AI system. |
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat documentation and validation coverage as in scope). ❌ Failed checks (1 error, 1 warning, 2 inconclusive)
|
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== PR changed files ==='
git diff --name-status origin/main...HEAD
echo '=== Helper outline and implementation ==='
ast-grep outline tests/helpers/subprocess_env.py --items all
cat -n tests/helpers/subprocess_env.py
echo '=== Direct helper-test declarations and implementations ==='
ast-grep outline tests/test_helpers.py --items all
rg -n -C 12 '\btest_generated_project_env_(strips_resolution_controls|returns_independent_copy)\b|\bgenerated_project_env\b|\bMAKE_RESOLUTION_VARIABLES\b' \
tests/test_helpers.py
echo '=== GitHub PR checks ==='
gh pr view 54 --repo leynos/agent-template-rust \
--json url,headRefName,statusCheckRollup \
--jq '{url, headRefName, checks: [.statusCheckRollup[] | {name, status, conclusion}]}'Length of output: 1186 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo='leynos/agent-template-rust'
pr='54'
echo '=== PR head and current checks ==='
head_sha=$(gh pr view "$pr" --repo "$repo" --json headRefOid --jq .headRefOid)
printf 'head_sha=%s\n' "$head_sha"
gh pr view "$pr" --repo "$repo" --json url,statusCheckRollup \
--jq '{url, checks: [.statusCheckRollup[] | {name, status, conclusion}]}'
echo '=== Changed files ==='
gh api "repos/$repo/pulls/$pr/files?per_page=100" \
--jq '.[] | [.filename, .status, .additions, .deletions] | `@tsv`'
echo '=== Helper and helper-test patches ==='
gh api "repos/$repo/pulls/$pr/files?per_page=100" \
--jq '.[] | select(.filename == "tests/helpers/subprocess_env.py" or .filename == "tests/test_helpers.py") | "----- \(.filename) -----\n\(.patch // "(No patch returned)")"'
echo '=== Current helper source ==='
gh api "repos/$repo/contents/tests/helpers/subprocess_env.py?ref=$head_sha" --jq .content \
| base64 --decode | nl -ba
echo '=== Current helper-test source: relevant sections ==='
gh api "repos/$repo/contents/tests/test_helpers.py?ref=$head_sha" --jq .content \
| base64 --decode \
| rg -n -C 14 '\btest_generated_project_env_(strips_resolution_controls|returns_independent_copy)\b|\bgenerated_project_env\b|\bMAKE_RESOLUTION_VARIABLES\b'Length of output: 6142
GitHub reports successful You are interacting with an AI system. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/helpers/subprocess_env.py`:
- Around line 15-23: Annotate MAKE_RESOLUTION_VARIABLES explicitly as
frozenset[str] while preserving its existing values and frozenset
initialization.
In `@tests/test_helpers.py`:
- Around line 76-85: Add an explicit override for one MAKE_RESOLUTION_VARIABLES
member, such as WHITAKER, when calling generated_project_env. Assert the
returned env preserves that override while the other inherited resolution
controls remain absent, and retain the existing PATH, CARGO, and
process-environment assertions.
🪄 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: 103b5d6a-c4da-4f9e-b6db-5295742f4179
📒 Files selected for processing (4)
docs/developers-guide.mdtests/helpers/subprocess_env.pytests/test_helpers.pytests/test_template/test_lint_targets.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/whitaker(auto-detected)
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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_helpers.py`:
- Around line 97-108: Update every assertion in the shown test and the
referenced lines 139–156 to include a diagnostic message instead of using bare
asserts. In the assertions around MAKE_RESOLUTION_VARIABLES, env,
inherited_controls, PATH, CARGO, and WHITAKER, make messages identify the
relevant variable, expected value, and applicable override mapping so
generated-example failures are actionable.
- Around line 62-72: Add type annotations to the environment_value and
generated_project_overrides Hypothesis strategy declarations, using
st.SearchStrategy[str] and st.SearchStrategy[dict[str, str]] respectively while
preserving their existing strategy definitions.
🪄 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: d49b9e41-f976-4ead-8d60-ee479ee321b8
📒 Files selected for processing (1)
tests/test_helpers.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/whitaker(auto-detected)
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
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_helpers.py`:
- Around line 152-179: Extend the assertions around generated_project_env to
verify inherited CARGO preservation: when overrides does not contain "CARGO",
assert env["CARGO"] equals the parent-cargo value established by
monkeypatch.setenv. Keep the existing override behavior unchanged.
🪄 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: 09004b8e-018c-46ad-bd7d-4787f2e4704d
📒 Files selected for processing (1)
tests/test_helpers.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/whitaker(auto-detected)
b5f0f99 to
23811ee
Compare
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_helpers.py`:
- Around line 220-221: Add diagnostic messages to both assertions in the
relevant test, clearly identifying the expected child value ("changed") and
parent value ("after") when copy isolation fails; retain the existing assertion
conditions and use the repository’s required assert-with-message style.
🪄 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: 51718bf2-75b1-47ac-9d75-fc56ff15b5e4
📒 Files selected for processing (4)
docs/developers-guide.mdtests/helpers/subprocess_env.pytests/test_helpers.pytests/test_template/test_lint_targets.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/whitaker(auto-detected)
Strip inherited make and Whitaker resolution variables from generated-project lint subprocesses so the fallback cases exercise the intended PATH, HOME, and missing-tool branches regardless of the parent test environment. Include captured stdout and stderr in fallback assertions so future resolution failures report the selected Whitaker command directly.
Strip `GNUMAKEFLAGS`, isolate the test PATH while supplying only `bash`, and assert the resolved Whitaker binary in successful fallback cases. Document the hermetic generated-project subprocess environment helper.
Describe the hermetic subprocess environment helper's public contract and quote the marker filename written by the generated Whitaker fixture.
Apply the repository formatter after rebasing onto the current parent tooling.
Cover inherited Make and Whitaker control sanitisation, override precedence, and parent-environment isolation in the shared subprocess helper.
Exercise arbitrary generated-project overrides, resolution-control stripping, and copy isolation alongside the focused Whitaker override case.
Type Hypothesis strategies and include expected values, actual values, and override mappings in environment helper assertion failures.
Assert that generated subprocess environments retain the inherited `CARGO` value whenever callers do not provide a CARGO override.
Include expected child and parent values in copy-isolation assertions to make future generated environment failures actionable.
Declare the Whitaker and Make resolution-control collection as `frozenset[str]`.
Report only unexpected resolution-control names when the test fails.
640246e to
ef1487a
Compare
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_template/test_lint_targets.py (1)
124-133: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winResolve executable paths before changing directories or creating symlinks.
When
PATHcontains a relative directory,shutil.which()returns a relative path. ResolvemakeandbashwithPath(...).resolve()immediately after each lookup at all four listed sites. Otherwise, themakecalls can fail aftercwd=project.path, and thebashsymlinks can be broken.🤖 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_template/test_lint_targets.py` around lines 124 - 133, Resolve executable paths immediately after each shutil.which lookup by converting the returned make and bash paths through Path(...).resolve(). Apply this at tests/test_template/test_lint_targets.py lines 124-133, 204-213, 244-247, and 262-278; preserve the existing assertions, subprocess calls, and symlink behavior while ensuring all paths remain valid after directory changes.Source: Path instructions
🤖 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.
Outside diff comments:
In `@tests/test_template/test_lint_targets.py`:
- Around line 124-133: Resolve executable paths immediately after each
shutil.which lookup by converting the returned make and bash paths through
Path(...).resolve(). Apply this at tests/test_template/test_lint_targets.py
lines 124-133, 204-213, 244-247, and 262-278; preserve the existing assertions,
subprocess calls, and symlink behavior while ensuring all paths remain valid
after directory changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 97f17430-2230-4061-b6c3-199fdf052d29
📒 Files selected for processing (4)
docs/developers-guide.mdtests/helpers/subprocess_env.pytests/test_helpers.pytests/test_template/test_lint_targets.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/whitaker(auto-detected)
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat documentation and validation coverage as in scope). Check name Status Explanation Resolution Security And Privacy ❌ Error New test failure text formats the entire os.environ-derived env at tests/test_helpers.py:99; this can expose CI tokens and other secrets in pytest errors. Replace env={env} with allow-listed, redacted diagnostics. Do not log inherited environment values; retain assertions on individual variables. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo='leynos/agent-template-rust'
pr='54'
echo '=== PR head and checks ==='
head_sha="$(gh pr view "$pr" --repo "$repo" --json headRefOid --jq .headRefOid)"
printf 'head_sha=%s\n' "$head_sha"
gh pr view "$pr" --repo "$repo" --json url,statusCheckRollup \
--jq '{url, checks: [.statusCheckRollup[] | {name, status, conclusion}]}'
echo '=== Relevant test source ==='
ast-grep outline tests/test_helpers.py --items all --type function --match 'generated_project_env|test_generated_project_env' || true
rg -n -C 12 'env=\{env\}|generated_project_env|MAKE_RESOLUTION_VARIABLES|assert ' \
tests/test_helpers.py
echo '=== Relevant PR patch ==='
gh api "repos/$repo/pulls/$pr/files?per_page=100" \
--jq '.[] | select(.filename == "tests/test_helpers.py") | .patch'
echo '=== Search for full-environment diagnostic patterns in changed Python tests ==='
git diff --unified=20 origin/main...HEAD -- tests \
| rg -n -C 3 '(env=\{|environment=\{|os\.environ|repr\(env\)|\{env\})' || trueLength of output: 33732
The assertion at Lines 97-103 now reports only:
The remaining related assertions report values for explicitly named, test-controlled variables. They do not serialize inherited
✏️ Learnings added
You are interacting with an AI system. |
|
@coderabbitai Have the following now been resolved? tests/test_template/test_lint_targets.py (1)
🤖 Detailed instructionsVerify each finding against current code. Fix only still-valid issues, skip the In Source: Path instructions |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Relevant test structure ==='
ast-grep outline tests/test_template/test_lint_targets.py --items all --type function || true
echo '=== All shutil.which call sites with context ==='
rg -n -C 8 'shutil\.which\s*\(' tests/test_template/test_lint_targets.py
echo '=== Requested source ranges ==='
nl -ba tests/test_template/test_lint_targets.py | sed -n '118,138p;198,218p;238,252p;256,284p'
echo '=== PR patch for the affected test file ==='
git diff --unified=12 origin/main...HEAD -- tests/test_template/test_lint_targets.py
echo '=== PR `#54` status checks ==='
gh pr view 54 --repo leynos/agent-template-rust \
--json url,headRefOid,statusCheckRollup \
--jq '{url, headRefOid, checks: [.statusCheckRollup[] | {name, status, conclusion}]}'Length of output: 4233 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Requested source ranges with line numbers ==='
awk 'NR >= 118 && NR <= 138 || NR >= 198 && NR <= 218 || NR >= 238 && NR <= 252 || NR >= 256 && NR <= 284 { printf "%5d %s\n", NR, $0 }' \
tests/test_template/test_lint_targets.py
echo '=== All make/bash lookup assignments and resolve calls ==='
rg -n -C 3 '(shutil\.which\("(make|bash)"\)|Path\(.*\)\.resolve\(\))' \
tests/test_template/test_lint_targets.py || true
echo '=== PR patch for affected test file ==='
git diff --unified=12 origin/main...HEAD -- tests/test_template/test_lint_targets.py || true
echo '=== PR `#54` status checks ==='
gh pr view 54 --repo leynos/agent-template-rust \
--json url,headRefOid,statusCheckRollup \
--jq '{url, headRefOid, checks: [.statusCheckRollup[] | {name, status, conclusion}]}' || trueLength of output: 6389
This remains a valid defect:
PR
You are interacting with an AI system. |
2 participants
Summary
This branch makes the generated-project Whitaker fallback tests hermetic by stripping inherited
WHITAKER,MAKEFLAGS,MFLAGS, andMAKELEVELvalues before invokingmake lint. It preserves the template Makefile behaviour and keeps the fallback cases focused on the intendedPATH,$HOME/.local/bin, and missing-tool branches.Closes #53.
Review walkthrough
Validation
WHITAKER=true uvx --with pytest-copier --with pyyaml --with syrupy --with make-parser --with hypothesis pytest -vv tests/test_template/test_lint_targets.py::test_makefile_resolves_whitaker_fallback: 3 passed.uvx --with pytest-copier --with pyyaml --with syrupy --with make-parser --with hypothesis pytest tests/: 50 passed, 1 skipped.coderabbit review --agent: first pass reported one docstring concern, which was fixed.coderabbit review --agent: second pass completed with 0 findings.Notes
No template or snapshot changes were needed because the failure was caused by inherited environment state in the test subprocess, not by the generated Makefile fallback chain.
References