Skip to content

test(reconcile): BDD integration scenarios for ambiguous publish outcomes (#99 follow-on)#119

Merged
EffortlessSteven merged 1 commit into
mainfrom
feat/99-reconcile-bdd-scenarios
Apr 17, 2026
Merged

test(reconcile): BDD integration scenarios for ambiguous publish outcomes (#99 follow-on)#119
EffortlessSteven merged 1 commit into
mainfrom
feat/99-reconcile-bdd-scenarios

Conversation

@EffortlessSteven

Copy link
Copy Markdown
Member

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.

Test Scenario Expected
`reconcile_bdd_ambiguous_resolves_to_published` cargo fails ambiguous; registry eventually returns 200 on reconcile poll `Published`, attempts=1, no retry
`reconcile_bdd_ambiguous_resolves_to_not_published_then_retries` cargo fails ambiguous; registry 404 throughout `Failed` after `max_attempts=2`; proves retry fall-through from NotPublished
`reconcile_bdd_resume_from_ambiguous_state_skips_republish` state pre-seeded with `PackageState::Ambiguous`; registry shows 200 on resume `Published`, attempts=0, cargo never invoked

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

  • `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

Files

1 file changed (`crates/shipper/src/engine/parallel/tests.rs`), +359 / -2.

Related

…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.
@coderabbitai

coderabbitai Bot commented Apr 17, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@EffortlessSteven has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 7 minutes and 12 seconds before requesting another review.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: bf39ea87-e17b-41e6-b79e-41240dc45b01

📥 Commits

Reviewing files that changed from the base of the PR and between 0589ba4 and e0d2d43.

📒 Files selected for processing (1)
  • crates/shipper/src/engine/parallel/tests.rs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/99-reconcile-bdd-scenarios

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@EffortlessSteven EffortlessSteven merged commit d8e7657 into main Apr 17, 2026
16 checks passed
@EffortlessSteven EffortlessSteven deleted the feat/99-reconcile-bdd-scenarios branch April 17, 2026 10:50

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Comment on lines +3811 to +3815
fn reconcile_scenario_opts(state_dir: PathBuf) -> RuntimeOptions {
let mut opts = default_opts(state_dir);
opts.readiness.enabled = false;
opts
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

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.

Suggested change
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

codecov Bot commented Apr 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant