Recovered: feat: support TESTSPRITE_PROJECT_ID default (#144 by @naufalfx805-source)#249
Conversation
WalkthroughThe CLI now falls back to ChangesProject ID fallback
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 `@src/commands/test.ts`:
- Around line 7471-7475: Update the test run --all command’s option description
and usage examples to state that the project ID may come from --project or
TESTSPRITE_PROJECT_ID, including that the explicit --project value takes
precedence. Keep the requireProjectId validation in the handler aligned with
this documented behavior and ensure user-facing help no longer says --project is
mandatory.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b27c056e-6cd7-4f12-b55c-1a32d6a73d49
📒 Files selected for processing (4)
DOCUMENTATION.mdsrc/commands/test.run.spec.tssrc/commands/test.test.tssrc/commands/test.ts
| const projectId = resolveProjectId(cmdOpts.project, deps); | ||
| requireProjectId( | ||
| projectId, | ||
| '--all requires a project id - pass --project <id> or set TESTSPRITE_PROJECT_ID', | ||
| ); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update test run --all help text for the environment fallback.
The handler now accepts --project or TESTSPRITE_PROJECT_ID, but the command descriptions still say --project is required. This contradicts the new validation behavior and hides the supported environment-based invocation; update the option description and examples to mention both sources and flag precedence.
As per path instructions, CLI correctness includes clear user-facing error and usage behavior.
🤖 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 `@src/commands/test.ts` around lines 7471 - 7475, Update the test run --all
command’s option description and usage examples to state that the project ID may
come from --project or TESTSPRITE_PROJECT_ID, including that the explicit
--project value takes precedence. Keep the requireProjectId validation in the
handler aligned with this documented behavior and ensure user-facing help no
longer says --project is mandatory.
Source: Path instructions
|
Hi @naufalfx805-source 👋 — this is the recovery of your PR #144 , which was auto-closed by an error in our 2026-07-09 release process. Your commits are intact — thank you for the contribution! It currently shows merge conflicts because Option A — merge (no force-push): git checkout BRANCH
git fetch upstream # your remote for TestSprite/testsprite-cli (may be "origin")
git merge upstream/main # resolve conflicts, commit
git pushOption B — rebase (cleaner history, needs force): git checkout BRANCH
git fetch upstream
git rebase upstream/main # resolve conflicts
git push --force-with-leaseOnce you push, the conflicts here resolve automatically and we'll take it into review. Sorry again for the extra step! |
|
Maintainer decision on the resolution-chain layering, since this PR and #225 currently collide: precedence is @naufalfx805-source: the implementation here is right (trim-guarded flag-then-env, and main has no equivalent today — the only other TESTSPRITE_PROJECT_ID reference is scaffold's placeholder fill). It just needs a rebase onto current main (branch is conflicting post-v0.4.0). Rebase and we'll merge. |
c70e2ad to
6495138
Compare
|
Thanks for the PR, @jangjos-128! A quick note on our workflow: for features and behavior changes we require contributors to open an issue first, claim it by commenting |
github-actions
Bot
added
the
needs-issue
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 `@DOCUMENTATION.md`:
- Line 608: Update the documentation entries for `test list`, `test create`, and
`test run --all` to state that `--project` may be omitted when
TESTSPRITE_PROJECT_ID is set, while remaining required otherwise. Ensure all
equivalent command descriptions consistently reflect this 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 87d444d1-6563-4da4-a0f4-03879c475e56
⛔ Files ignored due to path filters (1)
test/__snapshots__/help.snapshot.test.ts.snapis excluded by!**/*.snap,!**/*.snap
📒 Files selected for processing (4)
DOCUMENTATION.mdsrc/commands/test.run.spec.tssrc/commands/test.test.tssrc/commands/test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/commands/test.run.spec.ts
- src/commands/test.test.ts
- src/commands/test.ts
| | `TESTSPRITE_API_KEY` | API key - overrides the credentials file | | ||
| | `TESTSPRITE_API_URL` | API endpoint - overrides the credentials file | | ||
| | `TESTSPRITE_PROFILE` | Active profile (below `--profile`, above `default`) | | ||
| | `TESTSPRITE_PROJECT_ID` | Default project for `test list`, `test create`, and `test run --all` when `--project` is omitted | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Align the command reference with the new fallback.
The test list section still states that --project is required, contradicting this new TESTSPRITE_PROJECT_ID fallback. Update that command description—and any equivalent create/run-all wording—to explain when the flag may be omitted.
🤖 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 `@DOCUMENTATION.md` at line 608, Update the documentation entries for `test
list`, `test create`, and `test run --all` to state that `--project` may be
omitted when TESTSPRITE_PROJECT_ID is set, while remaining required otherwise.
Ensure all equivalent command descriptions consistently reflect this fallback.
|
Rebased onto current I also addressed the Local validation on the rebased branch:
The regular CI checks are green after the push. One remaining blocker is the |
3 participants
Refs #76
Discord: npall_805
Summary:
Validation:
Summary by CodeRabbit
TESTSPRITE_PROJECT_IDas the default project for test listing, creation, and running all tests when--projectis omitted.--projectis provided (including a non-blank value), it continues to take precedence.TESTSPRITE_PROJECT_ID, including cases where--projectis missing or blank.DOCUMENTATION.mdto documentTESTSPRITE_PROJECT_IDdefault behavior and refreshed related table formatting.