Skip to content

Centralise test helpers in test-support#56

Merged
leynos merged 11 commits intomainfrom
codex/refactor-temp_config-and-octocrab_for-location
Aug 9, 2025
Merged

Centralise test helpers in test-support#56
leynos merged 11 commits intomainfrom
codex/refactor-temp_config-and-octocrab_for-location

Conversation

@leynos
Copy link
Copy Markdown
Owner

@leynos leynos commented Aug 6, 2025

Summary

  • host temp_config and octocrab_for in test-support
  • drop duplicate helper implementations from crates and tests
  • update tests to import helpers from test_support

Testing

  • make fmt
  • make lint
  • make test

https://chatgpt.com/codex/tasks/task_e_6893d35949c083228a71115cae436ddc

Summary by Sourcery

Centralize common test helper functions in a dedicated test-support crate, update relevant crates and tests to use the shared implementations, and clean up obsolete local helpers.

Enhancements:

  • Centralize test helpers temp_config and octocrab_for in the test-support crate and refactor test-utils to re-export them
  • Update Cargo.toml for test-utils and comenqd crates to depend on test-support instead of local implementations

Tests:

  • Refactor Comenqd crate tests and project tests to import centralized helpers and remove duplicate test utility modules

Move temp_config and octocrab_for into test-support and update tests accordingly.
@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Aug 6, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR centralizes test helper implementations by delegating to the shared test-support crate, updating imports and dependencies accordingly, and removing duplicate helper code across crates and tests.

File-Level Changes

Change Details Files
Delegate helper functions in test-utils to the shared test-support crate
  • Removed inline temp_config and octocrab_for implementations
  • Re-exported helpers via pub use from test_support
crates/test-utils/src/lib.rs
Update crate dependencies to use test-support
  • Replaced individual test utility dependencies with a test-support dependency in test-utils
  • Added test-support dependency to comenqd and removed test-utils link
crates/test-utils/Cargo.toml
crates/comenqd/Cargo.toml
Refactor tests in daemon to import shared helpers
  • Removed local temp_config and octocrab_for definitions
  • Added use imports for octocrab_for and temp_config from test_support
crates/comenqd/src/daemon.rs
Remove obsolete test helper modules
  • Deleted tests/util/mod.rs
  • Deleted tests/util/test_helpers.rs
tests/util/mod.rs
tests/util/test_helpers.rs

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Aug 6, 2025

Summary by CodeRabbit

  • Refactor

    • Consolidated and streamlined test helper utilities by introducing a new TestConfig struct and centralising helpers in the test-support crate.
    • Updated test configuration logic to use TestConfig and improved conversion to Config for testing.
    • Simplified and reduced dependencies in test utility crates.
    • Removed redundant local test helper files and modules.
  • New Features

    • Added public TestConfig struct and conversion methods for easier test setup.
  • Chores

    • Updated dependency declarations and feature flags to improve workspace consistency and test support.

Walkthrough

Remove local implementations of the temp_config and octocrab_for test helpers across the codebase, deleting associated files and imports. Re-export these helpers from the test_support crate, centralising their usage. Update dependencies and imports to reflect this consolidation, eliminating duplication and redundant modules.

Changes

Cohort / File(s) Change Summary
Test Helper Consolidation
crates/comenqd/src/daemon.rs, tests/steps/listener_steps.rs, tests/steps/worker_steps.rs
Remove local definitions and imports of temp_config and octocrab_for; import them from test_support instead.
Test Utility Module Removal
tests/util/mod.rs, tests/util/test_helpers.rs
Delete the utility module and helper file that previously re-exported and defined the test helpers.
Test Utility Crate Refactor
crates/test-utils/Cargo.toml, crates/test-utils/src/lib.rs
Remove direct implementations and dependencies; re-export helpers from test_support and update documentation.
Dependency Cleanup
crates/comenqd/Cargo.toml, crates/test-utils/Cargo.toml, test-support/Cargo.toml
Remove unnecessary or redundant dependencies related to test helpers.
Test Import Cleanup
tests/cucumber.rs
Remove unused import of the now-deleted util module.
Test Support Enhancements
test-support/src/daemon.rs, test-support/src/lib.rs
Introduce TestConfig struct replacing Config in test helpers; publicly re-export TestConfig alongside helpers.
Config Conversion Additions
crates/comenqd/src/config.rs, Cargo.toml
Add From trait implementations converting TestConfig to Config gated by test-support feature; enable test-support feature in comenqd dependency.

Sequence Diagram(s)

sequenceDiagram
    participant Test as Test File
    participant test_support as test_support Crate
    Note right of Test: Request test helper (e.g., temp_config)
    Test->>test_support: Use temp_config / octocrab_for
    Note right of test_support: Provides implementation
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~7 minutes

Possibly related PRs

Poem

Helpers once scattered, now gathered with care,
In test_support's arms, they’re easy to share.
Redundant code gone, the imports are neat,
With modules deleted, the circle’s complete.
Refactor accomplished—let’s all have a cheer!
For tidier tests, and code that’s sincere.
🧹✨

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/refactor-temp_config-and-octocrab_for-location

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @leynos - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Aug 7, 2025

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Aug 7, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Aug 8, 2025

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Aug 8, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (3)
tests/steps/listener_steps.rs (1)

40-46: Apply the same refactoring as suggested in worker_steps.rs.

This file exhibits the same verbose field copying pattern. Implement the From<TestConfig> trait as suggested in the worker_steps.rs review to eliminate this duplication.

crates/comenqd/src/daemon.rs (2)

301-308: Apply the From trait refactoring for consistency.

This function demonstrates the same verbose field copying pattern seen in other test files. Implement the From<TestConfig> trait as suggested in previous reviews to maintain consistency across the codebase.


355-362: Apply the From trait refactoring for consistency.

The worker_test_context fixture exhibits the same pattern. Use the suggested From<TestConfig> implementation to reduce boilerplate.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9153b26 and 4407acc.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (6)
  • crates/comenqd/src/daemon.rs (2 hunks)
  • test-support/Cargo.toml (0 hunks)
  • test-support/src/daemon.rs (1 hunks)
  • test-support/src/lib.rs (1 hunks)
  • tests/steps/listener_steps.rs (2 hunks)
  • tests/steps/worker_steps.rs (2 hunks)
💤 Files with no reviewable changes (1)
  • test-support/Cargo.toml
🧰 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 return in single-line functions.

  • Move conditionals with >2 branches into a predicate function.

  • Avoid unsafe unless 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.
  • Use rstest fixtures for shared setup and to avoid repetition between tests.

  • Replace duplicated tests with #[rstest(...)] parameterised cases.

  • Prefer mockall for 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.

Files:

  • tests/steps/worker_steps.rs
  • tests/steps/listener_steps.rs
  • crates/comenqd/src/daemon.rs
  • test-support/src/lib.rs
  • test-support/src/daemon.rs
🧬 Code Graph Analysis (4)
tests/steps/worker_steps.rs (1)
test-support/src/daemon.rs (2)
  • octocrab_for (43-52)
  • temp_config (30-37)
tests/steps/listener_steps.rs (2)
test-support/src/daemon.rs (1)
  • temp_config (30-37)
test-support/src/util.rs (1)
  • wait_for_file (32-40)
crates/comenqd/src/daemon.rs (1)
test-support/src/daemon.rs (2)
  • octocrab_for (43-52)
  • temp_config (30-37)
test-support/src/lib.rs (1)
test-support/src/daemon.rs (2)
  • octocrab_for (43-52)
  • temp_config (30-37)
🔍 MCP Research (1 server)

Deepwiki:

  • There are no reported issues, bugs, or incidents related to the test helper functions temp_config or octocrab_for in the files crates/comenqd/src/daemon.rs, test-support/src/daemon.rs, test-support/src/lib.rs, tests/steps/listener_steps.rs, or tests/steps/worker_steps.rs. These functions are used in the test suite to set up configurations and mock GitHub API interactions for testing purposes. (Source: internal codebase context and wiki)
🔇 Additional comments (6)
test-support/src/lib.rs (1)

10-10: LGTM! TestConfig addition aligns with centralization.

The addition of TestConfig to the re-exports correctly supports the new test configuration struct introduced in the daemon module.

tests/steps/worker_steps.rs (1)

15-15: LGTM! Import correctly updated to use centralized test helpers.

The change from local util to test_support properly reflects the centralization effort.

tests/steps/listener_steps.rs (1)

11-11: LGTM! Import properly updated to centralized test support.

The change correctly imports temp_config from test_support instead of the local util module.

crates/comenqd/src/daemon.rs (1)

291-291: LGTM! Proper import of centralized test helpers.

The import correctly brings in octocrab_for and temp_config from the centralized test_support crate.

test-support/src/daemon.rs (2)

30-37: LGTM! Clean implementation of centralized test config.

The function properly returns the new TestConfig struct with sensible default values for testing. The implementation is clean and well-documented.


43-52: LGTM! octocrab_for function remains appropriately unchanged.

The function correctly creates an Octocrab client for testing with proper error handling using .expect() as per coding guidelines.

Comment thread test-support/src/daemon.rs
Comment thread tests/steps/worker_steps.rs Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4407acc and 264cfbd.

📒 Files selected for processing (6)
  • Cargo.toml (1 hunks)
  • crates/comenqd/Cargo.toml (1 hunks)
  • crates/comenqd/src/config.rs (1 hunks)
  • crates/comenqd/src/daemon.rs (2 hunks)
  • tests/steps/listener_steps.rs (2 hunks)
  • tests/steps/worker_steps.rs (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
Cargo.toml

📄 CodeRabbit Inference Engine (AGENTS.md)

Cargo.toml: Use explicit version ranges in Cargo.toml and keep dependencies up-to-date.
Mandate caret requirements for all dependencies. All crate versions specified in Cargo.toml must use SemVer-compatible caret requirements (e.g., some-crate = "1.2.3").
Prohibit unstable version specifiers. The use of wildcard (*) or open-ended inequality (>=) version requirements are strictly forbidden, as are tilde requirements (~) except for documented reasons.

Files:

  • Cargo.toml
**/*.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:

  • crates/comenqd/src/config.rs
  • crates/comenqd/src/daemon.rs
  • tests/steps/listener_steps.rs
  • tests/steps/worker_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 return in single-line functions.

  • Move conditionals with >2 branches into a predicate function.

  • Avoid unsafe unless 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.
  • Use rstest fixtures for shared setup and to avoid repetition between tests.

  • Replace duplicated tests with #[rstest(...)] parameterised cases.

  • Prefer mockall for 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.

Files:

  • crates/comenqd/src/config.rs
  • crates/comenqd/src/daemon.rs
  • tests/steps/listener_steps.rs
  • tests/steps/worker_steps.rs
🧬 Code Graph Analysis (2)
crates/comenqd/src/daemon.rs (1)
test-support/src/daemon.rs (2)
  • octocrab_for (43-52)
  • temp_config (30-37)
tests/steps/worker_steps.rs (1)
test-support/src/daemon.rs (2)
  • octocrab_for (43-52)
  • temp_config (30-37)
🪛 GitHub Check: build-test
crates/comenqd/src/daemon.rs

[failure] 353-353:
type annotations needed for Arc<_>

🪛 GitHub Actions: CI
crates/comenqd/src/daemon.rs

[error] 353-353: Rust compiler error E0282: type annotations needed for Arc<_> at line 353.

🔍 MCP Research (1 server)

Deepwiki:

  • The test-support crate centralizes common test helpers such as temp_config and octocrab_for, which were previously defined locally in crates/comenqd/src/daemon.rs and other test modules. This centralization improves organization and reusability of test utilities across the repository. (DeepWiki search result summary)

  • In crates/comenqd/src/daemon.rs, the test helper functions temp_config and octocrab_for are now imported from the test-support crate instead of being locally defined. These helpers are used in tests like run_creates_queue_directory and worker_test_context to set up temporary configurations and mock Octocrab clients. (DeepWiki search result summary)

  • The crates/comenqd/Cargo.toml file was updated to include test-support as a dependency, reflecting the migration of shared test utilities into this dedicated crate. (DeepWiki search result summary)

  • Behavioral test step files tests/steps/listener_steps.rs and tests/steps/worker_steps.rs now import test helpers like temp_config, octocrab_for, and wait_for_file from the test_support crate, facilitating consistent test setup across different test suites. (DeepWiki search result summary)

🔇 Additional comments (5)
Cargo.toml (1)

18-18: Dependency feature flag looks correct

Path-based dev-dependency comenqd now explicitly enables the test-support feature; this is compliant with the workspace’s version-pinning rules.

tests/steps/worker_steps.rs (1)

15-15: Import switch approved

Importing helpers from test_support removes local duplication and aligns with the centralisation effort.

tests/steps/listener_steps.rs (1)

11-11: Centralised helper import approved

Switching to test_support keeps tests DRY; no issues.

crates/comenqd/src/daemon.rs (2)

303-303: Approve: use ..temp_config(dir).into() to fill remaining Config fields.

The struct update gives Into a concrete target type (Config), avoiding inference issues. This is clean and idiomatic.


291-291: Verify test-support wiring – no action needed

  • crates/comenqd/Cargo.toml declares
    [dependencies]
    test-support = { path = "../../test-support", optional = true }
    
    [features]
    default = []
    test-support = ["dep:test-support"]
    so the crate pulls in test-support only when the test-support feature is enabled.
  • In crates/comenqd/src/daemon.rs, all use test_support::… lines live inside the #[cfg(test)] mod tests { … } block.

No changes required.

Comment thread crates/comenqd/Cargo.toml Outdated
Comment thread crates/comenqd/Cargo.toml
Comment thread crates/comenqd/src/config.rs
Comment thread crates/comenqd/src/config.rs
Comment thread crates/comenqd/src/daemon.rs Outdated
Comment thread crates/comenqd/src/daemon.rs Outdated
Comment thread tests/steps/listener_steps.rs
Comment thread tests/steps/worker_steps.rs Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🔭 Outside diff range comments (1)
tests/steps/worker_steps.rs (1)

1-5: Replace blanket allow with scoped expect.

Project guidelines forbid broad #[allow]. Use #![expect(clippy::expect_used, reason = "simplify test output")] etc., or apply per-call expect on the two offending sites.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 264cfbd and cb52f1e.

📒 Files selected for processing (7)
  • Cargo.toml (2 hunks)
  • crates/comenqd/Cargo.toml (1 hunks)
  • crates/comenqd/src/config.rs (1 hunks)
  • crates/comenqd/src/daemon.rs (2 hunks)
  • test-support/src/daemon.rs (1 hunks)
  • tests/steps/listener_steps.rs (2 hunks)
  • tests/steps/worker_steps.rs (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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/listener_steps.rs
  • tests/steps/worker_steps.rs
  • crates/comenqd/src/daemon.rs
  • crates/comenqd/src/config.rs
  • test-support/src/daemon.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 return in single-line functions.

  • Move conditionals with >2 branches into a predicate function.

  • Avoid unsafe unless 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.
  • Use rstest fixtures for shared setup and to avoid repetition between tests.

  • Replace duplicated tests with #[rstest(...)] parameterised cases.

  • Prefer mockall for 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.

Files:

  • tests/steps/listener_steps.rs
  • tests/steps/worker_steps.rs
  • crates/comenqd/src/daemon.rs
  • crates/comenqd/src/config.rs
  • test-support/src/daemon.rs
Cargo.toml

📄 CodeRabbit Inference Engine (AGENTS.md)

Cargo.toml: Use explicit version ranges in Cargo.toml and keep dependencies up-to-date.
Mandate caret requirements for all dependencies. All crate versions specified in Cargo.toml must use SemVer-compatible caret requirements (e.g., some-crate = "1.2.3").
Prohibit unstable version specifiers. The use of wildcard (*) or open-ended inequality (>=) version requirements are strictly forbidden, as are tilde requirements (~) except for documented reasons.

Files:

  • Cargo.toml
🧬 Code Graph Analysis (3)
tests/steps/listener_steps.rs (1)
test-support/src/daemon.rs (1)
  • temp_config (30-37)
tests/steps/worker_steps.rs (2)
crates/comenqd/src/daemon.rs (1)
  • run_worker (244-278)
test-support/src/daemon.rs (2)
  • octocrab_for (51-60)
  • temp_config (30-37)
crates/comenqd/src/daemon.rs (2)
test-support/src/util.rs (1)
  • poll_until (81-105)
test-support/src/daemon.rs (2)
  • octocrab_for (51-60)
  • temp_config (30-37)
🔍 MCP Research (1 server)

Deepwiki:

  • Added conditional implementations (gated by the test-support feature) to convert from test_support::daemon::TestConfig to the main Config struct. This includes two From trait implementations: one consuming an owned TestConfig and one for a borrowed reference that clones before conversion. Additionally, a new trait IntoConfig was introduced with a single method into_config that consumes a TestConfig and returns a Config. This trait is implemented for test_support::daemon::TestConfig by delegating to the existing From implementation. These additions provide explicit conversions from test configuration types to the main daemon configuration. (crates/comenqd/src/config.rs)

  • The code removes the dependency on the external Config type and replaces it with a new local struct TestConfig specifically designed for daemon tests. This struct explicitly defines fields for GitHub token, socket path, queue path, and cooldown period, all with appropriate types and documentation. The temp_config function is updated to return a TestConfig instance instead of a Config. Additionally, a new method with_cooldown is added to TestConfig to allow overriding the cooldown period fluently. The import statements are adjusted to include PathBuf and remove the import of the external Config type. The rest of the code, including the octocrab_for function, remains unchanged. (test-support/src/daemon.rs)

  • The import of IntoConfig from comenqd::config was added. The temp_config function is now imported from test_support instead of being referenced from a local crate module. In the running_listener step, the configuration object created by temp_config(&dir) is converted into a Config instance by calling .into_config() before being wrapped in an Arc. This replaces the previous direct wrapping of the temp_config result in an Arc. The rest of the logic in the step, including channel creation, task spawning, and assertions, remains unchanged. (tests/steps/listener_steps.rs)

  • The import source for the octocrab_for and temp_config functions was changed from the local crate's util module to an external or differently named test_support module. The queued_request function's construction of the Config object was refactored: instead of mutating the cooldown_period_seconds field on a mutable Config returned by temp_config, it now uses a method chain calling with_cooldown(0) followed by into_config() to produce the final Config instance wrapped in an Arc. This replaces the previous pattern of creating a mutable copy and manually setting the cooldown period. The rest of the function and the file remain unchanged. (tests/steps/worker_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: build-test
🔇 Additional comments (10)
crates/comenqd/Cargo.toml (1)

21-22: Dependency and feature wiring looks sound.

Optional test-support dependency is correctly gated behind the likewise-named feature; no action required.

Also applies to: 29-32

tests/steps/listener_steps.rs (1)

41-41: Conversion chain is clear and explicit.

temp_config(...).into_config() expresses intent well; good job replacing the implicit From call.

Cargo.toml (1)

18-19: Workspace alignment OK.

test-support is now supplied through workspace metadata; no version drift risk spotted.

Also applies to: 24-24, 58-58

tests/steps/worker_steps.rs (1)

40-40: Chain remains readable and concise.

with_cooldown(0).into_config() eliminates the earlier manual struct update—nice improvement.

crates/comenqd/src/daemon.rs (4)

293-293: Centralise imports to test_support — good change

Import octocrab_for and temp_config from test_support to remove duplication across tests. This aligns with the PR objective.


302-302: Builder-style config in tests — good refactor

Return Config via temp_config(dir).with_cooldown(secs).into_config() for clarity and immutability; this keeps functions short and single-purpose.


347-347: Promote inline note to fixture-level doc — good

Document the cooldown behaviour at the fixture boundary and keep attributes after doc comments per guidelines.


284-284: Enable the test-support feature on every test invocation

The IntoConfig trait and its From<TestConfig> implementations are gated behind #[cfg(feature = "test-support")]. Your local and CI test runs must include --features test-support to compile and link those conversions.

Pinpoint updates:

  • Makefile: change any cargo test lines to
    cargo test --features test-support [other args]
  • .github/workflows/*.yml: ensure every run: cargo test step includes
    --features test-support

Run your CI locally and verify all test suites pass with cargo test --features test-support.

crates/comenqd/src/config.rs (2)

38-49: Owned From<TestConfig> impl — correct and documented

Map fields 1:1, keep it gated behind test-support, and document the conversion. This enables explicit Config::from(test_cfg) without inference issues.


51-57: Delegate the borrowed impl to the owned one — good

Delegate via Self::from(value.clone()) to avoid duplicating field lists and reduce maintenance.

Comment thread crates/comenqd/src/config.rs Outdated
Comment thread crates/comenqd/src/daemon.rs Outdated
Comment thread test-support/src/daemon.rs Outdated
Comment thread test-support/src/daemon.rs
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cb52f1e and ca0bdb3.

📒 Files selected for processing (5)
  • crates/comenqd/src/config.rs (1 hunks)
  • crates/comenqd/src/daemon.rs (2 hunks)
  • test-support/src/daemon.rs (1 hunks)
  • tests/steps/listener_steps.rs (2 hunks)
  • tests/steps/worker_steps.rs (2 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:

  • crates/comenqd/src/config.rs
  • tests/steps/listener_steps.rs
  • crates/comenqd/src/daemon.rs
  • test-support/src/daemon.rs
  • tests/steps/worker_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 return in single-line functions.

  • Move conditionals with >2 branches into a predicate function.

  • Avoid unsafe unless 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.
  • Use rstest fixtures for shared setup and to avoid repetition between tests.

  • Replace duplicated tests with #[rstest(...)] parameterised cases.

  • Prefer mockall for 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.

Files:

  • crates/comenqd/src/config.rs
  • tests/steps/listener_steps.rs
  • crates/comenqd/src/daemon.rs
  • test-support/src/daemon.rs
  • tests/steps/worker_steps.rs
🧬 Code Graph Analysis (3)
tests/steps/listener_steps.rs (3)
test-support/src/daemon.rs (1)
  • temp_config (30-37)
test-support/src/util.rs (1)
  • wait_for_file (32-40)
crates/comenqd/src/config.rs (2)
  • from (41-48)
  • from (54-56)
crates/comenqd/src/daemon.rs (2)
test-support/src/daemon.rs (2)
  • octocrab_for (52-61)
  • temp_config (30-37)
crates/comenqd/src/config.rs (2)
  • from (41-48)
  • from (54-56)
tests/steps/worker_steps.rs (2)
test-support/src/daemon.rs (2)
  • octocrab_for (52-61)
  • temp_config (30-37)
crates/comenqd/src/config.rs (2)
  • from (41-48)
  • from (54-56)
🔍 MCP Research (2 servers)

Deepwiki:

  • The test-support crate centralizes common test helpers such as temp_config and octocrab_for in its daemon module, which are re-exported for convenient use in other crates and tests. This consolidation replaces previous local or crate-specific implementations, improving code reuse and maintainability. (Source: repository-wide explanation from the ask_question response)

  • In crates/comenqd/src/config.rs, the test-support crate's TestConfig type is integrated via From trait implementations to convert from test_support::daemon::TestConfig to the main Config struct, enabling seamless use of centralized test configurations in the main daemon code. (Source: PR summary for crates/comenqd/src/config.rs)

  • The crates/comenqd/src/daemon.rs test module now imports temp_config and octocrab_for from test_support::daemon instead of defining them locally, and uses the IntoConfig trait conversion to build Config instances from TestConfig. This reflects the centralization of test helpers and the new TestConfig struct usage. (Source: PR summary for crates/comenqd/src/daemon.rs)

  • Behavioral test step files such as tests/steps/listener_steps.rs and tests/steps/worker_steps.rs have updated imports to use temp_config and octocrab_for from the centralized test_support crate, replacing previous local or crate-specific imports. They also adopt the new pattern of converting TestConfig into Config via Config::from(...). (Source: PR summaries for these test step files)

Context7:

  • File changes in crates/comenqd/src/config.rs add conditional From trait implementations for converting test_support::daemon::TestConfig into the main Config struct, enabling seamless integration of the new test configuration type into existing code when the test-support feature is enabled. (PR #56)

  • In test-support/src/daemon.rs, a new TestConfig struct is defined with fields for GitHub token, socket path, queue path, and cooldown period. The temp_config function returns a TestConfig instance, and a method with_cooldown allows fluent modification of the cooldown period. This replaces the previous use of the external Config type in test helpers, isolating test configuration concerns within test-support. (PR #56)

  • The test-utils crate is refactored to remove its own implementations of temp_config and octocrab_for, instead re-exporting these from test_support. This simplifies dependencies and encourages direct use of test_support for new code. (PR #56)

  • Test files and steps (tests/steps/listener_steps.rs, tests/steps/worker_steps.rs) update imports to use temp_config and octocrab_for from test_support and adjust configuration construction to use Config::from(temp_config(...)) with the new TestConfig conversions, ensuring compatibility with the centralized helpers. (PR #56)

🔇 Additional comments (2)
tests/steps/listener_steps.rs (1)

11-11: Import set is spot-on

Importing temp_config, wait_for_file, and the retry constants from test_support cleanly reflects the new centralised helpers.

tests/steps/worker_steps.rs (1)

39-39: Nice use of with_cooldown

Calling with_cooldown(0) on the TestConfig keeps tests expressive and avoids manual field edits.

Comment thread crates/comenqd/src/config.rs
Comment thread crates/comenqd/src/daemon.rs Outdated
Comment thread test-support/src/daemon.rs Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ca0bdb3 and 1598fed.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • crates/comenqd/src/config.rs (1 hunks)
  • crates/comenqd/src/daemon.rs (4 hunks)
  • test-support/Cargo.toml (1 hunks)
  • test-support/src/daemon.rs (1 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:

  • crates/comenqd/src/config.rs
  • crates/comenqd/src/daemon.rs
  • test-support/src/daemon.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 return in single-line functions.

  • Move conditionals with >2 branches into a predicate function.

  • Avoid unsafe unless 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.
  • Use rstest fixtures for shared setup and to avoid repetition between tests.

  • Replace duplicated tests with #[rstest(...)] parameterised cases.

  • Prefer mockall for 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.

Files:

  • crates/comenqd/src/config.rs
  • crates/comenqd/src/daemon.rs
  • test-support/src/daemon.rs
🔍 MCP Research (1 server)

Deepwiki:

  • The test-support crate now re-exports the helper functions temp_config and octocrab_for along with the new TestConfig struct, centralizing daemon test helpers for use across the project (test-support/src/lib.rs, test-support/src/daemon.rs).
  • In crates/comenqd/src/config.rs, two conditional From trait implementations were added (enabled by the test-support feature) to convert from test_support::daemon::TestConfig to the main Config struct, enabling seamless conversion between test and production configurations.
  • The test-support crate removed its local dependency on comenqd and added a workspace dependency on serde, reflecting internal dependency adjustments (test-support/Cargo.toml).
  • The temp_config and octocrab_for functions were removed from crates/comenqd/src/daemon.rs and replaced by imports from test_support, consolidating test helper usage and simplifying configuration construction via the IntoConfig trait (crates/comenqd/src/daemon.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: build-test
🔇 Additional comments (12)
test-support/Cargo.toml (1)

12-12: Approve test-support Cargo.toml setup

  • Confirm no direct comenqd dependency in test-support/Cargo.toml
  • Verify root Cargo.toml’s workspace serde entry includes features = ["derive"]
  • Confirm no #[derive(Serialize|Deserialize)] usage in test-support/src

Merge changes without further edits.

crates/comenqd/src/config.rs (2)

38-49: Solid owned conversion; doc before attribute; aligns with centralised helpers

Keep this as-is. The owned From<TestConfig> maps fields directly and satisfies the consolidation strategy.


51-57: Correctly delegate borrowed conversion to the owned impl

Delegate via value.clone() to keep a single field list. This is idiomatic and maintainable.

crates/comenqd/src/daemon.rs (5)

291-292: Use centralised test helpers from test_support

Import octocrab_for and temp_config from the shared crate. This removes duplication and matches the PR objective.


342-344: Keep doc comment above #[fixture] per style guide

Doc is clear and placed correctly. No change needed.


345-347: Construct Config via Config::from(temp_config(&dir)) to avoid type inference issues

Make the Into target explicit and drop the bespoke trait. This fixes E0282 and matches earlier guidance.


392-396: Inline cooldown override at call-site

Use with_cooldown(1) for fast, deterministic tests without extra glue. Good trade-off.


443-446: Wrap Config in Arc after constructing from TestConfig

Build Arc<Config> directly and keep cooldown override inline. Clean and explicit.

test-support/src/daemon.rs (4)

8-11: Import serde traits alongside PathBuf/Arc

Enable serialisation for TestConfig without ad-hoc glue. Good change.


15-27: Define TestConfig with complete derives and docs

Expose the minimal surface for tests and derive Clone, PartialEq, Eq, Default, Serialize, Deserialize. This unblocks assertions and snapshotting.


31-38: Return TestConfig from temp_config with 1s cooldown

Document the cooldown; keep paths scoped to TempDir. This is clear and predictable for tests.


40-47: Provide with_cooldown and mark it #[must_use]

Prevent accidental discard of the updated value. This is the right ergonomics for a fluent test API.

Comment thread crates/comenqd/src/config.rs Outdated
Comment thread test-support/src/daemon.rs
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1598fed and 9bb1b30.

📒 Files selected for processing (2)
  • crates/comenqd/src/config.rs (1 hunks)
  • crates/comenqd/src/lib.rs (1 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:

  • crates/comenqd/src/config.rs
  • crates/comenqd/src/lib.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 return in single-line functions.

  • Move conditionals with >2 branches into a predicate function.

  • Avoid unsafe unless 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.
  • Use rstest fixtures for shared setup and to avoid repetition between tests.

  • Replace duplicated tests with #[rstest(...)] parameterised cases.

  • Prefer mockall for 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.

Files:

  • crates/comenqd/src/config.rs
  • crates/comenqd/src/lib.rs
🔍 MCP Research (1 server)

Deepwiki:

  • Added two conditional From trait implementations for the Config struct, gated by the test-support feature. The first implementation converts an owned test_support::daemon::TestConfig into a Config by directly copying all fields. The second implementation converts a borrowed test_support::daemon::TestConfig by cloning it and delegating to the owned conversion. These additions enable seamless conversion from test-specific configuration types to the main daemon configuration. (crates/comenqd/src/config.rs)

  • Added the crate-level attribute #![cfg_attr(docsrs, feature(doc_cfg))] to enable the doc_cfg feature conditionally when building documentation with docs.rs. No other functional or structural changes were made. (crates/comenqd/src/lib.rs)

🔇 Additional comments (3)
crates/comenqd/src/lib.rs (2)

17-17: Enable doc_cfg on docs.rs — LGTM

Keep this attribute to surface feature-gated APIs in generated docs on docs.rs.


17-17: Approve doc(cfg) annotations
Both impl From<test_support::daemon::TestConfig> and impl From<&test_support::daemon::TestConfig> in crates/comenqd/src/config.rs already carry

#[cfg_attr(docsrs, doc(cfg(feature = "test-support")))]

No further annotations required.

crates/comenqd/src/config.rs (1)

52-60: LGTM: Delegate borrowed conversion to the owned impl.

Keep the delegation via Self::from(value.clone()) to avoid field duplication and centralise maintenance.

Comment thread crates/comenqd/src/config.rs Outdated
Comment thread crates/comenqd/src/lib.rs Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9bb1b30 and 3ebec51.

📒 Files selected for processing (2)
  • crates/comenqd/src/config.rs (2 hunks)
  • crates/comenqd/src/lib.rs (1 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:

  • crates/comenqd/src/lib.rs
  • crates/comenqd/src/config.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 return in single-line functions.

  • Move conditionals with >2 branches into a predicate function.

  • Avoid unsafe unless 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.
  • Use rstest fixtures for shared setup and to avoid repetition between tests.

  • Replace duplicated tests with #[rstest(...)] parameterised cases.

  • Prefer mockall for 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.

Files:

  • crates/comenqd/src/lib.rs
  • crates/comenqd/src/config.rs
🪛 GitHub Check: build-test
crates/comenqd/src/config.rs

[failure] 252-252:
unused import: TestConfig

🪛 GitHub Actions: CI
crates/comenqd/src/config.rs

[error] 252-252: Rust compiler error: unused import 'TestConfig'.

🔍 MCP Research (2 servers)

Deepwiki:

  • The Config struct in crates/comenqd/src/config.rs now includes conditional From trait implementations converting from test_support::daemon::TestConfig, enabling seamless conversion from the centralized test configuration to the daemon's runtime configuration. A test case verifies correct field copying during conversion. (crates/comenqd/src/config.rs)

  • The daemon's test modules in crates/comenqd/src/daemon.rs import temp_config and octocrab_for from the centralized test_support crate instead of defining them locally, using Config::from(temp_config(...).with_cooldown(...)) for configuration construction, improving consistency and reducing duplication. (crates/comenqd/src/daemon.rs)

  • The test-utils crate in crates/test-utils/src/lib.rs now solely re-exports temp_config and octocrab_for from the test_support crate, removing its own implementations and encouraging direct use of test_support for new code. (crates/test-utils/src/lib.rs)

  • Test step definitions in tests/steps/listener_steps.rs and tests/steps/worker_steps.rs import test helpers from test_support instead of local modules, constructing configuration objects via Config::from(temp_config(...)) with optional cooldown adjustments using with_cooldown, reflecting centralized test helper usage. (tests/steps/listener_steps.rs, tests/steps/worker_steps.rs)

Context7:

  • Added two conditional From trait implementations for the Config struct, gated by the test-support feature. The first implementation converts an owned test_support::daemon::TestConfig into a Config by directly copying all fields. The second implementation converts a borrowed test_support::daemon::TestConfig by cloning it and delegating to the owned conversion. Additionally, a test case was added (also gated by test-support) to verify that conversion from TestConfig to Config correctly copies all configuration fields. (crates/comenqd/src/config.rs)

  • Added the crate-level attribute #![cfg_attr(docsrs, feature(doc_cfg))] to conditionally enable the doc_cfg feature when building documentation with docs.rs. No other functional or structural changes were made. (crates/comenqd/src/lib.rs)

🔇 Additional comments (3)
crates/comenqd/src/lib.rs (1)

1-2: Add LGTM for crate attribute placement.

The #![cfg_attr(docsrs, feature(doc_cfg))] attribute is correctly placed at the very top, ahead of inner docs, aligning with earlier guidance and project standards.

crates/comenqd/src/config.rs (2)

54-63: Approve From<TestConfig> conversion (field-for-field copy).

The owned conversion copies all fields verbatim and is correctly feature-gated and documented. This matches the consolidation objective and prior guidance.


70-74: Approve borrowed From<&TestConfig> delegating to owned.

Delegating via Self::from(value.clone()) avoids duplication and centralises the field list. This is concise and maintainable for the test-only feature.

Comment thread crates/comenqd/src/config.rs
Comment thread crates/comenqd/src/config.rs
leynos and others added 2 commits August 9, 2025 19:17
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3ebec51 and 37d36a1.

📒 Files selected for processing (1)
  • crates/comenqd/src/config.rs (2 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:

  • crates/comenqd/src/config.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 return in single-line functions.

  • Move conditionals with >2 branches into a predicate function.

  • Avoid unsafe unless 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.
  • Use rstest fixtures for shared setup and to avoid repetition between tests.

  • Replace duplicated tests with #[rstest(...)] parameterised cases.

  • Prefer mockall for 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.

Files:

  • crates/comenqd/src/config.rs
🧬 Code Graph Analysis (1)
crates/comenqd/src/config.rs (2)
test-support/src/daemon.rs (1)
  • temp_config (31-38)
tests/steps/cli_steps.rs (1)
  • socket_path (47-53)
🔍 MCP Research (1 server)

Deepwiki:

  • Added two conditional From trait implementations for the Config struct, gated by the test-support feature. The first implementation converts an owned test_support::daemon::TestConfig into a Config by directly copying all fields. The second implementation converts a borrowed test_support::daemon::TestConfig by cloning it and delegating to the owned conversion. Additionally, a test case was added (also gated by test-support) to verify that conversion from TestConfig to Config correctly copies all configuration fields. (crates/comenqd/src/config.rs)
🔇 Additional comments (1)
crates/comenqd/src/config.rs (1)

38-63: LGTM: Feature-gated owned conversion with clear docs

Keep as-is. The conversion is correct, docs are explicit, doc(cfg) is present, and it complies with the Deepwiki requirement to add a From<TestConfig> gated by test-support.

Comment thread crates/comenqd/src/config.rs
Comment thread crates/comenqd/src/config.rs
@leynos leynos merged commit 90e847c into main Aug 9, 2025
1 of 2 checks passed
@leynos leynos deleted the codex/refactor-temp_config-and-octocrab_for-location branch August 9, 2025 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant