docs(agents): say which runner the EXIT-trap failure mode needs - #3779
docs(agents): say which runner the EXIT-trap failure mode needs#3779myasnikovdaniil wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe E2E testing guide clarifies ChangesBATS execution guidance
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This is a localized documentation clarification with no production behavior change; no actionable merge-blocking risk remains beyond normal checks and review. 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/agents/e2e-testing.md (1)
48-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNarrow the repository-wide
batsclaim.The repository contains
#!/usr/bin/env batsshebangs and documents manualbatscommands. State that the automated CI and Makefile paths do not invokebatsdirectly.🤖 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 `@docs/agents/e2e-testing.md` at line 48, Update the documentation’s repository-wide runner claim to acknowledge the existing `#!/usr/bin/env bats` shebangs and documented manual `bats` commands, while explicitly stating that automated CI and Makefile paths invoke `hack/cozytest.sh` rather than `bats` directly. Keep the distinction between manual local execution and automated paths clear.
🤖 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.
Nitpick comments:
In `@docs/agents/e2e-testing.md`:
- Line 48: Update the documentation’s repository-wide runner claim to
acknowledge the existing `#!/usr/bin/env bats` shebangs and documented manual
`bats` commands, while explicitly stating that automated CI and Makefile paths
invoke `hack/cozytest.sh` rather than `bats` directly. Keep the distinction
between manual local execution and automated paths clear.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8b438ba2-f6bd-41f3-b48b-bf7967288522
📒 Files selected for processing (1)
docs/agents/e2e-testing.md
§3 bans test-level EXIT traps because such a trap displaces the handler the bats binary installs, so a failing test prints no TAP line and a grep for `not ok` reads a green suite. That is true, and it is unreachable in CI. hack/cozytest.sh runs each @test inside an explicit subshell and reads the status out of band on the next line, so a trap in a test body fires when that subshell exits and takes nothing with it. The real bats binary is invoked nowhere in the tree -- make bats-unit-tests and every recipe in packages/core/testing/Makefile go through cozytest.sh. The disappearing `not ok` needs someone running bats by hand. Worth writing down because the rule has cost four PRs and three successive mechanisms, and the omission reads as an unpaid backlog: 68 declared traps that a reader assumes CI is one refactor away from needing. It is a review aid over a hazard outside CI. The note also records that two of the covered files resist a mechanical conversion, so the next person to try does not discover it halfway. No other line of the document changes. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
04f4dcf to
c3b80ea
Compare
§3 bans test level EXIT traps because such trap replaces the handler bats binary installs, so a failing test prints no TAP line at all and grep for
not okreads a green suite. That is true, and it does not happen in CI.hack/cozytest.shruns each@testinside explicit subshell and reads the status out of band on the next line, so a trap in a test body fires when that subshell exits and takes nothing with it. Real bats binary is not called anywhere in this tree,make bats-unit-testsand every recipe inpackages/core/testing/Makefilego through cozytest. Disappearingnot okneeds someone running bats by hand on a file.Worth writing down because the rule already cost four PRs and three successive mechanisms, and the missing note reads as unpaid debt, 68 declared traps that a reader assumes CI is one refactor away from needing. The bullet also records that two of the covered files resist a mechanical conversion, one body cd's into its scratch dir and never returns so an end of body cleanup would delete its own cwd, another has its last command deciding the verdict. Next person to try that should not find it halfway through.
Summary by CodeRabbit