Add BDD and E2E coverage for lockfile rebuild after bump (#92) - #130
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis pull request adds BDD and end-to-end test coverage for the nested lockfile rebuild behaviour of the ChangesLockfile rebuild test coverage
Sequence Diagram(s)sequenceDiagram
participant Test as E2E test step
participant CmdMox as cargo::update stub
participant CLI as lading bump
participant FS as Cargo.lock files
Test->>FS: write stale content to root and crates/app Cargo.lock
Test->>CmdMox: stub cargo::update by --manifest-path
Test->>CLI: run lading bump 1.0.0
CLI->>CmdMox: invoke cargo::update per manifest
CmdMox->>FS: rewrite matching Cargo.lock
Test->>CLI: assert stdout lists regenerated lockfiles
Test->>FS: assert lockfile contents match marker
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 20✅ Passed checks (20 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
The lockfile rebuild path introduced with configurable lockfile regeneration had no BDD scenario and no end-to-end assertion that Cargo.lock files are actually rewritten after lading bump on a workspace with nested manifests. BDD: a new cli.feature scenario configures bump.lockfile_manifests with a nested manifest and asserts the CLI output lists both the root and nested lockfiles with the (lockfile) suffix. E2E: a new e2e.feature scenario configures the non-trivial fixture workspace to rebuild the app crate lockfile, seeds stale Cargo.lock files, and backs the cargo::update stub with a handler that rewrites the targeted lockfile the way cargo would. After lading bump runs via the real CLI process, the test asserts the output lists the regenerated lockfiles and that both files on disk carry the regenerated content. Closes #92
8f564fe to
6ba351a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/e2e/steps/test_e2e_steps.py`:
- Around line 311-317: The lading.toml fixture edit in the E2E step is using a
raw string replace, which ties the test to exact formatting. Update the
workspace config mutation in the step that reads and writes config_path to use
the same toml_utils helper flow already used in test_bump_steps, so the bump
section is modified structurally instead of by matching the literal "[bump]"
text. Keep the change localized to the existing step logic around workspace.root
and config_path, and preserve the lockfile_manifests insertion through the TOML
helper API.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6eec53cb-b563-44e8-ac13-c2e7012f8706
📒 Files selected for processing (4)
tests/bdd/features/cli.featuretests/bdd/steps/test_bump_steps.pytests/e2e/features/e2e.featuretests/e2e/steps/test_e2e_steps.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/cmd-mox(auto-detected)leynos/shared-actions(auto-detected)
Refactor the `given_workspace_rebuilds_app_lockfile` E2E step to configure
`bump.lockfile_manifests` through the structural `toml_utils` helper flow
(`load_or_create_document`/`ensure_table`/`ensure_array_field`/
`append_if_absent`) already used in `test_bump_steps`, rather than a raw
`str.replace("[bump]\n", ...)`. The literal replacement coupled the test to
the exact formatting of the fixture config; the helper API edits the `[bump]`
table structurally and is robust to layout changes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Closes #92
cli.featurescenario configuresbump.lockfile_manifestswith a nested manifest (nested/Cargo.toml) and asserts the CLI output lists both the root and nested lockfiles with the(lockfile)suffix.e2e.featurescenario configures the non-trivial fixture workspace to rebuild the app crate lockfile, seeds staleCargo.lockfiles, and backs thecargo::updatestub with aruns()handler that rewrites the targeted lockfile the way cargo would. Afterlading bumpruns via the real CLI process, the test asserts the output lists the regenerated lockfiles and that both files on disk carry the regenerated content — exercising the complete workflow through the CLI to the cargo boundary and resulting filesystem state.Testing
make check-fmt,make lint,make typecheck, andmake test(563 passed) all green after rebasing onto currentmain.coderabbit review --agent: 0 findings.References
🤖 Generated with Claude Code