Fall Back to the Default Search Path in the Archived End-to-End Test - #1863
Conversation
EndToEndCase put its gh stub first on a PATH built from
os.environ.get('PATH', ''), so with PATH unset the copied configure.sh
ran with only the stub directory and could not find jq, although
require() had found it through the default search path. It now falls
back to os.defpath, as the dotnet-leg test does.
Raised by Copilot on the promotion PR #1850.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Repository: ptr727/ProjectTemplate/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 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 |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The harness now uses os.defpath, but require() relies on shutil.which()'s CS_PATH fallback when PATH is unset, so the test can still fail on platforms where CS_PATH differs from defpath.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
This PR updates the archived-repository end-to-end test harness so that when PATH is unset, the subprocess running the copied configure.sh still has a usable search path (with the gh stub prepended), preventing false failures due to missing jq.
Changes:
- Change the test harness
PATHfallback from an empty string toos.defpathwhenPATHis unset.
| File | Description |
|---|---|
| scripts/tests/test_configure_archived.py | Adjusts the e2e harness environment PATH construction for PATH-unset runs. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…and Overnight Fixes to Main (#1850) Promotes `develop` to `main`. ## Carried - #1846: exits `repo-config/configure.sh` early, writing nothing, for a repository whose registry `status` is `archived`. - #1848: fails the validator's C# unit test step when the run wrote no non-empty Cobertura report, clearing `./coverage` first. It declines #1134's `ref` input with evidence, since a bare checkout already validates `github.sha`, and D1.2 now says so. #1134's third gap moved to #1800. - #1851: asserts that the two planted registration defects are themselves reported. - #1854, #1858: correct #1846's test docstrings, which claimed the script makes no `gh` call before the archived check in cases where it does. Raised by Copilot on this pull request. - #1856: passes `--repo` on documented handoff commands and guards handoff reads against a full page, per #1847. - #1859, #1863: fall back to the default search path when `PATH` is unset in #1848's and #1846's test harnesses. Raised by Copilot on this pull request. - #1861: scopes `VerifyReferenceAotCompatibility` to an AOT publish in `dotnet-codestyle`, per #1857. - #1867: establishes the `PATH` order `tool_shadow_path` names, per #1644. - #1870: makes the installer's dirty-checkout tests independent of the real checkout's state, per #1641. - #1873: pins and decodes git's quoting in `repo_gate.py`'s `ls-files` read, per #1580 and #1872. - #1878: folds typographic punctuation in `pr_review.py reply --match`, per #1299. - #1883: routes `configure.sh`'s `ruleset_id()` through `jqr`, per #1253. - #1885: states the pin comment as the release tag and defines `$/`, per #1805. - #1888: distinguishes `./` from `$/` resolution in the pin rule's prose, per #1886. - #1893: drops the issue reference from `repo-config/README.md`'s archived-exemption note, which Copilot flagged on six rounds of this pull request. - #1895: describes IL3058 in `dotnet-codestyle` as a referenced assembly lacking `IsAotCompatible` metadata set to `true`, and drops the unversioned package examples. Raised by CodeRabbit on this pull request. Callers that pin a hub release get the new C# check on their next pin bump. A test project that runs `dotnet test --coverage` without writing a report now fails its step rather than passing silently. Closes #1134 Closes #1847 Closes #1857 Closes #1644 Closes #1641 Closes #1580 Closes #1872 Closes #1299 Closes #1253 Closes #1805 Closes #1886 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Backlog counts and rankings now consistently exclude handoff issues, including those also marked blocked. * Repository configuration commands now exit without writing when a repository is archived. * Review-thread matching handles typographic punctuation, and no-match responses report the unresolved-thread count. * Tool setup handles PATH entries more precisely, and repository checks report unusual file paths without crashing. * **Reliability** * Validation now fails when C# or Python tests produce no coverage report. * Agent setup can use an explicit dirty-checkout override. * Workflow and repository guidance clarifies reference resolution, release-tag pinning, and AOT configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai -->

Fixes a finding Copilot raised on the promotion PR #1850, against #1846's test.
EndToEndCase.run_configureputs aghstub first on aPATHbuilt fromos.environ.get('PATH', ''). WithPATHunset,require("bash", "jq")still finds both tools through the default search path, but the copiedconfigure.shthen runs with only the stub directory on itsPATHand can't findjq. The fallback is nowos.defpath, matching the dotnet-leg test's fix in #1859. WithPATHset to an empty string,require()skips the case, so nothing changes there.Verification: under
env -u PATH, the base commit fails two tests and the head passes all seven. The suite passes withPATHset. One local strict review pass is recorded, with no findings.Three older harnesses use the same empty fallback in files this promotion doesn't touch. They're filed as #1862.
🤖 Generated with Claude Code