From 8d063a49082f98a6e0bfaaa19b649c58a54b8468 Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Fri, 15 Sep 2023 12:50:01 +1000 Subject: [PATCH] Fix example tests (#3614) * Fix example tests I went to enable testing for the `synchronous-instantiation` example now that Firefox supports module workers, but then found that the `wasm-audio-worklet` example was failing because the test server doesn't set the headers needed to enable `SharedArrayBuffer`. It turns out that CI wasn't failing because it's been broken this whole whole time: it specifies the path to the built examples as simply `exbuild`, which doesn't work because the tests are run with their working directory set to `crates/example-tests`, not the root of the repo. This means that any requests that the examples try to make will 404. So this PR specifies it as an absolute path instead. At the moment, Firefox doesn't directly indicate any kind of error when navigation fails, which meant that the tests would just silently fail without actually testing anything. According to the spec, `browsingContext.navigate` is supposed to wait for the navigation to complete, and result in an error if something goes wrong; but I think Firefox is behind, because it seems to instead immediately return. To work around this, I've made it so that the tests manually wait for the `network.responseCompleted` event to check if fetching the page suceeded, and so this shouldn't happen again. I've left the actual fix for the `wasm-audio-worklet` example commented out to make sure that CI actually catches the issue now; that's why this PR is a draft. * properly interpolate repo root * use correct variable It looks like `env` is specifically for variables that you set manually, not arbitrary environment variables. * Fix wasm_audio_worklet * tweak doc comment --- .github/workflows/main.yml | 2 +- crates/example-tests/Cargo.toml | 11 +- crates/example-tests/src/lib.rs | 206 ++++++++++++++++++---------- crates/example-tests/tests/shell.rs | 1 - 4 files changed, 142 insertions(+), 78 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index eafcc99c43e..b4078fe6d75 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -323,7 +323,7 @@ jobs: - run: rustup update --no-self-update stable && rustup default stable - run: cargo test -p example-tests env: - EXBUILD: exbuild + EXBUILD: ${{ github.workspace }}/exbuild build_benchmarks: runs-on: ubuntu-latest diff --git a/crates/example-tests/Cargo.toml b/crates/example-tests/Cargo.toml index b7df800fb60..ff75a3b65f1 100644 --- a/crates/example-tests/Cargo.toml +++ b/crates/example-tests/Cargo.toml @@ -6,14 +6,15 @@ edition = "2018" rust-version = "1.56" [dependencies] -anyhow = "1.0.58" -futures-util = { version = "0.3.21", features = ["sink"] } -hyper = { version = "0.14.20", features = ["server", "tcp", "http1"] } +anyhow = "1.0.75" +futures-util = { version = "0.3.28", features = ["sink"] } +http = "0.2.9" +hyper = { version = "0.14.27", features = ["server", "tcp", "http1"] } mozprofile = "0.8.0" mozrunner = "0.14.0" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" -tokio = { version = "1.20.0", features = ["macros", "time"] } +tokio = { version = "1.29.1", features = ["macros", "time"] } tokio-tungstenite = "0.17.2" tower = { version = "0.4.13", features = ["make"] } -tower-http = { version = "0.3.4", features = ["fs"] } +tower-http = { version = "0.3.5", features = ["fs", "util", "set-header"] } diff --git a/crates/example-tests/src/lib.rs b/crates/example-tests/src/lib.rs index 9dbbf05a96c..b064858c0db 100644 --- a/crates/example-tests/src/lib.rs +++ b/crates/example-tests/src/lib.rs @@ -8,6 +8,7 @@ use std::{env, str}; use anyhow::{bail, Context}; use futures_util::{future, SinkExt, StreamExt}; +use http::{HeaderName, HeaderValue}; use mozprofile::profile::Profile; use mozrunner::firefox_default_path; use mozrunner::runner::{FirefoxProcess, FirefoxRunner, Runner, RunnerProcess}; @@ -20,7 +21,9 @@ use tokio::time::timeout; use tokio_tungstenite::tungstenite::{self, Message}; use tokio_tungstenite::{MaybeTlsStream, WebSocketStream}; use tower::make::Shared; +use tower::ServiceBuilder; use tower_http::services::ServeDir; +use tower_http::ServiceBuilderExt; /// A command sent from the client to the server. #[derive(Serialize)] @@ -241,6 +244,73 @@ impl WebDriver { } } +/// Handles a `log.entryAdded` event with the given parameters, and returns an +/// error if the log entry is an error (or something else goes wrong). +fn handle_log_event(params: Value) -> anyhow::Result<()> { + #[derive(Deserialize)] + #[serde(rename_all = "camelCase")] + struct LogEntry { + level: LogLevel, + text: Option, + stack_trace: Option, + } + + #[derive(Deserialize, Debug, PartialEq, Eq, Clone, Copy)] + #[serde(rename_all = "lowercase")] + enum LogLevel { + Debug, + Info, + Warn, + Error, + } + + #[derive(Deserialize)] + #[serde(rename_all = "camelCase")] + struct StackTrace { + call_frames: Vec, + } + + #[derive(Deserialize)] + #[serde(rename_all = "camelCase")] + struct StackFrame { + column_number: i64, + function_name: String, + line_number: i64, + url: String, + } + + impl Display for StackFrame { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + write!( + f, + "{} (at {}:{}:{})", + self.function_name, self.url, self.line_number, self.column_number + ) + } + } + + let entry: LogEntry = serde_json::from_value(params).context("invalid log entry received")?; + + if entry.level == LogLevel::Error { + if let Some(text) = entry.text { + let mut msg = format!("An error occurred: {text}"); + + if let Some(stack_trace) = entry.stack_trace { + write!(msg, "\n\nStack trace:").unwrap(); + for frame in stack_trace.call_frames { + write!(msg, "\n{frame}").unwrap(); + } + } + + bail!("{msg}") + } else { + bail!("An error occurred") + } + } + + Ok(()) +} + /// Run a single example with the passed name, using the passed closure to /// build it if prebuilt examples weren't provided. pub async fn test_example( @@ -256,8 +326,18 @@ pub async fn test_example( let mut driver = WebDriver::new().await?; // Serve the path. - let server = hyper::Server::try_bind(&"127.0.0.1:0".parse().unwrap())? - .serve(Shared::new(ServeDir::new(path))); + let service = ServiceBuilder::new() + .override_response_header( + HeaderName::from_static("cross-origin-opener-policy"), + HeaderValue::from_static("same-origin"), + ) + .override_response_header( + HeaderName::from_static("cross-origin-embedder-policy"), + HeaderValue::from_static("require-corp"), + ) + .service(ServeDir::new(path)); + let server = + hyper::Server::try_bind(&"127.0.0.1:0".parse().unwrap())?.serve(Shared::new(service)); let addr = server.local_addr(); @@ -281,13 +361,18 @@ pub async fn test_example( .issue_cmd( "session.subscribe", json!({ - "events": ["log.entryAdded"], + "events": ["log.entryAdded", "network.responseCompleted"], "contexts": [&context], }), ) .await?; - let _: Value = driver + #[derive(Deserialize)] + struct BrowsingContextNavigateResult { + navigation: Option, + } + + let BrowsingContextNavigateResult { navigation } = driver .issue_cmd( "browsingContext.navigate", json!({ @@ -296,6 +381,51 @@ pub async fn test_example( }), ) .await?; + // Apparently this being null means that 'the navigation [was] canceled before + // making progress'. + // source: https://w3c.github.io/webdriver-bidi/#module-browsingContext + let navigation = navigation.context("navigation canceled")?; + + // Wait for the page to be fetched, so that we can check whether it succeeds. + // Note: I'm pretty sure that `browsingContext.navigate` is supposed to report + // an error anyway if this fails, but Firefox seems to be behind the spec here. + loop { + let event = driver + .next_event() + .await + .context("websocket unexpectedly closed")?; + match event.method.as_str() { + "log.entryAdded" => handle_log_event(event.params)?, + "network.responseCompleted" => { + #[derive(Deserialize)] + struct NetworkReponseCompletedParameters { + navigation: Option, + response: NetworkResponseData, + } + + #[derive(Deserialize)] + #[serde(rename_all = "camelCase")] + struct NetworkResponseData { + status: u64, + status_text: String, + } + + let params: NetworkReponseCompletedParameters = + serde_json::from_value(event.params)?; + if params.navigation.as_ref() == Some(&navigation) { + if !(200..300).contains(¶ms.response.status) { + bail!( + "fetching page failed ({} {})", + params.response.status, + params.response.status_text + ) + } + break; + } + } + _ => {} + } + } let start = Instant::now(); // Wait 5 seconds for any errors to occur. @@ -305,73 +435,7 @@ pub async fn test_example( Ok(event) => { let event = event?; if event.method == "log.entryAdded" { - #[derive(Deserialize)] - #[serde(rename_all = "camelCase")] - struct LogEntry { - level: LogLevel, - // source: Source, - text: Option, - // timestamp: i64, - stack_trace: Option, - // kind: LogEntryKind, - } - - #[derive(Deserialize, Debug, PartialEq, Eq, Clone, Copy)] - #[serde(rename_all = "lowercase")] - enum LogLevel { - Debug, - Info, - Warning, - Error, - } - - #[derive(Deserialize)] - #[serde(rename_all = "camelCase")] - struct StackTrace { - call_frames: Vec, - } - - #[derive(Deserialize)] - #[serde(rename_all = "camelCase")] - struct StackFrame { - column_number: i64, - function_name: String, - line_number: i64, - url: String, - } - - impl Display for StackFrame { - fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - write!( - f, - "{} (at {}:{}:{})", - self.function_name, - self.url, - self.line_number, - self.column_number - ) - } - } - - let entry: LogEntry = serde_json::from_value(event.params) - .context("invalid log entry received")?; - - if entry.level == LogLevel::Error { - if let Some(text) = entry.text { - let mut msg = format!("An error occurred: {text}"); - - if let Some(stack_trace) = entry.stack_trace { - write!(msg, "\n\nStack trace:").unwrap(); - for frame in stack_trace.call_frames { - write!(msg, "\n{frame}").unwrap(); - } - } - - bail!("{msg}") - } else { - bail!("An error occurred") - } - } + handle_log_event(event.params)?; } } Err(_) => break, diff --git a/crates/example-tests/tests/shell.rs b/crates/example-tests/tests/shell.rs index bec7b8c7a39..19af0db4fec 100644 --- a/crates/example-tests/tests/shell.rs +++ b/crates/example-tests/tests/shell.rs @@ -36,7 +36,6 @@ macro_rules! shell_tests { shell_tests! { #["RUSTUP_TOOLCHAIN" = "nightly"] raytrace_parallel = "raytrace-parallel", - #[ignore = "This requires module workers, which Firefox doesn't support yet."] synchronous_instantiation = "synchronous-instantiation", wasm2js = "wasm2js", #["RUSTUP_TOOLCHAIN" = "nightly"]