cargo test fails on 70c78a7854577bc78e31623d699b9331eb84c192 (current master)

[artsiomkaltovich]

Bug Report

Version

commit: 70c78a7

Description

Running cargo test on a freshly clonned repo returns

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.11s

     Running tests/connection.rs (target/debug/deps/connection-4f76aaf09595a889)

running 4 tests
test connect_returns_err ... ok
test connect_handles_tls ... FAILED
test connect_returns_err_via_call_after_connected ... ok
test connect_lazy_reconnects_after_first_failure ... ok

failures:

---- connect_handles_tls stdout ----

thread 'connect_handles_tls' (4825732) panicked at tests/integration_tests/tests/connection.rs:58:53:
called `Result::unwrap()` on an `Err` value: tonic::transport::Error(Transport, ConnectError(Custom { kind: InvalidData, error: InvalidCertificate(UnknownIssuer) }))
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    connect_handles_tls

test result: FAILED. 3 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.21s

error: test failed, to rerun pass `-p integration-tests --test connection`

This is the investigation made by llm locally (in case it helps and isn't restricted by code of conduct :) )

Found it. The failure is unrelated to anything we changed (git status shows only the two plan/notes markdown files) — it's a latent feature-unification trap in the test suite itself.

What happens: connect_handles_tls dials the real https://github.com and expects the certificate to validate. The certificate chain GitHub serves is perfectly valid (I verified with openssl: leaf → Sectigo E36 intermediate → E46 root cross-signed by the universally trusted USERTrust ECC root — no corporate MITM, dates current). The problem is on the client side: tonic was compiled with TLS enabled but with an empty trust store, and rustls reports an empty store the same way as an unknown CA — InvalidCertificate(UnknownIssuer).

Why the trust store is empty: it's the intersection of three facts —

integration-tests declares tonic with default features only. Verified with cargo tree -p integration-tests -i tonic -e features: no TLS features at all when the crate builds alone.
You almost certainly ran a workspace-wide test command. Then Cargo unifies features across all members, and examples (default = full → tls = ["tonic/tls-ring"]), interop, tonic-xds, and grpc-benchmark all switch tonic's TLS on — but no crate in the entire workspace enables tls-native-roots or tls-webpki-roots (those strings only exist in tonic's own Cargo.toml).
With TLS on but no roots feature, [service/tls.rs:107-131](vscode-webview://137nuod52i939fngpcdmjbbvr8crja1s5rvmain7i13avndtvthc/tonic/src/transport/channel/service/tls.rs#L107-L131) builds RootCertStore::from_iter(trust_anchors) from an empty list and both root-loading blocks are compiled out — so the connector trusts nothing, and every server cert fails as UnknownIssuer.
Why CI doesn't catch it: [CI.yml:242](vscode-webview://137nuod52i939fngpcdmjbbvr8crja1s5rvmain7i13avndtvthc/.github/workflows/CI.yml#L242) runs cargo nextest run --workspace --all-features, which enables both root-store features, so the test passes there.

What to do:

To match CI locally: cargo test --workspace --all-features (or cargo nextest run --workspace --all-features). Note that plain cargo test -p integration-tests also can't pass this test — tonic then has no TLS at all and the https connect fails with a different transport error.
The test is arguably broken by design: it silently depends on --all-features builds and live network access to github.com. If you want to fix it upstream (nice small PR alongside #2790): either have integration-tests explicitly enable tonic/tls-ring + tonic/tls-native-roots for its dev build, or replace the live-GitHub call with a local TLS server using the repo's test certificates.

[dfawley]

It seems there's a new reason this fails, too: the stream cancellation tests added in #2791 require the "h2" feature to be set, which is not on by default -- @arjan-bal @LucioFranco I thought we agreed it would be on by default? -- so they fail, too. (I can't set it, either, because xds-client uses tonic 0.14 which didn't support the flag.)

When we run our tests in CI was pass --all-features, and all tests pass just fine on master@HEAD for me with this flag.

Is this a known issue @LucioFranco?

[arjan-bal]

@dfawley The h2 feature is enabled by default transitively through the following paths:

• [Existing] default -> transport -> server -> h2
• [New] default -> transport -> channel -> h2

#2791 ensured that users building without the server flag but with the channel flag still get a direct dependency on h2. (Previously, they already had h2 transitively pulled in via hyper). Additionally, they can explicitly use the h2 feature flag if they're not using the channel feature.

With this PR, there is no change in the actual dependency set being compiled; we are simply promoting h2 to a direct dependency for the channel feature where it was previously transitive.

The cargo tree output confirms this resolution:

$ cargo tree --features default -i h2 -p tonic
h2 v0.4.14
├── hyper v1.11.0
│   ├── hyper-timeout v0.5.2
│   │   └── tonic v0.14.6 (./tonic)    # transitive dependency
│   ├── hyper-util v0.1.20
│   │   ├── hyper-timeout v0.5.2 (*)
│   │   └── tonic v0.14.6 (./tonic)    # transitive dependency
│   └── tonic v0.14.6 (./tonic)        # transitive dependency
└── tonic v0.14.6 (./tonic)            # direct dependency

[arjan-bal]

@dfawley, I've raised a PR with the fix for you to review: #2805

I didn't encounter any issues with the tonic-xds crate, cargo test worked after enabling the tls-webpki-roots tonic feature in the integration tests crate.