[protonjls]

What does this PR do?

test run <test-id> --allow-empty was accepted and silently ignored. --allow-empty only softens the zero-dispatch gate of a batch run, which exits 5 when a batch dispatches no tests. A single <test-id> never reaches that gate, so the flag had no effect: the run was triggered as if it weren't there. test rerun already refuses --allow-empty without --all, and test run already refuses --filter without --all.

This is part 2 of #376:

• One check in the test run action, right after the existing --filter / --status checks. --allow-empty with a single <test-id> and no --all is refused before any request, with exit 5 VALIDATION_ERROR, details.field allow-empty, and the nextAction "Flag --allow-empty is invalid: --allow-empty only applies with --all or several test ids (it permits a batch that dispatches zero tests). Remove --allow-empty, or add --all with --project or TESTSPRITE_PROJECT_ID."
• Several test ids are left alone, as I noted on [Hackathon] fix(test): flags documented "with --history / --run / --all" are silently ignored without that flag (their siblings exit 5) #376. Since v0.16.0 they run as a batch, which reaches the same zero-dispatch gate as --all (for example when the batch answer skips every id), whose error says "pass --allow-empty to exit 0 intentionally". The flag works there, so refusing it would be a regression. If you'd rather refuse it whenever --all is absent, exactly like test rerun, I'll change it.
• DOCUMENTATION.md gets one sentence in the test run section.

command	before (main, 4b5ae55 / v0.16.0)	after
test run t1 --allow-empty (also with --wait, --local 3000, or t1 t1)	run triggered, flag ignored	exit 5, details.field allow-empty, no request
test run --all --project P --allow-empty	accepted	unchanged
test run a b --allow-empty (several ids, a batch)	accepted	unchanged

Related issue

Part of #376

Thanks @zeshi-du for confirming #376 and splitting it into three PRs. This is part 2; part 1 is #598 and part 3 is tracked in #569.

Season 4 CLI Improvement Bonus

• I'm entering this PR for the Season 4 CLI Improvement Bonus

Discord username: 1149447580477042769

Type of change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that changes existing behavior)
• Documentation only
• Build / CI / chore

Checklist

• PR targets the main branch.
• Commits follow Conventional Commits
  (feat(...), fix(...), docs(...), …).
• npm run lint and npm run format:check pass.
• npm run typecheck passes.
• npm test passes and coverage stays at or above the 80% gate.
• New behavior is covered by unit tests (mock-based; no network or
  credentials required).
• No secrets, API keys, internal endpoints, or personal data are included.
• User-facing changes are reflected in README.md / DOCUMENTATION.md where
  relevant.

Notes for reviewers

• Tests (src/commands/test.run.spec.ts, next to <test-id> --filter (without --all) → exit 5):
  • One case per single-test form through createTestCommand: plain, --wait, --local 3000 --skip-preflight, and a repeated id (test_xyz test_xyz, which runs one test). Each asserts exit 5, VALIDATION_ERROR, details.field, the exact nextAction, and zero fetch calls.
  • Control: run --all --project project_be --allow-empty against an empty batch still exits 0 ("--allow-empty set — exiting 0").
• Control for the several-id batch (src/commands/test.run-ids.spec.ts, in "run : project grouping"): run a b whose batch answer skips every id exits 5 with "pass --allow-empty to exit 0 intentionally" without the flag. With it, it exits 0 after the one batch request.
• With the two spec files applied to main (4b5ae55, v0.16.0), exactly the 4 refusal cases fail, because the requests go out. Both controls pass on both commits.
• Compatibility: a script that passes --allow-empty with a single test id now gets exit 5 instead of a run. The nextAction names the fix. This is the same trade test rerun --allow-empty and test run --filter already make.
• End to end (built CLI against a request-logging mock): on main, test run test_x --allow-empty --target-url https://example.com --skip-preflight --no-wait sends the trigger request and exits 0, the same as without the flag. On this branch it exits 5 and sends nothing. Single-test runs without the flag, and --all --project P --allow-empty --dry-run, give the same exit code, request count and stdout on both commits.
• The --allow-empty help text is unchanged; it already reads "with --all: …".
• Local checks on this commit (Node 24): npm run lint, npm run format:check, npm run typecheck, the .github/scripts tests, build + smoke, npm run test:coverage and npm run test:e2e all pass.
• Rebased onto v0.16.0 (4b5ae55). v0.16.0 runs every several-id test run as a batch, so the old several-id control (backend ids through the automatic tunnel) no longer applies; it is replaced by the batch control above. The guard itself is unchanged, and all the checks above were re-run on the rebased commit.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • test run now rejects --allow-empty when used with exactly one distinct test ID, including when combined with --wait or --local. The command reports a validation error before making a network request.
  • Empty batch runs using --all --allow-empty or multiple test IDs with --allow-empty remain accepted.

• Documentation

  • Clarified that --allow-empty applies to batches, not a single test ID.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: TestSprite/testsprite-cli
• Branch: main
• File: .coderabbit.yaml
• Commit: 4b5ae55
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 6368f106-0aa3-492e-817e-becacd545d9f

Commits

Reviewing files that changed from the base of the PR and between 4eeae1b and 43be289.

Files selected for processing: 4

• DOCUMENTATION.md
• src/commands/test.run-ids.spec.ts
• src/commands/test.run.spec.ts
• src/commands/test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

Walkthrough

The test run command now rejects --allow-empty when exactly one distinct test ID is selected without --all. Tests cover this validation and confirm that empty batches remain accepted with --allow-empty. Documentation describes the restriction.

Changes

Empty batch handling

Layer / File(s)	Summary
Batch-only flag validation and coverage src/commands/test.ts, src/commands/test.run.spec.ts, src/commands/test.run-ids.spec.ts, DOCUMENTATION.md	The command rejects --allow-empty for one distinct test ID without --all. Tests cover rejection before network access and accepted empty batches for --all and multiple IDs. Documentation describes the restriction.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related issues

• Issue 376 identifies test run <id> --allow-empty as an ignored flag. This change adds validation for that command and flag.

Merge Risk: Minimal · up to 43be2

The change rejects the ignored flag for single-test runs while retaining it for batches. No merge-blocking issue is identified.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the main change: test run now refuses --allow-empty when a single test ID is given without --all. The raw summary and objectives confirm this is the primary change, with the vali…
Docstring Coverage		No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title names the main change: test run now refuses --allow-empty when a single test ID is given without --all. The raw summary and objectives confirm this is the primary change, with the validation added in src/commands/test.ts, tests added, and one documentation sentence updated. The title's parenthetical notes that only batches read the flag, which matches the change. The title is specific and sufficiently descriptive.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files. (2 skipped: 1 unsupported, 1 too large.)

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[github-actions]

🏆 Season 4 CLI Improvement Bonus: entry recorded. Thanks, @protonjls!

To be counted, this PR needs the following by Oct 29, 11:59 PM PT:

• ✅ A linked issue assigned to you.
• ✅ Out of draft.
• ✅ A Discord username in the description.

Rewards go to merged PRs judged a genuine improvement, and up to three bonus-eligible PRs count per person. Untick the box to withdraw the entry. This comment refreshes when you edit the PR. Full rules.

[protonjls]

Rebased onto v0.16.0 (4b5ae55) to clear the conflict with the release. The guard is unchanged. v0.16.0 runs every several-id test run as a batch, so I replaced the several-id control: it now covers a batch whose answer skips every id (exit 5 without --allow-empty, exit 0 with it) in test.run-ids.spec.ts. The full local suite passes on the new head, and the 4 refusal cases still fail on v0.16.0 without the fix. PR description updated.

[zeshi-du]

Thanks @protonjls, and sorry for the slow answer to the scoping question you raised on #376. You were right to raise it: since 0.16 a run with several ids is a batch that honours --allow-empty, so refusing the flag only when exactly one id is given would make test run $IDS --allow-empty run or exit 5 depending on how many ids the variable holds. We would rather leave it a harmless no-op on a single id, so we are dropping item 2 of #376 and closing this. Nothing is wrong with the code. #598 (item 1) is going in.

[protonjls]

Thanks @zeshi-du, that makes sense: a flag whose effect depends on how many ids $IDS holds would be worse than a no-op on a single id. Glad #598 is going in.