Skip to content

test(retrieve): install rustls CryptoProvider in reranker tests#12

Open
doctatortot wants to merge 1 commit into
ourmem:mainfrom
doctatortot:upstream-clean/reranker-crypto
Open

test(retrieve): install rustls CryptoProvider in reranker tests#12
doctatortot wants to merge 1 commit into
ourmem:mainfrom
doctatortot:upstream-clean/reranker-crypto

Conversation

@doctatortot
Copy link
Copy Markdown
Contributor

Summary

Mirrors the PR #6 pattern for retrieve::reranker tests. Adds an own install_crypto_provider() helper (guarded by its own Once) and calls it from the three reranker tests that construct a reqwest::Client.

Why

The install_crypto_provider() from PR #6 lives in api::tests::setup_app(). Because rustls's provider install is process-global and guarded by Once, the reranker tests appear to pass under cargo test --lib — but only because the api tests happen to run first in the same test binary, install the provider, and the side effect persists.

Run in isolation, the failure is immediate:

$ cargo test --lib retrieve::reranker::
thread 'retrieve::reranker::tests::test_new_with_endpoint' panicked at
  reqwest-0.13.2/src/async_impl/client.rs:2461:5:
  no process-level CryptoProvider available

So the green-CI claim from 3472c74 is order-dependent — sound today, sound under cargo test --lib, but breaks the moment anyone scopes the test run or cargo's scheduler changes.

After this PR

$ cargo test --no-fail-fast --lib retrieve::reranker::
test result: ok. 8 passed; 0 failed; 0 ignored; 0 measured

The helper is intentionally a copy rather than a shared utility — two separate Once guards in two test modules is safe (both call install_default(), which itself returns Err after the first call; we ignore that). Promoting it to a shared mod test_util is fine if you'd prefer; happy to revise.

🤖 Generated with Claude Code

PR ourmem#6 installed a process-global rustls CryptoProvider in the api
test setup_app() so the ~20 api tests stop panicking with "no
process-level CryptoProvider available." That fix happened to also
unblock the 3 retrieve::reranker tests that call reqwest::Client::new()
— but only because api tests run before reranker tests in the same
test binary, install the provider as a side effect, and the static
Once is shared.

Running `cargo test --lib retrieve::reranker::` in isolation still
panics: no api test runs first, no provider is installed. That makes
the green-CI claim from 3472c74 order-dependent.

Mirror the PR ourmem#6 pattern here: own install_crypto_provider() helper
guarded by its own Once, called at the top of each test that
constructs a Reranker. Now `cargo test --lib retrieve::reranker::`
passes 8/8 standalone.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
yhyyz pushed a commit that referenced this pull request May 20, 2026
`lance-encoding`'s build.rs invokes `protoc` to generate prost bindings.
Without it `cargo clippy` and `cargo test` fail at compile time with
"Could not find `protoc`. If `protoc` is installed, try setting the
`PROTOC` environment variable..."

This was masked previously because CI bailed at the `cargo fmt --check`
step (229 sites of fmt drift across upstream main). After the fmt
cleanup in 508e9e6, PRs now reach the clippy step and surface this dep.
Three currently-open PRs (#11 #12 #14) hit this on their last CI runs.

Adding `apt-get install protobuf-compiler` before the rust toolchain
step matches the standard pattern for Rust projects with prost-build
deps (see lancedb, datafusion, deltalake CI configs).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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