Conversation
Reviewer's GuideThis PR extends the test suite by adding two end-to-end validation tests for generated projects—one ensuring the Makefile passes Sequence diagram for test execution with mbake validate and cargo clippysequenceDiagram
actor User
participant Pytest
participant GeneratedProject
User->>Pytest: Run pytest
Pytest->>GeneratedProject: Run mbake validate
GeneratedProject-->>Pytest: mbake validate result
Pytest->>GeneratedProject: Run cargo clippy
GeneratedProject-->>Pytest: cargo clippy result
Pytest-->>User: Display test results
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughUpdate the documentation to clarify and expand on the testing process, specifically regarding Makefile validation and Rust linting. Add two new test functions: one to validate the generated Makefile with Changes
Sequence Diagram(s)sequenceDiagram
participant Tester
participant Copier
participant Project
participant MakefileValidator as mbake
participant Clippy as cargo_clippy
Tester->>Copier: Render template with parameters
Copier->>Project: Create project files
Tester->>Project: Check for Makefile
Tester->>MakefileValidator: Validate Makefile
MakefileValidator-->>Tester: Validation result
Tester->>Copier: Render template with clippy params
Copier->>Project: Create project files
Tester->>Clippy: Run cargo clippy (all targets, all features, warnings as errors)
Clippy-->>Tester: Linting result
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (4)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- Add mbake to the test dependencies (e.g. requirements.txt) so the
mbake validatecommand can run without manual setup. - Either document or guard the Clippy test to ensure the Rust toolchain and
cargo clippyare available in CI, or skip it when they’re missing to avoid unexpected failures.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Add mbake to the test dependencies (e.g. requirements.txt) so the `mbake validate` command can run without manual setup.
- Either document or guard the Clippy test to ensure the Rust toolchain and `cargo clippy` are available in CI, or skip it when they’re missing to avoid unexpected failures.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
docs/testing.md(1 hunks)tests/test_template.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.md
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🪛 LanguageTool
docs/testing.md
[style] ~7-~7: Consider shortening or rephrasing this to strengthen your wording.
Context: ...renders correctly. Run the tests after making changes to the template: ```bash pip install -r r...
(MAKE_CHANGES)
🪛 Ruff (0.11.9)
tests/test_template.py
66-66: datetime.datetime.now() called without a tz argument
(DTZ005)
80-80: datetime.datetime.now() called without a tz argument
(DTZ005)
🔇 Additional comments (4)
docs/testing.md (2)
4-5: LGTM!The reformatting improves readability whilst maintaining the same information.
14-15: LGTM!The addition clearly describes the validation process and aligns with the new test functions added in the PR.
tests/test_template.py (2)
60-71: LGTM!The test correctly validates the generated Makefile using
mbake validateand follows the established pattern of the existing test functions.
74-84: LGTM!The test properly runs
cargo clippywith appropriate flags to treat warnings as errors, which aligns with the PR objectives for validation testing.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
README.md(1 hunks)copier.yaml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.md
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🔇 Additional comments (1)
copier.yaml (1)
1-1: Synchronise CI and dependency constraints with the new minimum Copier versionRaising
_min_copier_versionto9.0is sensible, yet the project’s CI workflow and any lock-files (e.g.requirements*.txt) still need to guaranteecopier>=9. Otherwise, pipelines may pick up an older cached 8.x build and violate the constraint at runtime.
Update all automated installation steps to pincopier>=9explicitly.
| The template requires **Copier 9.0** or later to avoid incompatibilities. | ||
|
|
||
| ## How to use | ||
|
|
||
| 1. Install Copier: `pip install copier`. | ||
| 1. Install Copier 9.0 or later: `pip install copier`. | ||
| 2. Run `copier copy gh:leynos/agent-template-rust <destination>`. |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enforce the ≥9.0 requirement in the installation command
The prose mandates Copier 9+, but the command pip install copier can still resolve to an older cached wheel when a developer already has Copier 7 installed. Replace the instruction with an explicit version specifier to guarantee compliance.
-1. Install Copier 9.0 or later: `pip install copier`.
+1. Install Copier 9.0 or later: `pip install "copier>=9"`.📝 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 template requires **Copier 9.0** or later to avoid incompatibilities. | |
| ## How to use | |
| 1. Install Copier: `pip install copier`. | |
| 1. Install Copier 9.0 or later: `pip install copier`. | |
| 2. Run `copier copy gh:leynos/agent-template-rust <destination>`. | |
| The template requires **Copier 9.0** or later to avoid incompatibilities. | |
| ## How to use | |
| 1. Install Copier 9.0 or later: `pip install "copier>=9"`. | |
| 2. Run `copier copy gh:leynos/agent-template-rust <destination>`. |
🤖 Prompt for AI Agents
In README.md around lines 7 to 12, the installation command for Copier does not
explicitly specify the minimum required version 9.0, which can lead to
installing an older version if cached. Update the pip install command to include
the version specifier "copier>=9.0" to enforce installing Copier version 9.0 or
later.
Summary
mbake validateon generated Makefilecargo clippyon generated templateTesting
pip install -r requirements.txtpytest -q(fails: lint error from cargo clippy)https://chatgpt.com/codex/tasks/task_e_68727965f70883229e0c9e6f8b00cd50
Summary by Sourcery
Add tests to validate the generated Makefile with mbake and enforce cargo clippy linting on the rendered project, and update the testing documentation accordingly.
Documentation:
Tests: