Skip to content

integration-tests workspace drifts silently from root workspace changes #1043

@zancas

Description

@zancas

Problem

integration-tests/ is a separate cargo workspace, excluded from the root workspace (see integration-tests/Cargo.toml; landed in #954). This decoupling creates three reinforcing sources of friction:

  1. Root-workspace changes silently break integration-tests. cargo check --all-features --all-targets --tests at the repo root does not compile the integration-tests crate. Breaking API changes — e.g., a sync method becoming async, a previously-pub type becoming pub(crate) — land via PRs whose CI never exercises the dependent test-utility code. The rot is discovered only when someone runs makers container-test --manifest-path integration-tests/Cargo.toml -E ....

  2. integration-tests/Cargo.lock is gitignored (.gitignore:5). Fresh clones and container runs resolve transitive deps from scratch. If any transitive dep has no non-yanked version satisfying its requirement, the resolve fails — even though the root workspace's committed Cargo.lock has a pinned-yanked version that works fine. The root lockfile tolerates yanked versions it already knows about; a fresh resolve doesn't.

  3. Container-test is the only canary, and it's slow — image build + podman volume setup + full test run. Developers are disincentivized to run it pre-commit, so the rot accumulates unchecked between container runs.

Concrete friction from PR #642

Merging dev into feat_rpc-getblockhash surfaced two distinct symptoms of the above:

(1) Yanked transitive dep

zcash_protocol ^0.7.2 requires core2 ^0.3; all core2 0.3.x are yanked. Root Cargo.lock has core2 0.3.3 pinned (still works because cargo tolerates yanked versions already in a lockfile). integration-tests had no lockfile, tried to fresh-resolve, failed with failed to select a version for the requirement \core2 = "^0.3"``.

Workaround applied on feat_rpc-getblockhash: added a phantom direct dep core2 = \"=0.3.3\" in integration-tests/Cargo.toml. This is both weird (integration-tests doesn't use core2) and fragile (breaks if core2 0.3.4 is released, or if zcash_protocol updates to ^0.4).

(2) Silent API drift in zaino-testutils

A dev commit (appears to be 55500407 move best_chaintip to chainindex method...) changed:

  • ChainIndex::snapshot_nonfinalized_state(&self) -> Self::Snapshotasync fn snapshot_nonfinalized_state(&self) -> Result<ChainIndexSnapshot, Self::Error>
  • NonfinalizedBlockCacheSnapshot: pubpub(crate), with #[allow(private_interfaces)] on the enum variant field

The field best_tip was never reachable from outside zaino-state afterwards. integration-tests/zaino-testutils/src/lib.rs:647,663 and integration-tests/tests/chain_cache.rs:271,324,368,429,435,491,506,508,567,681… all still called .snapshot_nonfinalized_state().best_tip.height and nothing flagged them as broken until someone tried to compile the integration-tests workspace.

Workaround applied on feat_rpc-getblockhash: zaino-testutils patched to use the new ChainIndex::best_chaintip(&snapshot) public API. chain_cache.rs sites remain broken at time of writing.

Suggested remedies

Not mutually exclusive; pick a subset:

  • Commit integration-tests/Cargo.lock. Delete the .gitignore entry, run cargo generate-lockfile --manifest-path integration-tests/Cargo.toml, commit the result. Matches the root-workspace pattern. Kills friction source (2); converts yanked-dep surprises from per-clone recurring failures into a one-time fix.

  • Add an integration-tests compile gate to CI. On every PR, run cargo check --manifest-path integration-tests/Cargo.toml --all-targets --tests (check only — no test execution). Fast enough for per-PR CI. Catches API-drift (friction source 1) at review time, not weeks later.

  • Reconsider the workspace split. The split (divide workspaces by promoting integration-tests to become its own *excluded* cargo workspace #954) was motivated by dependency hygiene. But if integration-tests consumes internal types from zaino-state (like ChainIndex::Snapshot), the split trades CI safety for dep isolation. Worth re-evaluating whether the tradeoff still pays.

  • Document the workflow. At minimum, the CONTRIBUTING guide should make clear that root-workspace green ≠ integration-tests green, and that API changes to public traits consumed by zaino-testutils or integration-tests/tests/* need explicit cross-workspace check before merge.

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions