test(reconcile): BDD integration scenarios for ambiguous publish outcomes (#99 follow-on)#119
Conversation
…omes (#99 follow-on) Closes the last remaining #99 follow-on: end-to-end integration coverage for the three reconciliation outcomes the state machine can produce. ## Three scenarios added to engine::parallel::tests Each exercises publish_package() against a real tiny_http mock registry with a fake cargo binary configured to exit ambiguously (exit 1, empty stdout/stderr — which classify_publish_failure maps to CargoFailureClass::Ambiguous per existing unit tests). Readiness polling is disabled (readiness.enabled = false) so each reconcile site reduces to a single registry query — avoids flaky request counts that a full polling loop would produce. ### 1. reconcile_bdd_ambiguous_resolves_to_published - cargo fails ambiguously - entry + post-cargo quick checks return 404 - reconcile's single version_exists poll returns 200 - expected: PackageState::Published, attempts=1 (no second cargo) - verifies PublishReconciling + PublishReconciled{Published} events emitted to the on-disk event log ### 2. reconcile_bdd_ambiguous_resolves_to_not_published_then_retries - cargo fails ambiguously on every attempt - registry consistently returns 404 - reconcile resolves to NotPublished, falls through to normal retry - after max_attempts=2, package ends Failed - verifies PublishReconciled{NotPublished} event emitted - verifies cargo was actually retried (attempts=2) ### 3. reconcile_bdd_resume_from_ambiguous_state_skips_republish - state pre-seeded with PackageState::Ambiguous (simulating a prior interrupted run) - entry check returns 404 (bypasses "already published" short-circuit) - resume-path reconcile (from PR #115) fires BEFORE retry loop - single version_exists query returns 200 → Published - expected: attempts=0 (cargo NOT invoked) - verifies via SHIPPER_CARGO_ARGS_LOG that cargo binary was never called - verifies PublishReconciling + PublishReconciled{Published} events emitted ## Verification - cargo test -p shipper --lib -- reconcile_bdd: 3 pass - cargo test -p shipper --lib -- engine::parallel: 70 pass (no regressions) - cargo clippy --all-targets -- -D warnings: clean - cargo fmt --check: clean ## Why events are read from disk publish_package flushes event batches to events.jsonl and clears the in-memory log after each write. Assertions read via EventLog::read_from_file(&events_path) to see the full persisted stream. ## #99 tracker after this merge - [x] In-flight reconcile state machine (#111) - [x] Resume-path reconciliation (#115) - [x] Docs/contracts: cargo text is a hint, registry is truth (#117) - [x] BDD integration scenarios for all 3 outcomes (this PR) #99 is now fully closed.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 7 minutes and 12 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive BDD tests for the reconciliation state machine within the parallel engine. These tests validate the handling of ambiguous cargo failures by mocking registry responses to ensure correct transitions to published or retry states, and verify that resuming from an ambiguous state avoids unnecessary republishing. A review comment suggests restricting the visibility of the reconcile_scenario_opts helper function to better reflect its internal scope within the test module.
| fn reconcile_scenario_opts(state_dir: PathBuf) -> RuntimeOptions { | ||
| let mut opts = default_opts(state_dir); | ||
| opts.readiness.enabled = false; | ||
| opts | ||
| } |
There was a problem hiding this comment.
The reconcile_scenario_opts function is only used within the test module. It should be marked as pub(crate) or pub(super) to indicate its restricted scope, or kept private if it is only used within this file.
| fn reconcile_scenario_opts(state_dir: PathBuf) -> RuntimeOptions { | |
| let mut opts = default_opts(state_dir); | |
| opts.readiness.enabled = false; | |
| opts | |
| } | |
| pub(super) fn reconcile_scenario_opts(state_dir: PathBuf) -> RuntimeOptions { |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Closes the last remaining #99 follow-on: end-to-end integration coverage for all three reconciliation outcomes.
Three new scenarios in `engine::parallel::tests`
Each exercises `publish_package()` against a `tiny_http` mock registry with a fake cargo configured to exit ambiguously (exit 1, empty stderr — which `classify_publish_failure` maps to `Ambiguous`). Readiness polling is disabled so each reconcile site reduces to a single registry query, keeping request counts deterministic.
All three also assert on the event log (`PublishReconciling` + `PublishReconciled{outcome}`) read from disk — since the in-memory log is cleared after each flush.
What this closes
#99 status after merge:
#99 is fully closed.
Verification
Files
1 file changed (`crates/shipper/src/engine/parallel/tests.rs`), +359 / -2.
Related