From 6521e6660acf00047ea9e0e77494c4236d011121 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 3 Oct 2023 11:58:44 -0700 Subject: [PATCH 1/7] Refactor HTTP tests to be less flaky and more robust This commit refactors the wasi-http tests we have in this repository with a few changes: * Each test now uses a dedicated port for the test. The port 0 is bound and whatever the OS gives is used for the duration of the test. This prevents tests from possibly interfering with each other. * Server spawning is abstracted behind a single `Server` type now which internally has http1/2 constructors. * Timeouts for server shutdown are removed since they were failing in CI and are likely best handled for each test individually if necessary. As a minor cleanup the `tokio` usage in guest programs was removed as it was boilerplate in this case. This shouldn't affect the runtimes of tests, however. --- Cargo.lock | 1 - crates/test-programs/src/http_server.rs | 264 +++++------------- .../tests/wasi-http-components-sync.rs | 77 +++-- .../tests/wasi-http-components.rs | 77 +++-- .../test-programs/wasi-http-tests/Cargo.toml | 1 - .../src/bin/outbound_request_get.rs | 14 +- .../bin/outbound_request_invalid_dnsname.rs | 7 +- .../src/bin/outbound_request_invalid_port.rs | 7 +- .../bin/outbound_request_invalid_version.rs | 15 +- .../src/bin/outbound_request_large_post.rs | 12 +- .../src/bin/outbound_request_post.rs | 12 +- .../src/bin/outbound_request_put.rs | 21 +- .../bin/outbound_request_unknown_method.rs | 7 +- .../outbound_request_unsupported_scheme.rs | 7 +- .../test-programs/wasi-http-tests/src/lib.rs | 23 +- 15 files changed, 165 insertions(+), 380 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f72c8cf99a7a..ebde4e29ce0a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3063,7 +3063,6 @@ name = "wasi-http-tests" version = "0.0.0" dependencies = [ "anyhow", - "tokio", "wit-bindgen", ] diff --git a/crates/test-programs/src/http_server.rs b/crates/test-programs/src/http_server.rs index ff4c0d9cd475..8ad0bea6c1c2 100644 --- a/crates/test-programs/src/http_server.rs +++ b/crates/test-programs/src/http_server.rs @@ -1,15 +1,12 @@ -use anyhow::Context; +use anyhow::{Context, Result}; use http_body_util::{combinators::BoxBody, BodyExt, Full}; use hyper::{body::Bytes, service::service_fn, Request, Response}; use std::{ future::Future, - net::{SocketAddr, TcpListener}, - sync::{mpsc, OnceLock}, - time::Duration, + net::{SocketAddr, TcpListener, TcpStream}, + thread::JoinHandle, }; -const DEFAULT_TIMEOUT: Duration = Duration::from_secs(50); - async fn test( mut req: Request, ) -> http::Result>> { @@ -26,66 +23,93 @@ async fn test( .body(Full::::from(buf).boxed()) } -struct ServerHttp1 { - receiver: mpsc::Receiver>, +pub struct Server { + addr: SocketAddr, + worker: Option>>, } -impl ServerHttp1 { - fn new() -> Self { - tracing::debug!("initializing http1 server"); - static CELL_HTTP1: OnceLock = OnceLock::new(); - let listener = CELL_HTTP1.get_or_init(|| { - let addr = SocketAddr::from(([127, 0, 0, 1], 3000)); - tracing::debug!("preparing tcp listener at localhost:3000"); - TcpListener::bind(addr).unwrap() - }); - let (sender, receiver) = mpsc::channel::>(); - std::thread::spawn(move || { +impl Server { + fn new(run: impl FnOnce(tokio::net::TcpStream) -> F + Send + Sync + 'static) -> Result + where + F: Future>, + { + let addr = SocketAddr::from(([127, 0, 0, 1], 0)); + let listener = TcpListener::bind(addr).context("failed to bind")?; + let addr = listener.local_addr().context("failed to get local addr")?; + + let rt = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .context("failed to start tokio runtime")?; + let worker = std::thread::spawn(move || { tracing::debug!("dedicated thread to start listening"); - match tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - { - Ok(rt) => { - tracing::debug!("using tokio runtime"); - sender - .send(rt.block_on(async move { - tracing::debug!("preparing to accept connection"); - let (stream, _) = listener.accept().map_err(anyhow::Error::from)?; - tracing::trace!("tcp stream {:?}", stream); + rt.block_on(async move { + tracing::debug!("preparing to accept connection"); + let (stream, _) = listener.accept().map_err(anyhow::Error::from)?; + let io = tokio::net::TcpStream::from_std(stream).map_err(anyhow::Error::from)?; + run(io).await + }) + }); + Ok(Self { + worker: Some(worker), + addr, + }) + } - let mut builder = hyper::server::conn::http1::Builder::new(); - let http = builder.keep_alive(false).pipeline_flush(true); - let io = tokio::net::TcpStream::from_std(stream) - .map_err(anyhow::Error::from)?; + pub fn http1() -> Result { + tracing::debug!("initializing http1 server"); + Self::new(|io| async move { + let mut builder = hyper::server::conn::http1::Builder::new(); + let http = builder.keep_alive(false).pipeline_flush(true); + + tracing::debug!("preparing to bind connection to service"); + let conn = http.serve_connection(io, service_fn(test)).await; + tracing::trace!("connection result {:?}", conn); + conn?; + Ok(()) + }) + } - tracing::debug!("preparing to bind connection to service"); - let conn = http.serve_connection(io, service_fn(test)).await; - tracing::trace!("connection result {:?}", conn); - conn.map_err(anyhow::Error::from) - })) - .expect("value sent from http1 server dedicated thread"); - } - Err(e) => { - tracing::debug!("unable to start tokio runtime"); - sender.send(Err(anyhow::Error::from(e))).unwrap() + pub fn http2() -> Result { + tracing::debug!("initializing http2 server"); + Self::new(|io| async move { + let mut builder = hyper::server::conn::http2::Builder::new(TokioExecutor); + let http = builder.max_concurrent_streams(20); + + tracing::debug!("preparing to bind connection to service"); + let conn = http.serve_connection(io, service_fn(test)).await; + tracing::trace!("connection result {:?}", conn); + if let Err(e) = &conn { + let message = e.to_string(); + if message.contains("connection closed before reading preface") + || message.contains("unspecific protocol error detected") + { + return Ok(()); } - }; - }); - Self { receiver } + } + conn?; + Ok(()) + }) } - fn shutdown(self) -> anyhow::Result<()> { + pub fn addr(&self) -> &SocketAddr { + &self.addr + } +} + +impl Drop for Server { + fn drop(&mut self) { tracing::debug!("shutting down http1 server"); - self.receiver - .recv_timeout(DEFAULT_TIMEOUT) - .context("value received from http1 server dedicated thread")? + // Force a connection to happen in case one hasn't happened already. + let _ = TcpStream::connect(&self.addr); + let worker = self.worker.take().unwrap(); + worker.join().unwrap().unwrap(); } } #[derive(Clone)] /// An Executor that uses the tokio runtime. -pub struct TokioExecutor; +struct TokioExecutor; impl hyper::rt::Executor for TokioExecutor where @@ -96,139 +120,3 @@ where tokio::task::spawn(fut); } } - -struct ServerHttp2 { - receiver: mpsc::Receiver>, -} - -impl ServerHttp2 { - fn new() -> Self { - tracing::debug!("initializing http2 server"); - static CELL_HTTP2: OnceLock = OnceLock::new(); - let listener = CELL_HTTP2.get_or_init(|| { - let addr = SocketAddr::from(([127, 0, 0, 1], 3001)); - tracing::debug!("preparing tcp listener at localhost:3001"); - TcpListener::bind(addr).unwrap() - }); - let (sender, receiver) = mpsc::channel::>(); - std::thread::spawn(move || { - tracing::debug!("dedicated thread to start listening"); - match tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - { - Ok(rt) => { - tracing::debug!("using tokio runtime"); - sender - .send(rt.block_on(async move { - tracing::debug!("preparing to accept incoming connection"); - let (stream, _) = listener.accept().map_err(anyhow::Error::from)?; - tracing::trace!("tcp stream {:?}", stream); - - let mut builder = - hyper::server::conn::http2::Builder::new(TokioExecutor); - let http = builder.max_concurrent_streams(20); - let io = tokio::net::TcpStream::from_std(stream) - .map_err(anyhow::Error::from)?; - - tracing::debug!("preparing to bind connection to service"); - let conn = http.serve_connection(io, service_fn(test)).await; - tracing::trace!("connection result {:?}", conn); - if let Err(e) = &conn { - let message = e.to_string(); - if message.contains("connection closed before reading preface") - || message.contains("unspecific protocol error detected") - { - return Ok(()); - } - } - conn.map_err(anyhow::Error::from) - })) - .expect("value sent from http2 server dedicated thread"); - } - Err(e) => { - tracing::debug!("unable to start tokio runtime"); - sender.send(Err(anyhow::Error::from(e))).unwrap() - } - }; - }); - Self { receiver } - } - - fn shutdown(self) -> anyhow::Result<()> { - tracing::debug!("shutting down http2 server"); - self.receiver - .recv_timeout(DEFAULT_TIMEOUT) - .context("value received from http2 server dedicated thread")? - } -} - -pub async fn setup_http1(f: impl Future>) -> anyhow::Result<()> { - tracing::debug!("preparing http1 server asynchronously"); - let server = ServerHttp1::new(); - - tracing::debug!("running inner function (future)"); - let result = f.await; - - if let Err(err) = server.shutdown() { - tracing::error!("[host/server] failure {:?}", err); - } - result -} - -pub fn setup_http1_sync(f: F) -> anyhow::Result<()> -where - F: FnOnce() -> anyhow::Result<()> + Send + 'static, -{ - tracing::debug!("preparing http1 server synchronously"); - let server = ServerHttp1::new(); - - let (tx, rx) = mpsc::channel::>(); - tracing::debug!("running inner function in a dedicated thread"); - std::thread::spawn(move || { - let _ = tx.send(f()); - }); - let result = rx - .recv_timeout(DEFAULT_TIMEOUT) - .context("value received from request dedicated thread"); - - if let Err(err) = server.shutdown() { - tracing::error!("[host/server] failure {:?}", err); - } - result? -} - -pub async fn setup_http2(f: impl Future>) -> anyhow::Result<()> { - tracing::debug!("preparing http2 server asynchronously"); - let server = ServerHttp2::new(); - - tracing::debug!("running inner function (future)"); - let result = f.await; - - if let Err(err) = server.shutdown() { - tracing::error!("[host/server] Failure: {:?}", err); - } - result -} - -pub fn setup_http2_sync(f: F) -> anyhow::Result<()> -where - F: FnOnce() -> anyhow::Result<()> + Send + 'static, -{ - tracing::debug!("preparing http2 server synchronously"); - let server = ServerHttp2::new(); - - let (tx, rx) = mpsc::channel::>(); - tracing::debug!("running inner function in a dedicated thread"); - std::thread::spawn(move || { - let _ = tx.send(f()); - }); - let result = rx - .recv_timeout(DEFAULT_TIMEOUT) - .context("value received from request dedicated thread"); - - if let Err(err) = server.shutdown() { - tracing::error!("[host/server] failure {:?}", err); - } - result? -} diff --git a/crates/test-programs/tests/wasi-http-components-sync.rs b/crates/test-programs/tests/wasi-http-components-sync.rs index b27d1c2eec6d..1f5a196c361c 100644 --- a/crates/test-programs/tests/wasi-http-components-sync.rs +++ b/crates/test-programs/tests/wasi-http-components-sync.rs @@ -1,4 +1,7 @@ #![cfg(all(feature = "test_programs", not(skip_wasi_http_tests)))] + +use anyhow::Result; +use test_programs::http_server::Server; use wasmtime::{ component::{Component, Linker}, Config, Engine, Store, @@ -8,8 +11,6 @@ use wasmtime_wasi::preview2::{ }; use wasmtime_wasi_http::{WasiHttpCtx, WasiHttpView}; -use test_programs::http_server::{setup_http1_sync, setup_http2_sync}; - lazy_static::lazy_static! { static ref ENGINE: Engine = { let mut config = Config::new(); @@ -66,7 +67,7 @@ fn instantiate_component( Ok((store, command)) } -fn run(name: &str) -> anyhow::Result<()> { +fn run(name: &str, server: &Server) -> Result<()> { let stdout = MemoryOutputPipe::new(4096); let stderr = MemoryOutputPipe::new(4096); let r = { @@ -81,6 +82,7 @@ fn run(name: &str) -> anyhow::Result<()> { for (var, val) in test_programs::wasi_tests_environment() { builder.env(var, val); } + builder.env("HTTP_SERVER", server.addr().to_string()); let wasi = builder.build(); let http = WasiHttpCtx {}; @@ -109,70 +111,55 @@ fn run(name: &str) -> anyhow::Result<()> { } #[test_log::test] -#[cfg_attr( - windows, - ignore = "test is currently flaky in ci and needs to be debugged" -)] -fn outbound_request_get() { - setup_http1_sync(|| run("outbound_request_get")).unwrap(); +fn outbound_request_get() -> Result<()> { + let server = Server::http1()?; + run("outbound_request_get", &server) } #[test_log::test] -#[cfg_attr( - windows, - ignore = "test is currently flaky in ci and needs to be debugged" -)] -fn outbound_request_post() { - setup_http1_sync(|| run("outbound_request_post")).unwrap(); +fn outbound_request_post() -> Result<()> { + let server = Server::http1()?; + run("outbound_request_post", &server) } #[test_log::test] -#[cfg_attr( - windows, - ignore = "test is currently flaky in ci and needs to be debugged" -)] -fn outbound_request_large_post() { - setup_http1_sync(|| run("outbound_request_large_post")).unwrap(); +fn outbound_request_large_post() -> Result<()> { + let server = Server::http1()?; + run("outbound_request_large_post", &server) } #[test_log::test] -#[cfg_attr( - windows, - ignore = "test is currently flaky in ci and needs to be debugged" -)] -fn outbound_request_put() { - setup_http1_sync(|| run("outbound_request_put")).unwrap(); +fn outbound_request_put() -> Result<()> { + let server = Server::http1()?; + run("outbound_request_put", &server) } #[test_log::test] -#[cfg_attr( - windows, - ignore = "test is currently flaky in ci and needs to be debugged" -)] -fn outbound_request_invalid_version() { - setup_http2_sync(|| run("outbound_request_invalid_version")).unwrap(); +fn outbound_request_invalid_version() -> Result<()> { + let server = Server::http2()?; + run("outbound_request_invalid_version", &server) } #[test_log::test] -fn outbound_request_unknown_method() { - run("outbound_request_unknown_method").unwrap(); +fn outbound_request_unknown_method() -> Result<()> { + let server = Server::http1()?; + run("outbound_request_unknown_method", &server) } #[test_log::test] -fn outbound_request_unsupported_scheme() { - run("outbound_request_unsupported_scheme").unwrap(); +fn outbound_request_unsupported_scheme() -> Result<()> { + let server = Server::http1()?; + run("outbound_request_unsupported_scheme", &server) } #[test_log::test] -fn outbound_request_invalid_port() { - run("outbound_request_invalid_port").unwrap(); +fn outbound_request_invalid_port() -> Result<()> { + let server = Server::http1()?; + run("outbound_request_invalid_port", &server) } #[test_log::test] -#[cfg_attr( - windows, - ignore = "test is currently flaky in ci and needs to be debugged" -)] -fn outbound_request_invalid_dnsname() { - run("outbound_request_invalid_dnsname").unwrap(); +fn outbound_request_invalid_dnsname() -> Result<()> { + let server = Server::http1()?; + run("outbound_request_invalid_dnsname", &server) } diff --git a/crates/test-programs/tests/wasi-http-components.rs b/crates/test-programs/tests/wasi-http-components.rs index 51f194800854..2756407751f3 100644 --- a/crates/test-programs/tests/wasi-http-components.rs +++ b/crates/test-programs/tests/wasi-http-components.rs @@ -1,4 +1,7 @@ #![cfg(all(feature = "test_programs", not(skip_wasi_http_tests)))] + +use anyhow::Result; +use test_programs::http_server::Server; use wasmtime::{ component::{Component, Linker}, Config, Engine, Store, @@ -8,8 +11,6 @@ use wasmtime_wasi::preview2::{ }; use wasmtime_wasi_http::{WasiHttpCtx, WasiHttpView}; -use test_programs::http_server::{setup_http1, setup_http2}; - lazy_static::lazy_static! { static ref ENGINE: Engine = { let mut config = Config::new(); @@ -66,7 +67,7 @@ async fn instantiate_component( Ok((store, command)) } -async fn run(name: &str) -> anyhow::Result<()> { +async fn run(name: &str, server: &Server) -> Result<()> { let stdout = MemoryOutputPipe::new(4096); let stderr = MemoryOutputPipe::new(4096); let r = { @@ -81,6 +82,7 @@ async fn run(name: &str) -> anyhow::Result<()> { for (var, val) in test_programs::wasi_tests_environment() { builder.env(var, val); } + builder.env("HTTP_SERVER", &server.addr().to_string()); let wasi = builder.build(); let http = WasiHttpCtx; @@ -111,70 +113,55 @@ async fn run(name: &str) -> anyhow::Result<()> { windows, ignore = "test is currently flaky in ci and needs to be debugged" )] -async fn outbound_request_get() { - setup_http1(run("outbound_request_get")).await.unwrap(); +async fn outbound_request_get() -> Result<()> { + let server = Server::http1()?; + run("outbound_request_get", &server).await } #[test_log::test(tokio::test(flavor = "multi_thread"))] -#[cfg_attr( - windows, - ignore = "test is currently flaky in ci and needs to be debugged" -)] -async fn outbound_request_post() { - setup_http1(run("outbound_request_post")).await.unwrap(); +async fn outbound_request_post() -> Result<()> { + let server = Server::http1()?; + run("outbound_request_post", &server).await } #[test_log::test(tokio::test(flavor = "multi_thread"))] -#[cfg_attr( - windows, - ignore = "test is currently flaky in ci and needs to be debugged" -)] -async fn outbound_request_large_post() { - setup_http1(run("outbound_request_large_post")) - .await - .unwrap(); +async fn outbound_request_large_post() -> Result<()> { + let server = Server::http1()?; + run("outbound_request_large_post", &server).await } #[test_log::test(tokio::test(flavor = "multi_thread"))] -#[cfg_attr( - windows, - ignore = "test is currently flaky in ci and needs to be debugged" -)] -async fn outbound_request_put() { - setup_http1(run("outbound_request_put")).await.unwrap(); +async fn outbound_request_put() -> Result<()> { + let server = Server::http1()?; + run("outbound_request_put", &server).await } #[test_log::test(tokio::test(flavor = "multi_thread"))] -#[cfg_attr( - windows, - ignore = "test is currently flaky in ci and needs to be debugged" -)] -async fn outbound_request_invalid_version() { - setup_http2(run("outbound_request_invalid_version")) - .await - .unwrap(); +async fn outbound_request_invalid_version() -> Result<()> { + let server = Server::http2()?; + run("outbound_request_invalid_version", &server).await } #[test_log::test(tokio::test(flavor = "multi_thread"))] -async fn outbound_request_unknown_method() { - run("outbound_request_unknown_method").await.unwrap(); +async fn outbound_request_unknown_method() -> Result<()> { + let server = Server::http1()?; + run("outbound_request_unknown_method", &server).await } #[test_log::test(tokio::test(flavor = "multi_thread"))] -async fn outbound_request_unsupported_scheme() { - run("outbound_request_unsupported_scheme").await.unwrap(); +async fn outbound_request_unsupported_scheme() -> Result<()> { + let server = Server::http1()?; + run("outbound_request_unsupported_scheme", &server).await } #[test_log::test(tokio::test(flavor = "multi_thread"))] -async fn outbound_request_invalid_port() { - run("outbound_request_invalid_port").await.unwrap(); +async fn outbound_request_invalid_port() -> Result<()> { + let server = Server::http1()?; + run("outbound_request_invalid_port", &server).await } #[test_log::test(tokio::test(flavor = "multi_thread"))] -#[cfg_attr( - windows, - ignore = "test is currently flaky in ci and needs to be debugged" -)] -async fn outbound_request_invalid_dnsname() { - run("outbound_request_invalid_dnsname").await.unwrap(); +async fn outbound_request_invalid_dnsname() -> Result<()> { + let server = Server::http1()?; + run("outbound_request_invalid_dnsname", &server).await } diff --git a/crates/test-programs/wasi-http-tests/Cargo.toml b/crates/test-programs/wasi-http-tests/Cargo.toml index a648458bedd2..14206c3bbaa4 100644 --- a/crates/test-programs/wasi-http-tests/Cargo.toml +++ b/crates/test-programs/wasi-http-tests/Cargo.toml @@ -7,7 +7,6 @@ publish = false [dependencies] anyhow = { workspace = true } -tokio = { workspace = true, features = ["macros", "rt"] } wit-bindgen = { workspace = true, default-features = false, features = [ "macros", ] } diff --git a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_get.rs b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_get.rs index 0172ebba0c62..25d71c662ca7 100644 --- a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_get.rs +++ b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_get.rs @@ -2,30 +2,26 @@ use anyhow::Context; use wasi_http_tests::bindings::wasi::http::types::{Method, Scheme}; fn main() { - wasi_http_tests::in_tokio(async { run().await }) -} - -async fn run() { + let addr = std::env::var("HTTP_SERVER").unwrap(); let res = wasi_http_tests::request( Method::Get, Scheme::Http, - "localhost:3000", + &addr, "/get?some=arg&goes=here", None, None, ) - .await - .context("localhost:3000 /get") + .context("/get") .unwrap(); - println!("localhost:3000 /get: {res:?}"); + println!("{addr} /get: {res:?}"); assert_eq!(res.status, 200); let method = res.header("x-wasmtime-test-method").unwrap(); assert_eq!(std::str::from_utf8(method).unwrap(), "GET"); let uri = res.header("x-wasmtime-test-uri").unwrap(); assert_eq!( std::str::from_utf8(uri).unwrap(), - "http://localhost:3000/get?some=arg&goes=here" + format!("http://{addr}/get?some=arg&goes=here") ); assert_eq!(res.body, b""); } diff --git a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_dnsname.rs b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_dnsname.rs index 6cc5ce8bc97b..fff855598f8a 100644 --- a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_dnsname.rs +++ b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_dnsname.rs @@ -1,10 +1,6 @@ use wasi_http_tests::bindings::wasi::http::types::{Method, Scheme}; fn main() { - wasi_http_tests::in_tokio(async { run().await }) -} - -async fn run() { let res = wasi_http_tests::request( Method::Get, Scheme::Http, @@ -12,8 +8,7 @@ async fn run() { "/", None, None, - ) - .await; + ); let error = res.unwrap_err().to_string(); assert!(error.starts_with("Error::InvalidUrl(\"failed to lookup address information:")); diff --git a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_port.rs b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_port.rs index 88750fc3712b..fe0290a30d21 100644 --- a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_port.rs +++ b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_port.rs @@ -1,10 +1,6 @@ use wasi_http_tests::bindings::wasi::http::types::{Method, Scheme}; fn main() { - wasi_http_tests::in_tokio(async { run().await }) -} - -async fn run() { let res = wasi_http_tests::request( Method::Get, Scheme::Http, @@ -12,8 +8,7 @@ async fn run() { "/", None, None, - ) - .await; + ); let error = res.unwrap_err(); assert_eq!( diff --git a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_version.rs b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_version.rs index 53c767edec8f..994f9d687e32 100644 --- a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_version.rs +++ b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_version.rs @@ -1,19 +1,8 @@ use wasi_http_tests::bindings::wasi::http::types::{Method, Scheme}; fn main() { - wasi_http_tests::in_tokio(async { run().await }) -} - -async fn run() { - let res = wasi_http_tests::request( - Method::Connect, - Scheme::Http, - "localhost:3001", - "/", - None, - Some(&[]), - ) - .await; + let addr = std::env::var("HTTP_SERVER").unwrap(); + let res = wasi_http_tests::request(Method::Connect, Scheme::Http, &addr, "/", None, Some(&[])); let error = res.unwrap_err().to_string(); if error.ne("Error::ProtocolError(\"invalid HTTP version parsed\")") diff --git a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_large_post.rs b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_large_post.rs index 80e0688d47fc..e5620910fd17 100644 --- a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_large_post.rs +++ b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_large_post.rs @@ -3,27 +3,23 @@ use std::io::{self, Read}; use wasi_http_tests::bindings::wasi::http::types::{Method, Scheme}; fn main() { - wasi_http_tests::in_tokio(async { run().await }) -} - -async fn run() { // TODO: ensure more than 700 bytes is allowed without error const LEN: usize = 700; let mut buffer = [0; LEN]; + let addr = std::env::var("HTTP_SERVER").unwrap(); io::repeat(0b001).read_exact(&mut buffer).unwrap(); let res = wasi_http_tests::request( Method::Post, Scheme::Http, - "localhost:3000", + &addr, "/post", Some(&buffer), None, ) - .await - .context("localhost:3000 /post large") + .context("/post large") .unwrap(); - println!("localhost:3000 /post large: {}", res.status); + println!("/post large: {}", res.status); assert_eq!(res.status, 200); let method = res.header("x-wasmtime-test-method").unwrap(); assert_eq!(std::str::from_utf8(method).unwrap(), "POST"); diff --git a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_post.rs b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_post.rs index 131356fa91bc..6fa0e9ad02df 100644 --- a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_post.rs +++ b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_post.rs @@ -2,23 +2,19 @@ use anyhow::Context; use wasi_http_tests::bindings::wasi::http::types::{Method, Scheme}; fn main() { - wasi_http_tests::in_tokio(async { run().await }) -} - -async fn run() { + let addr = std::env::var("HTTP_SERVER").unwrap(); let res = wasi_http_tests::request( Method::Post, Scheme::Http, - "localhost:3000", + &addr, "/post", Some(b"{\"foo\": \"bar\"}"), None, ) - .await - .context("localhost:3000 /post") + .context("/post") .unwrap(); - println!("localhost:3000 /post: {res:?}"); + println!("/post: {res:?}"); assert_eq!(res.status, 200); let method = res.header("x-wasmtime-test-method").unwrap(); assert_eq!(std::str::from_utf8(method).unwrap(), "POST"); diff --git a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_put.rs b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_put.rs index 93bb7a053950..68e34f6cb578 100644 --- a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_put.rs +++ b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_put.rs @@ -2,23 +2,12 @@ use anyhow::Context; use wasi_http_tests::bindings::wasi::http::types::{Method, Scheme}; fn main() { - wasi_http_tests::in_tokio(async { run().await }) -} - -async fn run() { - let res = wasi_http_tests::request( - Method::Put, - Scheme::Http, - "localhost:3000", - "/put", - Some(&[]), - None, - ) - .await - .context("localhost:3000 /put") - .unwrap(); + let addr = std::env::var("HTTP_SERVER").unwrap(); + let res = wasi_http_tests::request(Method::Put, Scheme::Http, &addr, "/put", Some(&[]), None) + .context("/put") + .unwrap(); - println!("localhost:3000 /put: {res:?}"); + println!("/put: {res:?}"); assert_eq!(res.status, 200); let method = res.header("x-wasmtime-test-method").unwrap(); assert_eq!(std::str::from_utf8(method).unwrap(), "PUT"); diff --git a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_unknown_method.rs b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_unknown_method.rs index a2ab5e48dc02..cb7847e3b5c1 100644 --- a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_unknown_method.rs +++ b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_unknown_method.rs @@ -1,10 +1,6 @@ use wasi_http_tests::bindings::wasi::http::types::{Method, Scheme}; fn main() { - wasi_http_tests::in_tokio(async { run().await }) -} - -async fn run() { let res = wasi_http_tests::request( Method::Other("OTHER".to_owned()), Scheme::Http, @@ -12,8 +8,7 @@ async fn run() { "/", None, None, - ) - .await; + ); let error = res.unwrap_err(); assert_eq!( diff --git a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_unsupported_scheme.rs b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_unsupported_scheme.rs index 482550627e8d..b9d198a44381 100644 --- a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_unsupported_scheme.rs +++ b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_unsupported_scheme.rs @@ -1,10 +1,6 @@ use wasi_http_tests::bindings::wasi::http::types::{Method, Scheme}; fn main() { - wasi_http_tests::in_tokio(async { run().await }) -} - -async fn run() { let res = wasi_http_tests::request( Method::Get, Scheme::Other("WS".to_owned()), @@ -12,8 +8,7 @@ async fn run() { "/", None, None, - ) - .await; + ); let error = res.unwrap_err(); assert_eq!( diff --git a/crates/test-programs/wasi-http-tests/src/lib.rs b/crates/test-programs/wasi-http-tests/src/lib.rs index 90ad31b79daa..515ba0f8f648 100644 --- a/crates/test-programs/wasi-http-tests/src/lib.rs +++ b/crates/test-programs/wasi-http-tests/src/lib.rs @@ -42,7 +42,7 @@ impl Response { } } -pub async fn request( +pub fn request( method: http_types::Method, scheme: http_types::Scheme, authority: &str, @@ -170,24 +170,3 @@ pub async fn request( body, }) } - -static RUNTIME: OnceLock = OnceLock::new(); - -pub fn in_tokio(f: F) -> F::Output { - match tokio::runtime::Handle::try_current() { - Ok(h) => { - let _enter = h.enter(); - h.block_on(f) - } - Err(_) => { - let runtime = RUNTIME.get_or_init(|| { - tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .unwrap() - }); - let _enter = runtime.enter(); - runtime.block_on(f) - } - } -} From 5544509b452529986fc8b47ad39b8228ac677e2a Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 3 Oct 2023 12:10:35 -0700 Subject: [PATCH 2/7] Remove unused import --- crates/test-programs/wasi-http-tests/src/lib.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/test-programs/wasi-http-tests/src/lib.rs b/crates/test-programs/wasi-http-tests/src/lib.rs index 515ba0f8f648..94b103414340 100644 --- a/crates/test-programs/wasi-http-tests/src/lib.rs +++ b/crates/test-programs/wasi-http-tests/src/lib.rs @@ -8,12 +8,10 @@ pub mod bindings { } use anyhow::{anyhow, Result}; -use std::fmt; -use std::sync::OnceLock; - use bindings::wasi::http::{outgoing_handler, types as http_types}; use bindings::wasi::io::poll; use bindings::wasi::io::streams; +use std::fmt; pub struct Response { pub status: http_types::StatusCode, From 0fb051a2c4c2cb815fd59809cb277d29b65c9a3b Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 4 Oct 2023 07:34:52 -0700 Subject: [PATCH 3/7] Don't panic from worker thread --- crates/test-programs/src/http_server.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/test-programs/src/http_server.rs b/crates/test-programs/src/http_server.rs index 8ad0bea6c1c2..8422c1026036 100644 --- a/crates/test-programs/src/http_server.rs +++ b/crates/test-programs/src/http_server.rs @@ -102,8 +102,16 @@ impl Drop for Server { tracing::debug!("shutting down http1 server"); // Force a connection to happen in case one hasn't happened already. let _ = TcpStream::connect(&self.addr); + + // If the worker fails with an error, report it here but don't panic. + // Some tests don't make a connection so the error will be that the tcp + // stream created above is closed immediately afterwards. Let the test + // independently decide if it failed or not, and this should be in the + // logs to assist with debugging if necessary. let worker = self.worker.take().unwrap(); - worker.join().unwrap().unwrap(); + if let Err(e) = worker.join().unwrap() { + eprintln!("worker failed with error {e:?}"); + } } } From f96008814413c27f60e91f1db494ac2104e6cd6f Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 4 Oct 2023 07:35:02 -0700 Subject: [PATCH 4/7] Improve failure error message prtest:full --- .../src/bin/outbound_request_invalid_dnsname.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_dnsname.rs b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_dnsname.rs index fff855598f8a..9fef3bfdca25 100644 --- a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_dnsname.rs +++ b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_dnsname.rs @@ -11,5 +11,8 @@ fn main() { ); let error = res.unwrap_err().to_string(); - assert!(error.starts_with("Error::InvalidUrl(\"failed to lookup address information:")); + assert!( + error.starts_with("Error::InvalidUrl(\"failed to lookup address information:"), + "bad error: {error}" + ); } From d0154cd13768f848f491026681d64f5f164d3eae Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 4 Oct 2023 09:09:25 -0700 Subject: [PATCH 5/7] Fix some windows-specific issues --- crates/test-programs/src/http_server.rs | 18 ++++++++++-------- .../tests/wasi-http-components.rs | 6 +----- .../bin/outbound_request_invalid_dnsname.rs | 2 +- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/crates/test-programs/src/http_server.rs b/crates/test-programs/src/http_server.rs index 8422c1026036..a3c7efb63c62 100644 --- a/crates/test-programs/src/http_server.rs +++ b/crates/test-programs/src/http_server.rs @@ -3,9 +3,10 @@ use http_body_util::{combinators::BoxBody, BodyExt, Full}; use hyper::{body::Bytes, service::service_fn, Request, Response}; use std::{ future::Future, - net::{SocketAddr, TcpListener, TcpStream}, + net::{SocketAddr, TcpStream}, thread::JoinHandle, }; +use tokio::net::TcpListener; async fn test( mut req: Request, @@ -34,20 +35,21 @@ impl Server { F: Future>, { let addr = SocketAddr::from(([127, 0, 0, 1], 0)); - let listener = TcpListener::bind(addr).context("failed to bind")?; - let addr = listener.local_addr().context("failed to get local addr")?; let rt = tokio::runtime::Builder::new_current_thread() .enable_all() .build() .context("failed to start tokio runtime")?; + + let listener = + rt.block_on(async move { TcpListener::bind(addr).await.context("failed to bind") })?; + let addr = listener.local_addr().context("failed to get local addr")?; let worker = std::thread::spawn(move || { tracing::debug!("dedicated thread to start listening"); rt.block_on(async move { tracing::debug!("preparing to accept connection"); - let (stream, _) = listener.accept().map_err(anyhow::Error::from)?; - let io = tokio::net::TcpStream::from_std(stream).map_err(anyhow::Error::from)?; - run(io).await + let (stream, _) = listener.accept().await.map_err(anyhow::Error::from)?; + run(stream).await }) }); Ok(Self { @@ -92,8 +94,8 @@ impl Server { }) } - pub fn addr(&self) -> &SocketAddr { - &self.addr + pub fn addr(&self) -> String { + format!("localhost:{}", self.addr.port()) } } diff --git a/crates/test-programs/tests/wasi-http-components.rs b/crates/test-programs/tests/wasi-http-components.rs index 2756407751f3..7d70cb365036 100644 --- a/crates/test-programs/tests/wasi-http-components.rs +++ b/crates/test-programs/tests/wasi-http-components.rs @@ -82,7 +82,7 @@ async fn run(name: &str, server: &Server) -> Result<()> { for (var, val) in test_programs::wasi_tests_environment() { builder.env(var, val); } - builder.env("HTTP_SERVER", &server.addr().to_string()); + builder.env("HTTP_SERVER", server.addr()); let wasi = builder.build(); let http = WasiHttpCtx; @@ -109,10 +109,6 @@ async fn run(name: &str, server: &Server) -> Result<()> { } #[test_log::test(tokio::test(flavor = "multi_thread"))] -#[cfg_attr( - windows, - ignore = "test is currently flaky in ci and needs to be debugged" -)] async fn outbound_request_get() -> Result<()> { let server = Server::http1()?; run("outbound_request_get", &server).await diff --git a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_dnsname.rs b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_dnsname.rs index 9fef3bfdca25..a36e5384336e 100644 --- a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_dnsname.rs +++ b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_dnsname.rs @@ -12,7 +12,7 @@ fn main() { let error = res.unwrap_err().to_string(); assert!( - error.starts_with("Error::InvalidUrl(\"failed to lookup address information:"), + error.starts_with("Error::InvalidUrl(\""), "bad error: {error}" ); } From 2ad9f7453c40fc16358f0f0da0c45ce34b3d23fb Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 4 Oct 2023 09:14:50 -0700 Subject: [PATCH 6/7] Fix async tests --- crates/test-programs/src/http_server.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/crates/test-programs/src/http_server.rs b/crates/test-programs/src/http_server.rs index a3c7efb63c62..d22fe5b201bc 100644 --- a/crates/test-programs/src/http_server.rs +++ b/crates/test-programs/src/http_server.rs @@ -34,15 +34,18 @@ impl Server { where F: Future>, { - let addr = SocketAddr::from(([127, 0, 0, 1], 0)); - - let rt = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .context("failed to start tokio runtime")?; - - let listener = - rt.block_on(async move { TcpListener::bind(addr).await.context("failed to bind") })?; + let thread = std::thread::spawn(|| -> Result<_> { + let rt = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .context("failed to start tokio runtime")?; + let listener = rt.block_on(async move { + let addr = SocketAddr::from(([127, 0, 0, 1], 0)); + TcpListener::bind(addr).await.context("failed to bind") + })?; + Ok((rt, listener)) + }); + let (rt, listener) = thread.join().unwrap()?; let addr = listener.local_addr().context("failed to get local addr")?; let worker = std::thread::spawn(move || { tracing::debug!("dedicated thread to start listening"); From 29c11f1b951094869e37d9f2c6e68abc9d569cd7 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 4 Oct 2023 10:16:47 -0700 Subject: [PATCH 7/7] Relax test assertion string --- .../src/bin/outbound_request_invalid_version.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_version.rs b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_version.rs index 994f9d687e32..c61bfeab1b32 100644 --- a/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_version.rs +++ b/crates/test-programs/wasi-http-tests/src/bin/outbound_request_invalid_version.rs @@ -5,9 +5,7 @@ fn main() { let res = wasi_http_tests::request(Method::Connect, Scheme::Http, &addr, "/", None, Some(&[])); let error = res.unwrap_err().to_string(); - if error.ne("Error::ProtocolError(\"invalid HTTP version parsed\")") - && !error.starts_with("Error::ProtocolError(\"operation was canceled") - { + if !error.starts_with("Error::ProtocolError(\"") { panic!( r#"assertion failed: `(left == right)` left: `"{error}"`,