Skip to content

Extending Interop tests#2642

Open
znaczko wants to merge 11 commits into
grpc:masterfrom
znaczko:interop_tests
Open

Extending Interop tests#2642
znaczko wants to merge 11 commits into
grpc:masterfrom
znaczko:interop_tests

Conversation

@znaczko
Copy link
Copy Markdown

@znaczko znaczko commented May 13, 2026

Motivation
The current interop client and server test harnesses in Tonic lacked support for several core command-line input parameters defined in the official gRPC interoperability test specification.

Solution
This PR implements all remaining interop test specifications, fully handles network configuration inputs, and enables dynamic ASCII/Binary metadata injection.

@znaczko znaczko changed the title Extending Interop tests and Extending Interop tests May 13, 2026
Copy link
Copy Markdown

@mswierq mswierq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have assumed that some of the newly added Opts members are yet unused, since this PR does not provide the interop test cases that cover scenarios that would use those. The main goal here is to provide support for handling all the interop args to be in parity with grpc implementation and the missing test cases will be provided later on?

Comment thread interop/src/bin/client.rs

let test_cases = matches.test_case;

let additional_metadata = if let Some(ref am) = matches.additional_metadata {
Copy link
Copy Markdown

@mswierq mswierq May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Opts should keep Option<MetadataMap>. This parsing code could be implemented as a FromStr trait for MetadataMapWrapper, where MetadataMapWrapper wraps MetadataMap in order to satisfy the orphan rule.

Comment thread interop/src/bin/client.rs
use tonic::transport::ClientTlsConfig;
use tonic::transport::Endpoint;

#[allow(dead_code)]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add a comment which members and why are currently unused or apply #[allow(dead_code)] only to the members that are unused and make a general comment why they are currently unused

format!("result={:?}", result)
));

drop(tx);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think that we need an explicit drop here, tx is going out of the scope here and should be dropped automatically. Unless rx is still awaited somehow and the default order of dropping local variables would hang this function. If so, please add a comment to explain it.

));
}

drop(tx);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please look at the comment here, it seems to be similar situation

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.

2 participants