Migrate behavioural tests to rstest-bdd v0.5.0: code + docs + ExecPlan#263
Open
Migrate behavioural tests to rstest-bdd v0.5.0: code + docs + ExecPlan#263
Conversation
…v0.5.0 - Introduce `docs/developers-guide.md` detailing testing workflow, behavioural test strategy, and authoring policies. - Add `docs/execplans/rstest-bdd-v0-5-0-behavioural-suite-migration.md` as a living ExecPlan for the migration to rstest-bdd v0.5.0. - Include staged plan with constraints, risks, decisions, progress, and validation steps. - Provide comprehensive context to ensure migration completeness and maintain test coverage. This documentation will serve as the source of truth for contributors updating or maintaining the behavioural test suite during and after the migration. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Contributor
Reviewer's GuideMigrates the behavioural test suite and documentation to rstest-bdd v0.5.0 by bumping dev-dependencies, refactoring BDD step implementations to use typed parameter wrappers and inferred patterns, and adding/updating guides and an ExecPlan that document the new v0.5.0 testing strategy, async patterns, and language-server features. Sequence diagram for async_only_step in synchronous scenario with Tokio fallback runtimesequenceDiagram
actor Tester
participant TestRunner
participant ScenarioSync
participant StepAsync
participant TokioFallbackRuntime
Tester->>TestRunner: run_tests
TestRunner->>ScenarioSync: invoke_scenario_sync
ScenarioSync->>ScenarioSync: execute_sync_steps
ScenarioSync->>ScenarioSync: encounter_async_only_step
ScenarioSync->>TokioFallbackRuntime: create_runtime_if_needed
TokioFallbackRuntime->>StepAsync: run_step_async
StepAsync-->>TokioFallbackRuntime: step_result
TokioFallbackRuntime-->>ScenarioSync: return_step_result
ScenarioSync->>TokioFallbackRuntime: drop_runtime
ScenarioSync-->>TestRunner: scenario_result
TestRunner-->>Tester: report_result
Flow diagram for migrating shared mutable World state to per_scenario fixturesflowchart TD
Start[Start_migration] --> HasSharedState{Suite_relies_on_shared_mutable_state_across_scenarios}
HasSharedState -- Yes --> IdentifyWorld[Identify_global_World_structs_and_cross_scenario_mutation]
HasSharedState -- No --> VerifyFixtures[Verify_existing_fixtures_follow_per_scenario_isolation]
IdentifyWorld --> MoveToFixtures[Move_shared_mutable_data_into_scenario_local_fixtures_using_mut_refs_or_Slot]
MoveToFixtures --> CheckInfra{Is_there_expensive_mostly_read_only_infrastructure}
CheckInfra -- Yes --> UseOnce[Wrap_expensive_read_only_infra_in_once_fixtures]
CheckInfra -- No --> PerScenarioOnly[Keep_all_fixtures_per_scenario_without_once]
UseOnce --> RecreateData[Recreate_scenario_data_per_scenario_without_order_dependencies]
PerScenarioOnly --> RecreateData
VerifyFixtures --> ReserveInsertOwned[Reserve_StepContext_insert_owned_for_custom_plumbing_only]
RecreateData --> ReserveInsertOwned
ReserveInsertOwned --> UpdateDocs[Update_docs_and_templates_for_state_sharing_and_fixtures]
UpdateDocs --> End[End_migration]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
Summary by CodeRabbit
WalkthroughUpgrade rstest-bdd and its macros to v0.5.0, expand dev-dependencies, add comprehensive migration and developer guides, and refactor behavioural test steps and types to use strongly-typed newtype wrappers with From/FromStr impls replacing many string parameters. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
…terns - Upgraded rstest-bdd and rstest-bdd-macros dependencies to v0.5.0 in Cargo.toml and Cargo.lock. - Refactored BDD step functions to use typed wrappers with FromStr implementations, enabling direct domain type usage. - Adopted inferred step patterns for simple no-argument steps to reduce boilerplate. - Updated behavioural test documentation to reflect new usage guidelines. - Completed migration plan stages from dependency bump through refactors to quality gates. - Improved test clarity and maintenance by removing repetitive string conversions and explicit step patterns. This improves test suite expressiveness and leverages new features introduced in rstest-bdd v0.5.0. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
leynos
changed the title
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The updated user guide now references
cucumber-rs-migration-and-async-patterns.md, so please double‑check that this file (and any other newly referenced docs like ADR-001) actually exist indocs/and that the relative links resolve correctly from all call sites. - In the new examples and guidance around async and fixtures, consider aligning the sample function signatures with what you recommend elsewhere (for example, using
_scenario_db: ScenarioDbinstead oflet _ = scenario_db;) so readers see the idiomatic pattern you intend to enforce in real tests.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The updated user guide now references `cucumber-rs-migration-and-async-patterns.md`, so please double‑check that this file (and any other newly referenced docs like ADR-001) actually exist in `docs/` and that the relative links resolve correctly from all call sites.
- In the new examples and guidance around async and fixtures, consider aligning the sample function signatures with what you recommend elsewhere (for example, using `_scenario_db: ScenarioDb` instead of `let _ = scenario_db;`) so readers see the idiomatic pattern you intend to enforce in real tests.
## Individual Comments
### Comment 1
<location> `docs/developers-guide.md:1` </location>
<code_context>
+# Developers guide
+
+This guide describes the day-to-day engineering workflow for Netsuke, with a
</code_context>
<issue_to_address>
**suggestion (typo):** Consider correcting the title to use the possessive or singular form (e.g., "Developers' guide" or "Developer guide").
The current heading "Developers guide" is ungrammatical; please use a possessive or singular form to align with standard English usage.
```suggestion
# Developer guide
```
</issue_to_address>
### Comment 2
<location> `docs/rstest-bdd-users-guide.md:1159` </location>
<code_context>
+
+- **Tokio current-thread mode only:** Multi-threaded Tokio mode would require
+ `Send` futures, which conflicts with the `RefCell`-backed fixture storage.
+ See [ADR-001](adr-001-async-fixtures-and-test.md) for the full design
+ rationale.
+- **Nested runtime safeguards:** Async-only steps running in synchronous
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The acronym "ADR" is introduced in "ADR-001" without being expanded on first use, which can confuse readers unfamiliar with the term.
Consider expanding ADR on first use, for example: `See Architecture Decision Record 001 (ADR-001) ...` or similar, so that the acronym is defined for readers who may not know what ADR stands for.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*.md`
**Instructions:**
Define uncommon acronyms on first use.
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…fter PR review - Corrected heading grammar in developer's guide. - Replaced stale or broken migration and ADR links with valid, existing documentation targets. - Updated scenario fixture examples to use idiomatic `_scenario_db: ScenarioDb` without placeholders. - Expanded acronyms on first use and aligned citations. - Verified that linting and tests pass after documentation fixes. These changes address feedback and comments from the PR review to ensure documentation accuracy and consistency. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
leynos
changed the title
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/developers-guide.md`:
- Around line 102-103: Summary: missing comma before "so" in the compound
sentence. Edit the sentence "When test strategy or behavioural test usage
changes, update this file in the same change-set so the documented approach
remains aligned with the codebase." and insert a comma before "so" so it reads
"...same change-set, so the documented approach remains aligned with the
codebase." Locate this sentence in docs/developers-guide.md and update the
punctuation accordingly.
…guidance Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/developers-guide.md`:
- Around line 15-18: Replace the indented block containing the shell commands
with a fenced code block using a language identifier (use "bash" for these shell
commands) and ensure there are blank lines before and after the fenced block;
locate the current indented snippet ("set -o pipefail" / "make test 2>&1 | tee
/tmp/netsuke-make-test.log") and wrap it with ```bash and ``` fences so the
documentation follows the fenced-code-block rule.
- Add code block syntax to shell commands in developers-guide.md - Fix table alignments and formatting in execplans and translators-guide.md - Adjust line breaks and wording for better readability across guides - No functional changes, documentation polish only Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/execplans/3-7-3-translator-tooling-and-documentation.md`:
- Around line 94-99: Change the US spelling "localization" to British English
"localisation" in the documentation text that describes the netsuke localization
API; specifically update the running prose that references
LocalizedMessage::with_arg so it reads "netsuke localisation API
(`LocalizedMessage::with_arg`)" and ensure any other occurrences in that
paragraph use "localisation" to conform to en-GB-oxendict guidelines.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Migration plan at a glance
The plan emphasizes behavioural parity, reduced boilerplate, and clear per-scenario isolation using v0.5.0 features.
Documentation highlights
Validation and testing plan
Rationale
How to review
Acceptance criteria
Related artifacts
◳ Generated by DevBoxer ◰
Task: https://www.devboxer.com/task/5c3168d1-f2f1-46f2-955a-0d00e67f9d7a
Summary by Sourcery
Migrate the behavioural test suite and documentation to rstest-bdd v0.5.0, updating dependencies, step definitions, and guidance for async execution, state isolation, and language server diagnostics.
New Features:
Enhancements:
Build:
Documentation:
Tests: