feat: add standard test flowchart#11
Conversation
WalkthroughRewrites .roo/rules-test/rules-test.md to replace a single-phase testing rule with a two-phase workflow: Phase 1 guides creation/validation of a standardized test runner script (.cospec/scripts/test_api.sh) with templates and prompts; Phase 2 outlines generating and associating test cases. Updates examples, preface, and cross-references accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Rules as Rules Doc (rules-test.md)
participant Script as test_api.sh
participant App as App Server
participant Tests as Test Runner
User->>Rules: Read two-phase workflow
rect rgba(200,235,255,0.3)
note over Rules: Phase 1 — Environment bootstrap & validation (new)
Rules-->>User: Prompt to create/validate .cospec/scripts/test_api.sh
alt Script missing
User->>Rules: Choose create or skip
opt Create
Rules-->>User: Provide script template (build/start/port-check/test/cleanup)
User->>Script: Save and customize placeholders
end
opt Skip
Rules-->>User: Terminate flow
end
else Script present
User->>Script: Execute script
Script->>App: Build & start server
Script-->>Script: Wait for APP_PORT ready
Script->>Tests: Run TEST_COMMAND
Script-->>App: Cleanup via trap on exit
end
end
rect rgba(210,255,210,0.3)
note over Rules: Phase 2 — Test case generation (new)
User->>Rules: Generate cases per task (<=5, functional only)
Rules-->>User: Link cases to Phase 1 outputs and doc examples
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.roo/rules-test/rules-test.md (2)
7-17: Adjust heading level hierarchyThe document jumps from an H1 directly to an H3, which violates our Markdown style rule (MD001) and will fail linting. Please promote this heading to
##.-### **第一阶段:测试环境引导与校验** +## **第一阶段:测试环境引导与校验**
133-139: Annotate fenced code block with a languageThis fenced block lacks a language hint, triggering MD040. Add a language (e.g.,
markdown) ortextso the linter passes.-``` +```markdown - [ ] 1.1 创建【资源】API端点 - 实现GET、POST、PUT、DELETE操作 - 添加请求验证和清理 - _需求:[参考具体需求]_ - _测试:[参考具体测试功能点、测试命令]_</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between e368f630a6b3df90fa1c3e852709cab1b33c7b94 and 0fd9631473df96149d067150542be27c0c3754ab. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `.roo/rules-test/rules-test.md` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>.roo/rules-test/rules-test.md</summary> 7-7: Heading levels should only increment by one level at a time Expected: h2; Actual: h3 (MD001, heading-increment) --- 133-133: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| # The command to start the application server in the background. | ||
| # It must run in the background (using '&'). | ||
| # Example: './build/app &' or 'npm start &' | ||
| START_SERVER_COMMAND="" | ||
| # The port your application is listening on. Used to check if the server is ready. | ||
| APP_PORT=8080 | ||
| # The command to run the API tests. | ||
| # Example: 'go test ./...' or 'jest' or another test runner command. | ||
| TEST_COMMAND="" | ||
|
|
There was a problem hiding this comment.
Fix background command guidance
The template currently tells users to append & to START_SERVER_COMMAND, but the script already backgrounds the command with setsid … &. If they follow the instruction, the line expands to setsid <cmd> & &, which throws a syntax error and the server never starts. Please update the guidance/examples to omit the trailing ampersand so the generated script is runnable.
-# The command to start the application server in the background.
-# It must run in the background (using '&').
-# Example: './build/app &' or 'npm start &'
+# The command to start the application server.
+# Do not append '&'; the script backgrounds the command automatically.
+# Example: './build/app' or 'npm start'
START_SERVER_COMMAND=""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # The command to start the application server in the background. | |
| # It must run in the background (using '&'). | |
| # Example: './build/app &' or 'npm start &' | |
| START_SERVER_COMMAND="" | |
| # The port your application is listening on. Used to check if the server is ready. | |
| APP_PORT=8080 | |
| # The command to run the API tests. | |
| # Example: 'go test ./...' or 'jest' or another test runner command. | |
| TEST_COMMAND="" | |
| # The command to start the application server. | |
| # Do not append '&'; the script backgrounds the command automatically. | |
| # Example: './build/app' or 'npm start' | |
| START_SERVER_COMMAND="" | |
| # The port your application is listening on. Used to check if the server is ready. | |
| APP_PORT=8080 | |
| # The command to run the API tests. | |
| # Example: 'go test ./...' or 'jest' or another test runner command. | |
| TEST_COMMAND="" |
🤖 Prompt for AI Agents
In .roo/rules-test/rules-test.md around lines 36 to 45, the guidance incorrectly
instructs users to append '&' to START_SERVER_COMMAND even though the runner
already backgrounds the command (setsid … &), causing double ampersands and a
syntax error; remove any text that tells users to add '&', update the examples
to show commands without a trailing ampersand (e.g., './build/app' or 'npm
start'), and replace the line saying "It must run in the background (using
'&')." with a note that the command should be a foreground-starting command (no
trailing '&') because the script backgrounds it for you.
Summary by CodeRabbit