Allow mocking out requests #220

tclem · 2025-07-25T23:44:59Z

Introduces the ability to register (client) mocks for generated rpc traits. This allows testing code with twirp clients where you don't have an http server running (and don't want to otherwise mock one out).

You can see an example usage in example/src/bin/client.rs.

I still have a little work to do in getting all the cargo features sorted out. My goal is that you can opt-in to all this with a standard feature flag, but otherwise this code won't be in your runtime binaries.

This is possible because of the work in #212.

Copilot

Pull Request Overview

This PR introduces client mocking capabilities for twirp-rs, allowing developers to test code with twirp clients without requiring a running HTTP server. The implementation generates mock handlers for RPC traits and provides a MockClientBuilder for easy test setup.

Adds a new mock.rs module with request/response encoding/decoding utilities
Implements MockHandler trait and MockClientBuilder for creating test clients with mock service handlers
Generates mock client structs automatically via the twirp-build code generation

Reviewed Changes

Copilot reviewed 8 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
crates/twirp/src/mock.rs	New module providing mock request/response utilities
crates/twirp/src/client.rs	Enhanced client with mock handler support and MockClientBuilder
crates/twirp-build/src/lib.rs	Code generation for mock client structs
crates/twirp/src/error.rs	Added rust error chaining to error conversions
example/proto/status/v1/status_api.proto	New example proto file for testing
example/Cargo.toml	Added dependencies for mock support
Makefile	Enhanced linting configuration

crates/twirp/src/mock.rs

Copilot · 2025-07-25T23:46:01Z

crates/twirp/src/client.rs

+}
+#[cfg(any(test, feature = "test-support"))]
+impl MockHandlers {
+    #[allow(dead_code)]


[nitpick] The #[allow(dead_code)] attribute suggests this function may not be used. Consider removing it if it's truly unused or documenting its intended usage.

Suggested change

#[allow(dead_code)]

#[allow(dead_code)]

/// Creates a new instance of `MockHandlers`.

///

/// This method is intended for use in testing or mock environments

/// where mock handlers need to be set up for specific hosts and services.

Copilot · 2025-07-25T23:46:01Z

crates/twirp/src/client.rs

+        }
+    }
+
+    #[allow(dead_code)]


[nitpick] The #[allow(dead_code)] attribute suggests this function may not be used. Consider removing it if it's truly unused or documenting its intended usage.

Suggested change

#[allow(dead_code)]

#[allow(dead_code)]

/// Adds a mock handler for a specific host and service.

///

/// This function is intended for use in testing scenarios or when the `test-support`

/// feature is enabled. It allows associating a mock handler with a host/service pair

/// for use in mock request handling.

crates/twirp-build/src/lib.rs

aneubeck · 2025-07-29T10:02:14Z

crates/twirp/src/mock.rs

+// TODO: Mock out other headers and extensions in the request and response?
+
+// NOTE: For testing and mocking only.
+pub async fn decode_request<I>(mut req: reqwest::Request) -> Result<Request<I>>


I guess it is easiest to go through serialization/deserialization...
In theory it should be possible to short-cut this by adding some mocking functionality to the service implementations of the twirp Client (I think).
But that's kind of an "optimization" which we can look into, once we have everything working, right?

I guess it is easiest to go through serialization/deserialization...

Yes. This the easiest for sure if we want to allow calling code to just use a twirp::Client.

I agree that it would be nice to optimize further in the future.

aneubeck · 2025-07-29T10:07:19Z

example/src/bin/client.rs

+    #[tokio::test]
+    async fn test_client_with_mock() {
+        let client = MockClientBuilder::new()
+            .with_mock(MockHaberdasherApiClient::new(Mock))


Question: the Mock clients are simply forwarding implementations, correct?
If so, would it make sense to rename them?

For an actual Mock implementation, I would expect that I can do more stuff, like registering functions for individual RPCs or so. Not saying that we have to add such a feature right now, but just wondering whether the Mock term should be replaced by something more specific.

That is a good point. 🤔 They are really just a shim to pass data to whatever struct implements the rpc trait... Let me play with some other names here.

aneubeck

Like it!

CleanCut · 2025-07-29T21:02:08Z

crates/twirp/src/client.rs

+
+#[async_trait]
+pub trait DirectHandler: 'static + Send + Sync {
+    fn service(&self) -> &str;


Suggestion: Consider making this method name more explicit, to avoid confusion that it may have something to do with the Service struct or tower::Service.

Suggested change

fn service(&self) -> &str;

fn service_name(&self) -> &str;

I'm going to leave this as-is because it is an internal detail and for use client side (so no confusion with tower).

example/src/bin/client.rs

example/proto/status/v1/status_api.proto

jorendorff

I don't know about the naming. I guess "direct" is the new name? "Mock" communicates what we're doing better though. (Edit:) OK, I see Alex's comment. I don't have a better suggestion.

I noticed even ClientBuilder::direct() doesn't let you build a Client that simply never talks to the network. If an unexpected .with_host happens, you'll end up attempting HTTP requests. Our integration tests for Blackbird really don't want that. I guess it shouldn't happen in practice. But if it does, we're not making it easy on ourselves to debug it.

jorendorff · 2025-07-30T13:47:02Z

crates/twirp-build/src/lib.rs

+        let mut method_matches = Vec::with_capacity(service.methods.len());
+        for m in &service.methods {
+            let name = &m.name;
+            let method = &m.proto_name;
+            method_matches.push(quote! {
+                #method => {
+                    twirp::details::encode_response(self.inner.#name(twirp::details::decode_request(req).await?).await?)
+                }
+            });


Style suggestion: Use iterators and unpacking of the Method:

Suggested change

let mut method_matches = Vec::with_capacity(service.methods.len());

for m in &service.methods {

let name = &m.name;

let method = &m.proto_name;

method_matches.push(quote! {

#method => {

twirp::details::encode_response(self.inner.#name(twirp::details::decode_request(req).await?).await?)

}

});

let method_matches = service.methods

.iter()

.map(|Method { name, proto_name, .. }| quote! {

#proto_name => {

twirp::details::encode_response(self.inner.#name(twirp::details::decode_request(req).await?).await?)

}

})

.collect::<Vec<_>>();

I'm going to leave this using the more imperative style to be consistent with the rest of the code gen.

jorendorff · 2025-07-30T13:59:15Z

crates/twirp/src/client.rs

@@ -54,6 +103,7 @@ pub struct Client {
    http_client: reqwest::Client,
    inner: Arc<ClientRef>,
    host: Option<String>,
+    handlers: Option<RequestHandlers>,


I think this should be inside the ClientRef so that it's reference counted. That saves cloning the hash map in with_host.

tclem · 2025-07-30T14:47:52Z

I don't know about the naming.

I don't either... passthrough? forwarding? request handlers? @aneubeck is correct that these aren't true mocks, though the thing you pass in that implements the rpc trait is close. I think we're also realizing that these have potential uses beyond testing/mocking: e.g. when you want to use the twirp rpc traits as a contract, but co-locate some of the distributed system and just have a method call instead of an http call.

I noticed even ClientBuilder::direct() doesn't let you build a Client that simply never talks to the network.

Good point. I just made an attempt to fix this as well, let me know if you think that helps.

tclem added 14 commits July 23, 2025 14:17

Sketch out how this might work

f1695d6

This is working

005eec1

Remove hand written code that is now generated

90db91d

Notes

a62e87a

Include a more detailed request

adbde83

Fix a lint

ca69950

Multiple mocks

e0c7187

Match on fully qualified path

6e0610b

Builder for ergonomics

d540a47

Slightly nicer registration of mocks

c93b0f8

Support per host mocking

32bece3

Dedicated module, better error

dc34ace

Minor cleanup

cab0322

Make some of this conditional

dd8234c

Copilot AI review requested due to automatic review settings July 25, 2025 23:45

tclem requested a review from a team as a code owner July 25, 2025 23:45

Copilot AI reviewed Jul 25, 2025

View reviewed changes

aneubeck reviewed Jul 29, 2025

View reviewed changes

aneubeck approved these changes Jul 29, 2025

View reviewed changes

tclem added 4 commits July 29, 2025 12:37

Regoranize and dont require test-support

e9157e1

Continue to refine

2aad20e

Use a single builder

d403772

More docs

cd8e46a

CleanCut approved these changes Jul 29, 2025

View reviewed changes

jorendorff reviewed Jul 30, 2025

View reviewed changes

example/proto/status/v1/status_api.proto Outdated Show resolved Hide resolved

jorendorff approved these changes Jul 30, 2025

View reviewed changes

tclem added 3 commits July 30, 2025 07:02

Remove stale comment

963db4f

Move to inner

c3d424e

Avoid some confusion

82334c0

tclem added 3 commits July 30, 2025 08:52

Merge remote-tracking branch 'origin/main' into tclem/mocks

5dad8c6

++0.11.0

d7946c7

Not necessary

cfe2008

tclem added this pull request to the merge queue Jul 30, 2025

Merged via the queue into main with commit 3dcba2c Jul 30, 2025
5 checks passed

tclem deleted the tclem/mocks branch July 30, 2025 20:33

github-actions bot mentioned this pull request Jul 31, 2025

chore: release v0.9.0 #227

Merged

-    #[allow(dead_code)]
+    #[allow(dead_code)]
+    /// Creates a new instance of `MockHandlers`.
+    ///
+    /// This method is intended for use in testing or mock environments
+    /// where mock handlers need to be set up for specific hosts and services.

-    #[allow(dead_code)]
+    #[allow(dead_code)]
+    /// Adds a mock handler for a specific host and service.
+    ///
+    /// This function is intended for use in testing scenarios or when the `test-support`
+    /// feature is enabled. It allows associating a mock handler with a host/service pair
+    /// for use in mock request handling.

Allow mocking out requests #220

Allow mocking out requests #220

Uh oh!

Conversation

tclem commented Jul 25, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI Jul 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aneubeck left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jorendorff left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tclem commented Jul 30, 2025

Uh oh!

Uh oh!

Uh oh!

jorendorff left a comment •

edited

Loading