Skip to content

Commit

Permalink
refactor(app/test): consolidate anonymous futures
Browse files Browse the repository at this point in the history 
we create `async move {}` blocks in multiple places, without any clear
benefit.

this commit consolidates them into one place: when we spawn a task onto
the `JoinSet`.

Signed-off-by: katelyn martin <kate@buoyant.io>
  • Loading branch information
@cratelyn
cratelyn committed Dec 7, 2024
1 parent 15d1b09 commit 4a06bf8
Showing 1 changed file with 9 additions and 15 deletions.
24 changes: 9 additions & 15 deletions linkerd/app/test/src/http_util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use crate::{
app_core::{svc, Error},
io, ContextError,
};
use futures::FutureExt;
use hyper::{body::HttpBody, Body};
use tokio::task::JoinSet;
use tower::ServiceExt;
Expand All @@ -24,37 +23,32 @@ pub async fn connect_and_accept(
) -> (SendRequest<Body>, JoinSet<Result<(), Error>>) {
tracing::info!(settings = ?client_settings, "connecting client with");
let (client_io, server_io) = io::duplex(4096);
let proxy = async move {
let res = server.oneshot(server_io).await;
tracing::info!(?res, "proxy serve task complete");
res
};

let (client, conn) = client_settings
.handshake(client_io)
.await
.expect("Client must connect");
let client_bg = conn.map(|res| {
tracing::info!(?res, "Client background complete");
res.map_err(Error::from)
});

let mut bg = tokio::task::JoinSet::new();
bg.spawn(
async move {
proxy
let res = server
.oneshot(server_io)
.await
.map_err(ContextError::ctx("proxy background task failed"))
.map_err(Error::from)
.map_err(ContextError::ctx("proxy background task failed"))?;

Check failure on line 38 in linkerd/app/test/src/http_util.rs

View workflow job for this annotation

GitHub Actions / rust 

error: this let-binding has unit value
  --> linkerd/app/test/src/http_util.rs:35:13
   |
35 | /             let res = server
36 | |                 .oneshot(server_io)
37 | |                 .await
38 | |                 .map_err(ContextError::ctx("proxy background task failed"))?;
   | |_____________________________________________________________________________^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_unit_value
   = note: `-D clippy::let-unit-value` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(clippy::let_unit_value)]`
help: omit the `let` binding
   |
35 ~             server
36 +                 .oneshot(server_io)
37 +                 .await
38 +                 .map_err(ContextError::ctx("proxy background task failed"))?;
   |
help: variable `res` of type `()` can be replaced with explicit `()`
  --> /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/tracing-0.1.41/src/macros.rs:2828:85
   |
282|             @ { $($out),*, (&$next, $crate::__macro_support::Option::Some(&debug(&$(()$k).+) as &dyn Value)) },
   |                                                                                     ++
help: variable `res` of type `()` can be replaced with explicit `()`
  --> /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/tracing-0.1.41/src/macros.rs:2828:85
   |
282|             @ { $($out),*, (&$next, $crate::__macro_support::Option::Some(&debug(&$(()$k).+) as &dyn Value)) },
   |                                                                                     ++

Check failure on line 38 in linkerd/app/test/src/http_util.rs

View workflow job for this annotation

GitHub Actions / rust 

error: this let-binding has unit value
  --> linkerd/app/test/src/http_util.rs:35:13
   |
35 | /             let res = server
36 | |                 .oneshot(server_io)
37 | |                 .await
38 | |                 .map_err(ContextError::ctx("proxy background task failed"))?;
   | |_____________________________________________________________________________^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_unit_value
   = note: `-D clippy::let-unit-value` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(clippy::let_unit_value)]`
help: omit the `let` binding
   |
35 ~             server
36 +                 .oneshot(server_io)
37 +                 .await
38 +                 .map_err(ContextError::ctx("proxy background task failed"))?;
   |
help: variable `res` of type `()` can be replaced with explicit `()`
  --> /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/tracing-0.1.41/src/macros.rs:2828:85
   |
282|             @ { $($out),*, (&$next, $crate::__macro_support::Option::Some(&debug(&$(()$k).+) as &dyn Value)) },
   |                                                                                     ++
help: variable `res` of type `()` can be replaced with explicit `()`
  --> /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/tracing-0.1.41/src/macros.rs:2828:85
   |
282|             @ { $($out),*, (&$next, $crate::__macro_support::Option::Some(&debug(&$(()$k).+) as &dyn Value)) },
   |                                                                                     ++
tracing::info!(?res, "proxy serve task complete");
Ok(())
}
.instrument(tracing::info_span!("proxy")),
);
bg.spawn(
async move {
client_bg
let res = conn
.await
.map_err(ContextError::ctx("client background task failed"))
.map_err(Error::from)
.map_err(Error::from)?;

Check failure on line 49 in linkerd/app/test/src/http_util.rs

View workflow job for this annotation

GitHub Actions / rust 

error: this let-binding has unit value
  --> linkerd/app/test/src/http_util.rs:46:13
   |
46 | /             let res = conn
47 | |                 .await
48 | |                 .map_err(ContextError::ctx("client background task failed"))
49 | |                 .map_err(Error::from)?;
   | |_______________________________________^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_unit_value
help: omit the `let` binding
   |
46 ~             conn
47 +                 .await
48 +                 .map_err(ContextError::ctx("client background task failed"))
49 +                 .map_err(Error::from)?;
   |
help: variable `res` of type `()` can be replaced with explicit `()`
  --> /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/tracing-0.1.41/src/macros.rs:2828:85
   |
282|             @ { $($out),*, (&$next, $crate::__macro_support::Option::Some(&debug(&$(()$k).+) as &dyn Value)) },
   |                                                                                     ++
help: variable `res` of type `()` can be replaced with explicit `()`
  --> /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/tracing-0.1.41/src/macros.rs:2828:85
   |
282|             @ { $($out),*, (&$next, $crate::__macro_support::Option::Some(&debug(&$(()$k).+) as &dyn Value)) },
   |                                                                                     ++

Check failure on line 49 in linkerd/app/test/src/http_util.rs

View workflow job for this annotation

GitHub Actions / rust 

error: this let-binding has unit value
  --> linkerd/app/test/src/http_util.rs:46:13
   |
46 | /             let res = conn
47 | |                 .await
48 | |                 .map_err(ContextError::ctx("client background task failed"))
49 | |                 .map_err(Error::from)?;
   | |_______________________________________^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_unit_value
help: omit the `let` binding
   |
46 ~             conn
47 +                 .await
48 +                 .map_err(ContextError::ctx("client background task failed"))
49 +                 .map_err(Error::from)?;
   |
help: variable `res` of type `()` can be replaced with explicit `()`
  --> /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/tracing-0.1.41/src/macros.rs:2828:85
   |
282|             @ { $($out),*, (&$next, $crate::__macro_support::Option::Some(&debug(&$(()$k).+) as &dyn Value)) },
   |                                                                                     ++
help: variable `res` of type `()` can be replaced with explicit `()`
  --> /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/tracing-0.1.41/src/macros.rs:2828:85
   |
282|             @ { $($out),*, (&$next, $crate::__macro_support::Option::Some(&debug(&$(()$k).+) as &dyn Value)) },
   |                                                                                     ++
tracing::info!(?res, "client background complete");
Ok(())
}
.instrument(tracing::info_span!("client_bg")),
);
Expand Down

0 comments on commit 4a06bf8

Please sign in to comment.