ci(test): add lance-graph-contract unit tests to the test gate#328
Merged
Conversation
The Style Check / clippy job already type-checks crates/lance-graph-contract --lib --tests, but clippy does NOT execute tests — it just compiles them. So a logically-broken assertion compiles green, ships, and is caught only after merge. PR #322 hit exactly this trap. Its log_norm_growth_negative_when_m_attenuates test asserted that an attenuating M with seed = I produces NEGATIVE log-norm growth, but ‖log(propagated)‖²_F is structurally non-negative for any non-identity propagated, so the assertion was unsatisfiable from the start. Clippy passed, the PR shipped, and the regression sat red on every subsequent PR until PR #326 fixes the test body. Cost of closing this hole is minimal: - crates/lance-graph-contract is the zero-dep trait crate (per CLAUDE.md and the Cargo.toml), so the test build is fast. - Same toolchain, same Swatinem cache, same protobuf-compiler as the existing lance-graph build — no extra setup. - Test execution is sub-second (350 tests, all logic / no I/O). Sequencing: this PR depends on PR #326 (the test-body fix) landing first. If this lands before #326, the new step turns CI red on main. Mergers should land #326, then this. Co-authored-by: AdaWorldAPI <AdaWorldAPI@users.noreply.github.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
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.
Summary
Closes the gap that let PR #322's
log_norm_growth_negative_when_m_attenuatesship green and sit red onmainfor almost a day.The Style Check / clippy job already type-checks
crates/lance-graph-contract --lib --tests, but clippy does not execute tests — it just compiles them. So a logically-broken assertion compiles green, ships, and is caught only after merge.PR #322 hit exactly this trap. Its
log_norm_growth_negative_when_m_attenuatesasserted that an attenuatingMwithseed = Iproduces NEGATIVE log-norm growth, but‖log(propagated)‖²_Fis structurally non-negative for any non-identitypropagated, so the assertion was unsatisfiable from the start. Clippy passed, the PR shipped, and the regression sat red on every subsequent PR until PR #326 fixes the test body.Fix
Add one step to the existing
testjob inrust-test.yml:It rides on the existing job's setup — same toolchain, same Swatinem cache, same
protobuf-compilerinstall — so the marginal cost is just the build (~1 s, zero-dep crate perCLAUDE.md) plus test execution (sub-second, 350 logic-only tests).Cost / blast radius
test (stable)job. No new job, no new runner, no new cache.Sequencing
⚠ This PR depends on PR #326 landing first. If it lands before #326, the new step turns CI red on
main, because PR #322's broken test is still onmaintoday (349 passed; 1 failed). Recommended merge order:mainbecomes 350/350 green.If preferred, this can also be rebased on top of #326 directly, but keeping them separate makes the intent clear: #326 = correctness fix, #327 (the rustfmt drift) = style fix, this = process fix.
What this PR does NOT do
test-with-coveragealready runs againstlance-graphonly; widening coverage to the contract crate is a follow-up.cognitive-shader-driver,lance-graph-callcenter,lance-graph-plannerare still un-test-gated by CI; bringing them in is a separate scoping decision (some have heavier dep trees that would cost real CI time).Companion PRs