[kixelated]

Problem

The pinned cargo-nextest 0.9.146 ignores NEXTEST_NO_FAIL_FAST: two failing tests with one worker run only the first test, just like the unset default. Passing --no-fail-fast runs both.

Approach

• Translate a nonempty variable into --no-fail-fast in the shared sh/rs/nextest.sh launcher, for native nextest and llvm-cov nextest runs.
• Keep explicit runner options in control, avoid duplicate flags and --no-run conflicts, and insert before the test-filter -- boundary. Other commands keep their arguments.
• Verify real nextest behavior through the direct launcher and just rs test, extend the coverage caller's test, and remove the completed quest and its parent link.

Impact

• Public API: none.
• Wire: none.
• Unset and empty values keep fail-fast behavior. Test failures still fail the command.

Alternatives

The pinned runner options have no environment binding for fail-fast. No repository config or dispatcher argument overrides it.

just check dispatches to test-changed, the Rust selector, just rs test, and this same launcher. Changing the generic sh/local.sh gate or adding per-recipe flags would duplicate that shared source. The same-file gate change in moq-dev/moq#7710 is independent.

Implementation choices made unattended: treat a nonempty value as opt-in, consistent with the other shell controls; explicit command-line controls take precedence over the environment default.

Validation

• Before editing, the real two-failure fixture ran one test both unset and with NEXTEST_NO_FAIL_FAST=1; the explicit flag ran two. Each run returned nextest's failure exit code 100.
• Behavioral tests cover unset/empty/set values, explicit fail-fast and no-fail-fast aliases, both max-fail forms, compile-only runs, and the -- boundary. Coverage passes the flag to llvm-cov; non-nextest argument passthrough was checked separately.
• NEXTEST_NO_FAIL_FAST=1 quest gate just ci: passed on head 638707f1ae5ae408ef45a0f355110a6a35f83264 in 119 seconds, including 11,618 Rust tests and all selected repository tooling/lints. The known launcher-environment assertion did not reproduce. Flake checks were not selected by this diff, so no uid_map reproduction or full-flake pass is claimed.
• ShellCheck, shfmt, Ruff, git diff --check, and quest check --base origin/main: passed.
• Reproduced the review's empty-array failure on kixtop with stock Bash 3.2.57, then verified the fixed argument-dispatch block there for opt-in, compile-only, explicit no-fail-fast/max-fail, and -- cases. No full macOS suite is claimed.

Review

Claude Code / Claude Opus 5.5, medium effort, session d366cdb0-132c-4781-b1fa-c8477095bfa1, round 2 reviewed 638707f1ae5ae408ef45a0f355110a6a35f83264 against d66c8928339d51eb4c76925ced1490b37a0ec9be: no findings.

Round 1 reviewed bebbd78b6e49bd446803119ab3cb4063ea89f661. Both findings are fixed in the reviewed head:

• Replaced the empty Bash array with a scalar, verified on Bash 3.2.57.
• Matched the existing fixture's clippy-driver toolchain PATH selection.

The first attempt at the fixed-head review stopped before launching because Git fetch failed with git@github.com: Permission denied (publickey); ordinary fetch and push reproduced it. The same review resumed successfully with the authorized per-command HTTPS URL rewrite. Publication reuses existing gh credentials through a temporary credential helper. No persistent Git or remote settings changed.

Follow-ups

None. No open implementation, API, wire, or policy decisions.

(Written by GPT-6.1 Sol)

[kixelated]

Automated review by review (OpenAI)

Reviewed head 638707f1ae5ae408ef45a0f355110a6a35f83264.

I found no actionable issue. The shared launcher inserts the opt-in flag for both repository runner shapes, preserves explicit fail-fast and max-fail controls, avoids the compile-only conflict, and stops scanning at the test-filter boundary. The new tests exercise the direct and just rs test paths, with the coverage caller checked separately.

Verification: GitHub-only static review of the launcher, its callers, tests, and pinned nextest option definitions. I did not run the test suite or reproduce the macOS behavior.

(Written by review (OpenAI))

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

When NEXTEST_NO_FAIL_FAST is nonempty, nextest.sh adds --no-fail-fast to supported commands unless the arguments include a recognized fail-fast, max-fail, or --no-run option. Tests cover both Rust test command paths and coverage arguments. The CI planning document and README entry describing the setting are removed.

Merge Risk: Minimal · up to 63870

The launcher change preserves explicit runner options and has targeted behavior coverage. No actionable merge-blocking issue remains.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title names the primary change: Rust test runs now honor NEXTEST_NO_FAIL_FAST. The raw summary and objectives confirm the shared sh/rs/nextest.sh launcher translates the variable into --no-fail-fa…
Description check		The description states the problem (pinned cargo-nextest 0.9.146 ignores NEXTEST_NO_FAIL_FAST), the approach (translating a nonempty variable into --no-fail-fast in sh/rs/nextest.sh), the impact, alte…
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 primary change: Rust test runs now honor NEXTEST_NO_FAIL_FAST. The raw summary and objectives confirm the shared sh/rs/nextest.sh launcher translates the variable into --no-fail-fast for nextest and llvm-cov nextest runs. The title is specific and concise, and it follows a conventional commit style.

Full details: Description check

Explanation

The description states the problem (pinned cargo-nextest 0.9.146 ignores NEXTEST_NO_FAIL_FAST), the approach (translating a nonempty variable into --no-fail-fast in sh/rs/nextest.sh), the impact, alternatives, and validation steps. It matches the changeset in sh/rs/nextest.sh, sh/rs/test-lints.test.py, sh/coverage.test.py, and the removed quest documents. It is detailed and on-topic.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. 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 2 functions across 3 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Simplify code

• Commit to this branch
• 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

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 90a0998e-bd47-444b-839f-42341216a24e

Commits

Reviewing files that changed from the base of the PR and between d66c892 and 638707f.

Files selected for processing: 5

• quest/m0/ci/README.md
• quest/m0/ci/no-fail-fast.md
• sh/coverage.test.py
• sh/rs/nextest.sh
• sh/rs/test-lints.test.py

Files with no reviewable changes: 1

• quest/m0/ci/no-fail-fast.md

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

[pullfrog]

✅ No new issues found.

Reviewed changes

Reviewed the full 638707f diff against d66c892: the launcher change, its behavioral tests, and the quest cleanup.

• NEXTEST_NO_FAIL_FAST translation — sh/rs/nextest.sh injects --no-fail-fast into native and llvm-cov nextest runs when the variable is nonempty, while explicit runner options and post--- filters stay in control.
• Behavioral coverage — sh/rs/test-lints.test.py drives a real two-failure fixture through both just rs test and the launcher across unset/empty/set, explicit fail-fast/no-fail-fast/max-fail, and compile-only cases; sh/coverage.test.py asserts the flag reaches cargo llvm-cov nextest.
• Quest cleanup — removes the completed quest/m0/ci/no-fail-fast.md and its README entry.

I also re-ran the positional-slice block on Bash 3.2.57 built from source: it works with both 3 and 5 arguments under set -euo pipefail, so the earlier empty-array concern is resolved.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏}