Skip to content

Install the test crypto provider per binary in eksetasis and syndesmos instead of per call site #737

Description

@forkwright

Finding

Installing the rustls crypto provider per test site took four CI rounds to converge, and the reason is a specific shape rather than carelessness.

It is not "a test forgot to spawn a mock server." It is: a shared local helper funnels most tests through a safe path, while a minority in the same file construct at the site and bypass the precondition entirely.

That shape recurred independently in two crates:

  • eksetasisclient/cardigann/tests.rs has 52 tests behind a three-tier client()client_with_settings()client_with_sessions() wrapper, and two that call CardigannClient::new directly.
  • syndesmosplex/collections.rs and plex/stats.rs construct a real PlexClient at eight sites while containing zero literal "reqwest". A keyword sweep clears both files. They happen to be safe, but only reading every body establishes that.

Neither was found by grep. Both needed a test-by-test read.

Why enumeration is not the durable answer

The full enumeration is done — all eight reqwest-depending crates, every test individually verdicted. (The eighth, theatron/desktop, had never been checked; it constructs no client.)

But an enumeration is a one-time proof. It does not protect a test written next month that adds a ninth PlexClient::new call, or a 53rd cardigann test that bypasses the wrapper, without also learning that file's local convention. The next such test fails at runtime with a panic that names rustls rather than the missing call.

Proposal: #[ctor], scoped to two crates

One #[cfg(test)] #[ctor] function per crate:

#[ctor]
fn install_test_crypto_provider() {
    let _ = rustls::crypto::ring::default_provider().install_default();
}

This removes the precondition rather than satisfying it repeatedly. There is no "was this construction preceded by the right call" question left, because the answer is unconditionally yes before any test body runs. The existing test_support::install_test_crypto_provider() and its call sites can then be deleted.

Scope: eksetasis and syndesmos only.

Those are the two crates that have already produced a bypass in the wild, and they carry the most reqwest-touching test files (7 and 6) with the most varied construction idioms — three-tier wrappers, direct hardcoded-URL tests, mixed mock/real patterns. Exactly the conditions that generate this class.

epignosis, komide, prostheke and theatron/core each have a single narrow, already-centralised construction path and no history of a bypass. Adding ctor there defends against a failure that has not occurred on a surface small enough to keep visually verified at review.

One important constraint, worth recording

#[cfg(test)] code is per-crate-compilation. A #[ctor] declared in epignosis's test code would not protect archon's tests, even though archon calls epignosis's real constructors — when archon depends on epignosis as a normal library, epignosis's test code is never compiled into archon's test binary. The init has to live in whichever crate owns the test, not the one owning the constructor.

That is why this is not a single fleet-wide fix, and why the archon work had to be done in archon.

Land it one crate per PR

ctor has no compiled precedent in this workspace. A mistake in the macro shape trades a runtime-panic round for a compile-failure round, which is worse. One PR per crate keeps a shape error isolated and trivially bisected.

Done when: both crates install unconditionally per test binary, the per-site helper and its call sites are removed, and a deliberately-added test that constructs a client without any preceding install passes.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions