Repository navigation
Run CLI tests against the checkout - #159
Conversation
Coding-Agent: Codex Codex-Version: codex-cli 0.149.0 Model: gpt-5.6-sol Reasoning-Effort: xhigh
|
Warning Review limit reachedNext included review available in 49 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
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 |
njzjz-bot
left a comment
There was a problem hiding this comment.
Review result: one medium-severity issue found.
The replacement removes dependence on the dargs console script in PATH, but it still does not reliably pin child processes to the checkout. See the inline finding.
I reproduced seven CLI-test failures when unittest discovery was launched outside the repository: the parent imported the checkout, while child python -m dargs commands selected an older installed package. Root-directory CI remains green.
Coding agent: Codex
Codex version: codex-cli 0.149.0
Model: gpt-5.6-sol
Reasoning effort: xhigh
| from pathlib import Path | ||
|
|
||
| this_directory = Path(__file__).parent | ||
| DARGS_COMMAND = [sys.executable, "-m", "dargs"] |
There was a problem hiding this comment.
[P2] Pin subprocess module resolution to the checkout
python -m dargs resolves from the child process's working directory, not from the parent test runner's in-memory sys.path, so this constant does not reliably select the checkout. Running unittest discovery from /tmp imported these tests from the PR checkout in the parent, but the subprocesses selected an older installed dargs; I reproduced four errors and three failures with the old CLI that only exposed check. Please run the CLI subprocesses with cwd=this_directory.parent (preferably through a helper), or explicitly prepend the repository root to each child's PYTHONPATH.
njzjz-bot
left a comment
There was a problem hiding this comment.
Reviewed the CLI test command consolidation and ran the PR test_cli suite after installing the archived revision; all eight CLI tests pass. Using the current interpreter to invoke the module makes the tests exercise the checkout consistently. Package, type-check, CodeQL, docs, pre-commit, and Codecov checks pass; the mirror job is unrelated.
This PR is authored by njzjz-bot, so GitHub does not permit this account to approve or request changes. I am recording the review as COMMENTED.
Coding agent: Codex
Codex version: codex-cli 0.151.0
Model: gpt-5.6-sol
Reasoning effort: xhigh
njzjz-bot
left a comment
There was a problem hiding this comment.
Reviewed the CLI test command change. The shared module command consistently targets the checkout and the focused test logic remains intact; no blocking correctness, security, regression, or test issues found. Because njzjz-bot authored this PR, GitHub disallows APPROVE/REQUEST_CHANGES, so this review is COMMENTED.\n\nCoding agent: Codex\nCodex version: codex-cli 0.151.0\nModel: gpt-5.6-sol\nReasoning effort: xhigh
njzjz-bot
left a comment
There was a problem hiding this comment.
Reviewed the implementation and relevant tests. No blocking correctness, security, or regression issue found.
Self-review restriction: this PR is authored by njzjz-bot, so GitHub does not permit APPROVE or REQUEST_CHANGES; recording this review as COMMENT.
Validation: after generating the package version artifact, pytest -q tests/test_cli.py (8 passed); GitHub checks pass.
Coding agent: Codex
Codex version: codex-cli 0.151.0
Model: gpt-5.6-sol
Reasoning effort: xhigh
|
Three independent reviewers completed a full review of this PR. One P2 test-isolation issue was confirmed (matching the existing inline feedback at tests/test_cli.py:9): python -m dargs can import an installed package when discovery runs outside the checkout, so the subprocesses should pin cwd or PYTHONPATH. No other findings. Coding agent: Codex |
Run module-based CLI subprocesses from the repository root so tests cannot silently exercise an installed dargs package during out-of-tree discovery. Coding-Agent: Codex Codex-Version: codex-cli 0.151.0 Model: gpt-5.6-sol Reasoning-Effort: xhigh
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #159 +/- ##
==========================================
- Coverage 75.14% 74.25% -0.90%
==========================================
Files 9 9
Lines 853 870 +17
==========================================
+ Hits 641 646 +5
- Misses 212 224 +12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Summary
Fixes #123.
Validation
AI attribution
Coding agent: Codex
Codex version: codex-cli 0.149.0
Model: gpt-5.6-sol
Reasoning effort: xhigh