test(metrics): test_gather_metrics depends on another test having run first - #788
Open
ohdearquant wants to merge 1 commit into
Open
test(metrics): test_gather_metrics depends on another test having run first#788ohdearquant wants to merge 1 commit into
ohdearquant wants to merge 1 commit into
Conversation
`test_gather_metrics` asserts that `gather_metrics()` contains "ruvector", but
the metrics are `lazy_static` and only register with the default Prometheus
registry when first dereferenced. The test touches none of them, so it passes
only when another test in the same process registered them first.
Under `cargo test` that happens by luck: all tests share one process and
`test_record_search` touches two metrics. Under a per-test-process runner it
fails deterministically, which is what CI uses:
cargo test -p ruvector-metrics 15 passed
cargo nextest run -p ruvector-metrics 14 passed, 1 failed
cargo nextest run -p ruvector-metrics -E 'test(test_gather_metrics)'
0 passed, 1 failed
The test now touches a metric itself and asserts on that metric's name rather
than the "ruvector" substring, so it no longer depends on execution order or
on sharing a process with another test.
ohdearquant
marked this pull request as ready for review
August 3, 2026 17:55
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.
What this fixes
ruvector-metrics::tests::test_gather_metricspasses only because another test in thesame process happened to run first.
The metrics are
lazy_static, so each one registers with the default Prometheusregistry the first time it is dereferenced.
test_gather_metricsdereferences none ofthem; it calls
gather_metrics()and asserts the output contains"ruvector". Undercargo testall tests share one process andtest_record_searchtouches two metrics,so by the time the assertion runs the registry is usually populated. Under a
per-test-process runner the registry is empty and the assertion fails every time.
The workflow runs
cargo nextest run, so this is a deterministic failure there, not aflake.
The change
The test touches a metric itself before gathering, and asserts on that metric's name
instead of the bare
"ruvector"substring. It no longer depends on execution order oron sharing a process with another test.
Verification
cargo nextest run -p ruvector-metrics: 15 passed.cargo nextest run -p ruvector-metrics -E 'test(test_gather_metrics)': 1 passed,which is the case that failed before.
cargo test -p ruvector-metrics: 15 passed.cargo clippy -p ruvector-metrics --all-targets -- -D warningsclean;cargo fmtapplied.
Context
A recent
Tests (core-and-rest)run reached its 240-minute cap while still compiling,so it never got to the test phase and this failure has not been reported there. #784,
#786 and #787 address what keeps that job from finishing; this is one of the results
waiting behind them.