Conversation
Reviewer's GuideThis PR augments the Cucumber tests to verify the default cooldown_period_seconds by adding new step definitions for missing cooldown settings and corresponding assertions, updating the feature file with a dedicated scenario, and tidying up clippy annotations in config_steps.rs. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Reviews pausedUse the following commands to manage reviews:
Summary by CodeRabbit
WalkthroughAdd a Cucumber scenario and step definitions to verify the default value of cooldown_period_seconds (960) when omitted from the config. Reorder two test attributes; no production code or public API changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Runner as Cucumber Runner
participant Steps as Step Definitions
participant Loader as Config Loader
participant World as Test World
Runner->>Steps: Given a configuration file with token "abc" and no cooldown_period_seconds
Steps->>Steps: Call helper to write config with token only
Steps->>Loader: Trigger config load
Loader-->>World: Set world.result = Ok(Config{ cooldown_period_seconds: 960, ... })
Runner->>Steps: Then cooldown_period_seconds is 960
Steps->>World: Take world.result
Steps->>Steps: Assert cfg.cooldown_period_seconds == 960
Steps-->>Runner: Return assertion result
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
tests/features/config.feature(1 hunks)tests/steps/config_steps.rs(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Clippy warnings MUST be disallowed.
Fix any warnings emitted during tests in the code itself rather than silencing them.
Where a function is too long, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
Where a function has too many parameters, group related parameters in meaningfully named structs.
Where a function is returning a large error consider using Arc to reduce the amount of data returned.
Write unit and behavioural tests for new functionality. Run both before and after making any change.
Every module must begin with a module level (//! ) comment explaining the module's purpose and utility.
Document public APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Prefer immutable data and avoid unnecessary mut bindings.
Handle errors with the Result type instead of panicking where feasible.
Avoid unsafe code unless absolutely necessary and document any usage clearly.
Place function attributes after doc comments.
Do not use return in single-line functions.
Use predicate functions for conditional criteria with more than two branches.
Lints must not be silenced except as a last resort.
Lint rule suppressions must be tightly scoped and include a clear reason.
Prefer expect over allow.
Use rstest fixtures for shared setup.
Replace duplicated tests with #[rstest(...)] parameterised cases.
Prefer mockall for mocks/stubs.
Prefer .expect() over .unwrap().
Use concat!() to combine long string literals rather than escaping newlines with a backslash.
Prefer semantic error enums. Derive std::error::Error (via the thiserror crate) for any condition the caller might inspect, retry, or map to an HTTP status.
Use an opaque error only at the app boundary. Use eyre::Report for human-readable logs; these should not be exposed in public APIs.
Never export the opaque type from a library. Convert to domain enums at API boundaries, and to eyre only in the main main() entrypoint or top-level async task.
Files:
tests/steps/config_steps.rs
⚙️ CodeRabbit configuration file
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Where code is only used by specific features, it must be conditionally compiled or a conditional expectation for unused_code applied.
Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Environment access (env::set_var and env::remove_var) are always unsafe in Rust 2024 and MUST be marked as such
- For testing of functionality depending upon environment variables, dependency injection and the
mockablecrate are the preferred option.- If mockable cannot be used, env mutations in tests ...
Files:
tests/steps/config_steps.rs
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (4)
tests/features/config.feature (1)
35-38: Accept: DEFAULT_COOLDOWN is 960DEFAULT_COOLDOWN is set to 960 in crates/comenqd/src/config.rs (const DEFAULT_COOLDOWN: u64 = 960) and Config.cooldown_period_seconds uses #[serde(default = "default_cooldown")], so the test expecting 960 matches the implementation.
tests/steps/config_steps.rs (3)
35-36: Keep targeted lint expectations co-located with step attributes.The scoped
#[expect(clippy::expect_used, ...)]is correctly applied for test-only.expect(...)usage. LGTM.
45-46: Align lint expectations for parity with other steps.The added
#[expect(clippy::expect_used, ...)]is appropriate here. LGTM.
127-133: Assert cooldown cleanly from loaded config.The assertion pattern and error path match the existing steps. LGTM.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
leynos
force-pushed
the
codex/add-cucumber-test-for-cooldown_period_seconds-default
branch
from
745ed47 to
53670fb
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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)
tests/features/config.feature(1 hunks)tests/steps/config_steps.rs(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
⚙️ CodeRabbit configuration file
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Where code is only used by specific features, it must be conditionally compiled or a conditional expectation for unused_code applied.
Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Environment access (env::set_var and env::remove_var) are always unsafe in Rust 2024 and MUST be marked as such
- For testing of functionality depending upon environment variables, dependency injection and the
mockablecrate are the preferred option.- If mockable cannot be used, env mutations in tests ...
Files:
tests/steps/config_steps.rs
🔍 Remote MCP Ref
Summary of relevant facts for reviewing PR #93
-
Files added/changed in the PR branch:
- tests/features/config.feature — adds a scenario that loads a config with token "abc" and no cooldown_period_seconds, then asserts cooldown_period_seconds is 960.
- tests/steps/config_steps.rs — adds a step to set up a config without cooldown (delegates to config_file_with_token) and adds the then-step matching r#"^cooldown_period_seconds is (\d+)$"# which does
match world.result.take() { Some(Ok(cfg)) => assert_eq!(cfg.cooldown_period_seconds, expected), other => panic!(...)}.Also reorders two clippy attribute annotations (no semantic change).
-
Scope: The PR modifies only test files (Cucumber feature + step definitions); it does not change production/config-loading code in the PR itself. Reviewers should therefore verify the asserted default (960) against the canonical config implementation (src/config.rs or equivalent) to ensure the test matches the actual default.
Suggested quick review checklist
- Confirm the config implementation's default for cooldown_period_seconds is 960 (so the new test is correct).
- Run the test suite locally (make fmt, make lint, make test) to ensure no false failures.
- Verify the then-step's panic message and pattern matching are sufficient for diagnosability if parsing fails.
Sources used
- tests/features/config.feature (PR branch).
- tests/steps/config_steps.rs (PR branch).
- Repository search/metadata referenced during review.
🔇 Additional comments (4)
tests/steps/config_steps.rs (3)
35-35: Attribute order and scope are correct.Keep the clippy expectation narrowly scoped under the step attribute.
45-45: Attribute order and scope are correct.Maintain this placement for consistent lint scoping.
135-141: Then-step for cooldown assertion is correct.Regex, typing, and result handling match existing patterns.
tests/features/config.feature (1)
35-38: Leave the scenario as-is — the canonical default cooldown is 960.DEFAULT_COOLDOWN is set to 960 in crates/comenqd/src/config.rs; docs and packaging also reference 960.
|
@coderabbitai pause |
✅ Actions performedReviews paused. |
leynos
force-pushed
the
codex/add-cucumber-test-for-cooldown_period_seconds-default
branch
from
deb3225 to
94f0be9
Compare
leynos
deleted the
codex/add-cucumber-test-for-cooldown_period_seconds-default
branch
1 participant
Summary
cooldown_period_secondsTesting
make fmtmake lintmake testcloses #9
https://chatgpt.com/codex/tasks/task_e_68c3f86217ec8322bcf946454f3b48fe
Summary by Sourcery
Add Cucumber test for default cooldown period in daemon configuration
Tests: