Skip to content

test(metrics): test_gather_metrics depends on another test having run first - #788

Open
ohdearquant wants to merge 1 commit into
ruvnet:mainfrom
ohdearquant:fix/metrics-gather-test-isolation
Open

test(metrics): test_gather_metrics depends on another test having run first#788
ohdearquant wants to merge 1 commit into
ruvnet:mainfrom
ohdearquant:fix/metrics-gather-test-isolation

Conversation

@ohdearquant

@ohdearquant ohdearquant commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

What this fixes

ruvector-metrics::tests::test_gather_metrics passes only because another test in the
same process happened to run first.

The metrics are lazy_static, so each one registers with the default Prometheus
registry the first time it is dereferenced. test_gather_metrics dereferences none of
them; it calls gather_metrics() and asserts the output contains "ruvector". Under
cargo test all tests share one process and test_record_search touches 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.

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 workflow runs cargo nextest run, so this is a deterministic failure there, not a
flake.

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 or
on 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 warnings clean; cargo fmt
    applied.

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.

`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
ohdearquant marked this pull request as ready for review August 3, 2026 17:55
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