fix(ci): run Rust integration tests, not just unit tests - #202
Merged
beinan merged 1 commit intoJul 26, 2026
Conversation
- cargo test -p lance-context-core -p lance-context-master --lib + cargo test --workspace --all-targets `--lib` runs unit tests only. The preceding `--no-run` step compiled the crates/lance-context-core/tests/*.rs integration tests and then discarded them, so they never executed. The CI log from lance-format#199 shows it plainly — 167 (core lib) + 14 (master lib) + 1 (the one etcd test named explicitly), and no line for the 5 WAL-merge concurrency tests that PR added. They merged into main having never run in CI, as had wal_merge_generation_cleanup.rs before them. `--workspace` additionally covers lance-context-api, -server, -client, -metrics and the facade crate, none of which were tested at all. Local run of the new command: 250 tests pass, against 182 under `--lib`. Runtime ~7 minutes against the existing 30-minute timeout. Same class of failure as lance-format#195 (Python CI collecting the wrong directory), in a different mechanism: tests that exist, compile, and are silently not run. Co-Authored-By: Claude <noreply@anthropic.com>
beinan
added a commit
that referenced
this pull request
Jul 27, 2026
#211)⚠️ **Stacked on #209 → #210.** Both of those commits are included here. Merge order: **#209 → #210 → this**. Once they land, this diff reduces to the UI commit alone. ## Why this exists #210 removes cold experiments from the stats table — that's what keeps a scan round proportional to *active* work. But on its own it also removes them from `GET /experiments`, which is only acceptable once they stay findable another way. **This PR is what makes retirement safe to turn on.** | | behaviour | |---|---| | **default list** | the stats table — experiments written recently enough not to have been retired | | **search** | stats table **plus the registry** for names it no longer holds; cold matches observed on demand | | **detail** | on a stats miss, resolve via registry and observe on demand rather than returning 404 | Retirement therefore removes an experiment from the *default view* and **never makes it unfindable**. The default is also the right one at scale on its own merits: a flat list of every experiment ever created is neither renderable nor interesting once there are tens of thousands. ## Two deliberate decisions **1. `observe_cold` does not write a stats row.** Reading about a retired experiment must not make it hot again — otherwise **browsing the UI would silently undo retirement**, and the table would creep back toward holding everything, which is exactly the state retirement exists to prevent. It also takes no `MasterState`, so it *structurally* cannot write one. A refactor that hands it one has to justify itself. There's a test asserting the no-rehydration property. **2. Search bounds its on-demand observations by page size.** A query matching thousands of cold experiments must not open thousands of datasets to render one page. ## Known gap — stating it rather than hiding it The two handlers changed here live in `routes.rs`, whose tests **all require etcd and are `#[ignore]`d**, so CI does not cover them. `observe_cold` is tested in `scanner.rs`, which CI does run. So the registry-fallback logic in the handlers is verified locally but not by CI. That's a pre-existing gap in the test structure, worth fixing as its own change rather than as a fourth concern here — and it's the kind of blind spot that only became visible after #202 made CI actually run integration tests. ## Testing Full workspace: 178 core + 30 master + 51 server + 5 concurrency + 1 merge-cleanup, all pass. clippy `-D warnings` and fmt clean. ## The complete picture This finishes the line of work from the original report (`/experiments` at 30–43s): | | problem | fix | |---|---|---| | **#207** | write amplification — 100 versions/round | one snapshot commit; **84× fewer versions** | | **#209** | scan cost — full observe of everything, every round | skip unchanged; cold = one open | | **#210** | table size — held every experiment ever created | retire after 7 days idle, drained first | | **this** | retirement made cold experiments invisible | active-N default + registry-backed search | | **#208** | all of the above failing silently | maintenance failure metrics | 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>
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.
--libruns unit tests only. The preceding--no-runstep compiled thecrates/lance-context-core/tests/*.rsintegration tests and then discarded them — they never executed.Evidence
The CI log from #199:
No line for the 5 WAL-merge concurrency tests that PR added. They merged into
mainhaving never run in CI — as hadwal_merge_generation_cleanup.rsbefore them.--workspaceadditionally coverslance-context-api,-server,-client,-metricsand the facade crate, none of which were tested at all.Impact
250 tests pass under the new command, against 182 under
--lib. Runtime ~7 min against the existing 30 min timeout.Same class of failure as #195 (Python CI collecting the wrong directory), in a different mechanism: tests that exist, compile, and are silently not run. I flagged
--libin an earlier review and then failed to re-check it when adding integration tests in #199 — a test that never executes is the same as no test.Note
This PR is only the CI change, deliberately. It was originally bundled with a
ContextStoreconcurrency fix in #201; on review that fix addresses a scenario that is not currently reachable in single-process deployments, so it should be argued on its own merits rather than riding along with an unambiguous CI repair. #201 will be rescoped.🤖 Generated with Claude Code