From 8552b3aafcb5aeae8f12954af513456223b099f3 Mon Sep 17 00:00:00 2001 From: Austin Hicks Date: Fri, 8 Mar 2024 18:14:32 -0800 Subject: [PATCH] integration_tests: bootstrap all the way to being able to run a basic sine test --- Cargo.lock | 26 ++++- Cargo.toml | 1 + crates/integration_tests/Cargo.toml | 1 + crates/integration_tests/src/cli_args.rs | 10 ++ crates/integration_tests/src/commands/mod.rs | 2 + .../src/commands/view_response.rs | 48 ++++++++++ crates/integration_tests/src/context.rs | 95 +++++++++++++++++-- crates/integration_tests/src/environment.rs | 4 + .../src/process_coordination/protocol.rs | 2 +- crates/integration_tests/src/registry.rs | 3 + crates/integration_tests/src/reporter.rs | 21 ++-- crates/integration_tests/src/test_config.rs | 1 + crates/integration_tests/src/test_runner.rs | 10 +- .../integration_tests/src/tests/basic_sine.rs | 48 ++++++++++ crates/integration_tests/src/tests/mod.rs | 1 + .../src/validators/function.rs | 6 +- .../integration_tests/src/validators/mod.rs | 4 +- .../integration_tests/src/validators/range.rs | 4 +- 18 files changed, 260 insertions(+), 27 deletions(-) create mode 100644 crates/integration_tests/src/commands/view_response.rs create mode 100644 crates/integration_tests/src/tests/basic_sine.rs diff --git a/Cargo.lock b/Cargo.lock index 8cebe67..25cdfe8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -809,9 +809,9 @@ dependencies = [ [[package]] name = "indexmap" -version = "2.0.2" +version = "2.2.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8adf3ddd720272c6ea8bf59463c04e0f93d0bbf7c5439b691bca2987e0270897" +checksum = "7b0b929d511467233429c45a44ac1dcaa21ba0f5ba11e4879e6ed28ddb4f9df4" dependencies = [ "equivalent", "hashbrown 0.14.2", @@ -1088,7 +1088,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e1d3afd2628e69da2be385eb6f2fd57c8ac7977ceeff6dc166ff1657b0e386a9" dependencies = [ "fixedbitset", - "indexmap 2.0.2", + "indexmap 2.2.5", ] [[package]] @@ -1590,6 +1590,19 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_yaml" +version = "0.9.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8fd075d994154d4a774f95b51fb96bdc2832b0ea48425c92546073816cda1f2f" +dependencies = [ + "indexmap 2.2.5", + "itoa", + "ryu", + "serde", + "unsafe-libyaml", +] + [[package]] name = "sharded-slab" version = "0.1.7" @@ -1914,6 +1927,7 @@ dependencies = [ "regex", "serde", "serde_json", + "serde_yaml", "synthizer", ] @@ -2123,6 +2137,12 @@ version = "1.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1dd624098567895118886609431a7c3b8f516e41d30e0643f03d94592a147e36" +[[package]] +name = "unsafe-libyaml" +version = "0.2.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ab4c90930b95a82d00dc9e9ac071b4991924390d46cbd0dfe566148667605e4b" + [[package]] name = "utf8parse" version = "0.2.1" diff --git a/Cargo.toml b/Cargo.toml index ea060f0..c6d035b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -53,6 +53,7 @@ regex = "1.10.3" rubato = "0.14.1" serde = { version = "1.0.197", features = ["derive"] } serde_json = "1.0.114" +serde_yaml = "0.9.32" sharded-slab = "0.1.4" smallvec = { version = "1.10.0", features = ["write"] } spin = "0.9.8" diff --git a/crates/integration_tests/Cargo.toml b/crates/integration_tests/Cargo.toml index 0cd1772..22ab5fe 100644 --- a/crates/integration_tests/Cargo.toml +++ b/crates/integration_tests/Cargo.toml @@ -22,4 +22,5 @@ paste.workspace = true regex.workspace = true serde.workspace = true serde_json.workspace = true +serde_yaml.workspace = true synthizer.workspace = true diff --git a/crates/integration_tests/src/cli_args.rs b/crates/integration_tests/src/cli_args.rs index 6030ad0..8025919 100644 --- a/crates/integration_tests/src/cli_args.rs +++ b/crates/integration_tests/src/cli_args.rs @@ -17,6 +17,11 @@ pub enum Command { /// List all tests. List(ListArgs), + /// Given the name of a test, pretty print the response.json or response-panic.json + /// + /// Figures out whichever is appropriate, and then shows that. + ViewResponse(ViewResponseArgs), + /// Private command used as an entrypoint to subprocesses. SubprocessEntryPoint(SubprocessArgs), } @@ -47,3 +52,8 @@ pub struct ListArgs { #[command(flatten)] pub filter: FilterArgs, } + +#[derive(Debug, Parser)] +pub struct ViewResponseArgs { + pub test_name: String, +} diff --git a/crates/integration_tests/src/commands/mod.rs b/crates/integration_tests/src/commands/mod.rs index 21b6a49..8b31122 100644 --- a/crates/integration_tests/src/commands/mod.rs +++ b/crates/integration_tests/src/commands/mod.rs @@ -1,6 +1,7 @@ mod list; mod run; mod subprocess_entry_point; +mod view_response; use crate::cli_args; @@ -8,6 +9,7 @@ use crate::cli_args; pub fn dispatch_command(args: cli_args::CliArgs) { match &args.command { cli_args::Command::List(l) => list::list(&args, l), + cli_args::Command::ViewResponse(r) => view_response::view_response(&args, r), cli_args::Command::Run(r) => run::run(&args, r), cli_args::Command::SubprocessEntryPoint(sp) => { subprocess_entry_point::subprocess_entry_point(&args, sp) diff --git a/crates/integration_tests/src/commands/view_response.rs b/crates/integration_tests/src/commands/view_response.rs new file mode 100644 index 0000000..7cc381c --- /dev/null +++ b/crates/integration_tests/src/commands/view_response.rs @@ -0,0 +1,48 @@ +use crate::cli_args::*; + +pub fn view_response(_top_args: &CliArgs, cmd_args: &ViewResponseArgs) { + let env = crate::environment::get_env(); + + if crate::registry::get_test_by_name(&cmd_args.test_name).is_none() { + panic!("{} is not a valid, registered test", cmd_args.test_name); + }; + + let artifacts_dir = env.artifacts_dir_for(&cmd_args.test_name); + if !artifacts_dir.exists() { + panic!( + "{}: no artifacts directory found. Tried {}. This probably means the test passed.", + cmd_args.test_name, + artifacts_dir.display() + ); + } + + let possibilities = [ + env.panic_response_file_for(&cmd_args.test_name), + env.good_response_file_for(&cmd_args.test_name), + ]; + + let mut response: Option = None; + for p in possibilities { + match std::fs::read(&p) { + Ok(r) => { + response = Some( + String::from_utf8(r).expect("This is a JSON file and should always be UTF-8"), + ); + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => continue, + Err(e) => { + panic!("While trying to open path {}: {}", p.display(), e); + } + } + } + + let response = response.expect( + "Unable to find a response file in the artifacts directory. Manual examination is required", + ); + + // Converting to yaml gets us nice pretty printing of multiline strings. + let json: serde_json::Value = serde_json::from_str(&response).unwrap(); + let yaml = serde_yaml::to_string(&json).unwrap(); + println!("Response for {}", cmd_args.test_name); + println!("{yaml}"); +} diff --git a/crates/integration_tests/src/context.rs b/crates/integration_tests/src/context.rs index bc09a2a..7304ac7 100644 --- a/crates/integration_tests/src/context.rs +++ b/crates/integration_tests/src/context.rs @@ -1,4 +1,5 @@ use anyhow::Result; +use std::time::Duration; use synthizer as syz; @@ -8,15 +9,56 @@ use crate::validators::Validator; pub struct TestContext { pub test_name: String, pub channel_format: syz::ChannelFormat, + pub server: syz::Server, pub validators: Vec>, + + /// Last chunk of synthesized audio, if any. + /// + /// On failure to advance, set to the empty vec. + pub synthesized_audio: Vec, +} + +/// Trait representing various units of time. +pub trait Advanceable { + fn get_frame_count(&self) -> usize; +} + +/// Advance by some number of frames. +pub struct Frames(pub usize); + +/// Advance by some number of blocks. +pub struct Blocks(pub usize); + +impl Advanceable for Frames { + fn get_frame_count(&self) -> usize { + self.0 + } +} + +impl Advanceable for Blocks { + fn get_frame_count(&self) -> usize { + self.0.checked_mul(syz::BLOCK_SIZE).expect( + "Attempt to advance by more than the number of frames that could ever fit into memory", + ) + } } +impl Advanceable for Duration { + fn get_frame_count(&self) -> usize { + let secs = self.as_secs_f64(); + let frames = secs * (syz::SR as f64); + (frames.ceil() as u64).try_into().expect("Unable to advance by this many seconds because that would require more frames than can fit into the memory of this machine") + } +} impl TestContext { pub fn from_config(test_name: &str, config: TestConfig) -> Result { let mut ret = Self { channel_format: syz::ChannelFormat::Stereo, + server: syz::Server::new_inline() + .expect("Must be able to create a Synthizer server for test startup"), test_name: test_name.to_string(), validators: vec![], + synthesized_audio: vec![0.0; syz::BLOCK_SIZE * 2], }; let validators = config @@ -29,14 +71,55 @@ impl TestContext { Ok(ret) } + /// Run the specified closure over all validators in such a way as to allow it to itself get a context. + /// + /// Deals with the limitation of Rust that we cannot do field splitrting over the whole call grapha by taking the + /// validators out of the context, then putting them back. + fn validators_foreach(&mut self, mut callback: impl FnMut(&TestContext, &mut dyn Validator)) { + let mut vals = std::mem::take(&mut self.validators); + for v in vals.iter_mut() { + callback(self, &mut **v); + } + self.validators = vals; + } + /// Get the outcomes of all validators. pub fn finalize_validators(&mut self) -> Vec> { - // We need to be able to let the validators see the context, so take the list of validators out, then put it - // back. - let mut validators = std::mem::take(&mut self.validators); - - let ret = validators.iter_mut().map(|x| x.finalize(self)).collect(); - self.validators = validators; + let mut ret = vec![]; + self.validators_foreach(|ctx, v| { + ret.push(v.finalize(ctx)); + }); ret } + + #[track_caller] + fn advance_by_frames(&mut self, frame_count: usize) -> Result<()> { + let mut synthesized_audio = std::mem::take(&mut self.synthesized_audio); + synthesized_audio.resize( + frame_count * self.channel_format.get_channel_count().get(), + 0.0, + ); + // We want to test that Synthizer zeros this buffer, so we can fill it with NaN and then tests freak out if this + // is not the case. + synthesized_audio.fill(f32::NAN); + + self.server.synthesize_stereo(&mut synthesized_audio[..])?; + + // Must be here, otherwise track_caller doesn't work through the closure. + let loc = std::panic::Location::caller(); + self.validators_foreach(|ctx, v| { + v.validate_batched(ctx, loc, &synthesized_audio[..]); + }); + Ok(()) + } + + /// Advance this simulation. Time may be: + /// + /// - A [Duration]. This is converted to samples and rounded up to the next sample. + /// - [Frames]: E.g. `Frames(2)`. Advance by the specified number of frames. + /// - [Blocks]: e.g. `Blocks(5)`. Advance by the specified number of blocks. + #[track_caller] + pub fn advance(&mut self, time: T) -> Result<()> { + self.advance_by_frames(time.get_frame_count()) + } } diff --git a/crates/integration_tests/src/environment.rs b/crates/integration_tests/src/environment.rs index 976ccb7..f8e2459 100644 --- a/crates/integration_tests/src/environment.rs +++ b/crates/integration_tests/src/environment.rs @@ -60,4 +60,8 @@ impl> Environment { pub fn panic_response_file_for(&self, test_name: &str) -> PathBuf { self.artifacts_dir_for(test_name).join(RESPONSE_PANIC_FILE) } + + pub fn good_response_file_for(&self, test_name: &str) -> PathBuf { + self.artifacts_dir_for(test_name).join(RESPONSE_GOOD_FILE) + } } diff --git a/crates/integration_tests/src/process_coordination/protocol.rs b/crates/integration_tests/src/process_coordination/protocol.rs index 739ea2c..48cd8b7 100644 --- a/crates/integration_tests/src/process_coordination/protocol.rs +++ b/crates/integration_tests/src/process_coordination/protocol.rs @@ -32,7 +32,7 @@ pub struct SubprocessResponse { } /// The outcome of a test. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize, derive_more::IsVariant)] pub enum TestOutcome { /// Test passed this time. If there are further runs, cancel them. Passed, diff --git a/crates/integration_tests/src/registry.rs b/crates/integration_tests/src/registry.rs index a06cda5..b25dea2 100644 --- a/crates/integration_tests/src/registry.rs +++ b/crates/integration_tests/src/registry.rs @@ -65,6 +65,9 @@ fn build_test_cache() -> Vec<&'static TestRegistryEntry> { ret } +pub fn get_test_by_name(test_name: &str) -> Option<&'static TestRegistryEntry> { + get_tests().find(|x| x.name() == test_name) +} impl TestRegistryEntry { pub fn name(&self) -> &str { self.name diff --git a/crates/integration_tests/src/reporter.rs b/crates/integration_tests/src/reporter.rs index 8f227d8..4c795ec 100644 --- a/crates/integration_tests/src/reporter.rs +++ b/crates/integration_tests/src/reporter.rs @@ -43,23 +43,30 @@ fn report_test_fallible( write!(dest, "{test_name} ")?; match outcome { - proto::TestOutcome::Passed => write!(dest, "passed"), + proto::TestOutcome::Passed => write!(dest, "passed")?, proto::TestOutcome::Panicked(p) => { writeln!(dest, "panicked")?; - report_panic(&mut indented(&mut dest).ind(2), test_name, p) + report_panic(&mut indented(&mut dest).with_str(" "), test_name, p)?; } proto::TestOutcome::RunnerFailed(r) => { writeln!(dest, "Runner Failed")?; - report_runner_failed(&mut indented(&mut dest).ind(2), r) + report_runner_failed(&mut indented(&mut dest).with_str(" "), r)?; } proto::TestOutcome::ValidatorsFailed(v) => { writeln!(dest, "Validators failed")?; - report_validators_failed(&mut indented(&mut dest).ind(2), test_config, v) + report_validators_failed(&mut indented(&mut dest).with_str(" "), test_config, v)?; } proto::TestOutcome::Indeterminate => { - write!(dest, "Ended with an indeterminate result") + writeln!(dest, "Ended with an indeterminate result")?; } } + + if !outcome.is_passed() { + // Mind the double spaces at the beginning of this string. + writeln!(dest, " More information may be available. Try cargo run --bin synthizer_integration_tests -- view-response {test_name}")?; + } + + Ok(()) } fn report_panic(dest: &mut dyn Write, test_name: &str, info: &proto::PanicOutcome) -> Result { @@ -86,8 +93,8 @@ fn report_validators_failed( for v in info.entries.iter() { let mut ind_fmt = indented(&mut dest); let tag = test_config.validators[v.index].get_tag(); - write!(&mut ind_fmt, "Validator {} (a {tag}): ", v.index)?; - writeln!(ind_fmt.ind(2), "{}", v.payload)?; + writeln!(&mut ind_fmt, "Validator {} (a {tag}): ", v.index)?; + writeln!(ind_fmt.with_str(" "), "{}", v.payload)?; } Ok(()) } diff --git a/crates/integration_tests/src/test_config.rs b/crates/integration_tests/src/test_config.rs index 96edad3..039833a 100644 --- a/crates/integration_tests/src/test_config.rs +++ b/crates/integration_tests/src/test_config.rs @@ -11,6 +11,7 @@ pub struct TestConfig { /// /// This is used basically only for the self test where we have a test that does nothing to make sure the framework /// works. Other tests shouldn't have it, save while debugging weirdness, since the framework writes large files. + #[builder(default)] pub keep_artifacts_on_success: bool, } diff --git a/crates/integration_tests/src/test_runner.rs b/crates/integration_tests/src/test_runner.rs index f443419..7f9b171 100644 --- a/crates/integration_tests/src/test_runner.rs +++ b/crates/integration_tests/src/test_runner.rs @@ -80,9 +80,7 @@ pub fn run_single_test_in_parent(name: &str) -> Result Result impl FnMut(&TestContext, &[f32]) -> Result<(), String> + Send + Sync + 'static +{ + let mut frame_counter = 0u64; + + move |_ctx, frame| -> Result<(), String> { + let time_secs = frame_counter as f64 / syz::SR as f64; + let expected_val = (2.0 * std::f64::consts::PI * time_secs * FREQ).sin(); + for (c, s) in frame.iter().copied().enumerate() { + if (expected_val - (s as f64)).abs() > ERROR_RANGE { + return Err(format!( + "At frame {frame_counter} channel {c}: found {s} but expected {expected_val}" + )); + } + } + + frame_counter += 1; + Ok(()) + } +} + +fn basic_sine_config() -> TestConfig { + TestConfigBuilder::default() + .add_standard_validators() + .add_validator(validate_sin()) + .build() + .unwrap() +} + +fn basic_sine(context: &mut TestContext) -> Result<()> { + let sine = syz::nodes::TrigWaveformNode::new_sin(&context.server, FREQ)?; + let ao = syz::nodes::AudioOutputNode::new(&context.server, syz::ChannelFormat::Stereo)?; + context.server.connect(&sine, 0, &ao, 0)?; + context.advance(std::time::Duration::from_secs(10))?; + Ok(()) +} + +register_test!(basic_sine); diff --git a/crates/integration_tests/src/tests/mod.rs b/crates/integration_tests/src/tests/mod.rs index e086a23..b7291cb 100644 --- a/crates/integration_tests/src/tests/mod.rs +++ b/crates/integration_tests/src/tests/mod.rs @@ -1 +1,2 @@ +mod basic_sine; mod framework_self_test; diff --git a/crates/integration_tests/src/validators/function.rs b/crates/integration_tests/src/validators/function.rs index 71bfc6e..a4275de 100644 --- a/crates/integration_tests/src/validators/function.rs +++ b/crates/integration_tests/src/validators/function.rs @@ -10,13 +10,13 @@ enum FunctionValidator { impl super::Validator for FunctionValidator where - F: FnMut(&TestContext, &[f64]) -> Result<(), String> + Send + Sync + 'static, + F: FnMut(&TestContext, &[f32]) -> Result<(), String> + Send + Sync + 'static, { fn validate_frame( &mut self, context: &TestContext, location: &'static Location<'static>, - frame: &[f64], + frame: &[f32], ) { if let FunctionValidator::KeepGoing(ref mut cb) = self { if let Err(msg) = cb(context, frame) { @@ -37,7 +37,7 @@ where impl super::IntoValidator for F where - F: FnMut(&TestContext, &[f64]) -> Result<(), String> + Send + Sync + 'static, + F: FnMut(&TestContext, &[f32]) -> Result<(), String> + Send + Sync + 'static, { fn build_validator(self: Box, _context: &TestContext) -> Box { Box::new(FunctionValidator::KeepGoing(*self)) diff --git a/crates/integration_tests/src/validators/mod.rs b/crates/integration_tests/src/validators/mod.rs index 8b85419..9de0aa7 100644 --- a/crates/integration_tests/src/validators/mod.rs +++ b/crates/integration_tests/src/validators/mod.rs @@ -29,7 +29,7 @@ pub trait Validator: Send + Sync + 'static { &mut self, context: &TestContext, location: &'static Location<'static>, - frame: &[f64], + frame: &[f32], ); /// If this validator has failed, return `Err` explaining why. @@ -46,7 +46,7 @@ pub trait Validator: Send + Sync + 'static { &mut self, context: &TestContext, location: &'static Location<'static>, - data: &[f64], + data: &[f32], ) { let chancount = context.channel_format.get_channel_count().get(); diff --git a/crates/integration_tests/src/validators/range.rs b/crates/integration_tests/src/validators/range.rs index 42be80a..8ca2f53 100644 --- a/crates/integration_tests/src/validators/range.rs +++ b/crates/integration_tests/src/validators/range.rs @@ -14,7 +14,7 @@ enum State { Failed { index: u64, location: &'static Location<'static>, - value: f64, + value: f32, channel: u64, }, } @@ -30,7 +30,7 @@ impl Validator for RangeImpl { &mut self, _context: &TestContext, location: &'static Location<'static>, - frame: &[f64], + frame: &[f32], ) { if !self.state.is_good() { return;