alamb commented on code in PR #743:
URL:
https://github.com/apache/arrow-rs-object-store/pull/743#discussion_r3399028000
##########
src/aws/client.rs:
##########
@@ -1380,4 +1386,114 @@ mod tests {
.unwrap();
mock.shutdown().await;
}
+
+ /// A marker inserted into response extensions by [`MarkerMiddleware`]
+ #[derive(Clone, Debug, PartialEq)]
+ struct Marker(usize);
+
+ /// [`HttpService`] middleware that tags every response with a [`Marker`]
+ ///
+ /// [`HttpService`]: crate::client::HttpService
+ #[derive(Debug)]
+ struct MarkerMiddleware(reqwest::Client);
+
+ #[async_trait]
+ impl crate::client::HttpService for MarkerMiddleware {
+ async fn call(
+ &self,
+ req: crate::client::HttpRequest,
+ ) -> Result<HttpResponse, HttpError> {
+ let mut response = crate::client::HttpService::call(&self.0,
req).await?;
+ response.extensions_mut().insert(Marker(42));
+ Ok(response)
+ }
+ }
+
+ #[tokio::test]
+ async fn test_response_extensions_propagated() {
Review Comment:
I think we should add coverage of all new code, otherwise we coudl
potentially break this feature in some future refactoring but not know
> Not sure if you want to clutter up the other ones
I agree this test seems like a lot of boilerplate setup. It would be nice to
avoid that
How about writing something similar to
https://github.com/apache/arrow-rs-object-store/blob/e89f62b6508f6f65873ce6a5017cd0d5d7395184/src/integration.rs#L51-L53
And then calling it with an existing fixture, like
https://github.com/apache/arrow-rs-object-store/blob/a9a83f18071d9e4c5636d379aaa0513c1e06d0d4/src/aws/mod.rs#L688-L689
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
