diff --git a/rojo-insta-ext/src/redaction_map.rs b/rojo-insta-ext/src/redaction_map.rs index dce3cb5d..22574704 100644 --- a/rojo-insta-ext/src/redaction_map.rs +++ b/rojo-insta-ext/src/redaction_map.rs @@ -18,6 +18,16 @@ impl RedactionMap { } } + pub fn get_redacted_value(&self, id: impl ToString) -> Option { + let id = id.to_string(); + + if self.ids.contains_key(&id) { + Some(id) + } else { + None + } + } + pub fn intern(&mut self, id: impl ToString) { let last_id = &mut self.last_id; diff --git a/rojo-test/serve-test-snapshots/serve_test__add_folder_all-2.snap b/rojo-test/serve-test-snapshots/serve_test__add_folder_all-2.snap new file mode 100644 index 00000000..94948cf8 --- /dev/null +++ b/rojo-test/serve-test-snapshots/serve_test__add_folder_all-2.snap @@ -0,0 +1,26 @@ +--- +source: rojo-test/src/serve_test.rs +expression: "read_response.intern_and_redact(&mut redactions, root_id)" +--- +instances: + id-2: + Id: id-2 + Parent: ~ + Name: add_folder + ClassName: Folder + Properties: {} + Children: + - id-3 + Metadata: + ignoreUnknownInstances: false + id-3: + Id: id-3 + Parent: id-2 + Name: my-new-folder + ClassName: Folder + Properties: {} + Children: [] + Metadata: + ignoreUnknownInstances: false +messageCursor: 1 +sessionId: id-1 diff --git a/rojo-test/serve-test-snapshots/serve_test__add_folder_all.snap b/rojo-test/serve-test-snapshots/serve_test__add_folder_all.snap new file mode 100644 index 00000000..03362f41 --- /dev/null +++ b/rojo-test/serve-test-snapshots/serve_test__add_folder_all.snap @@ -0,0 +1,16 @@ +--- +source: rojo-test/src/serve_test.rs +expression: "read_response.intern_and_redact(&mut redactions, root_id)" +--- +instances: + id-2: + Id: id-2 + Parent: ~ + Name: add_folder + ClassName: Folder + Properties: {} + Children: [] + Metadata: + ignoreUnknownInstances: false +messageCursor: 0 +sessionId: id-1 diff --git a/rojo-test/serve-test-snapshots/serve_test__add_folder_info.snap b/rojo-test/serve-test-snapshots/serve_test__add_folder_info.snap new file mode 100644 index 00000000..f826d7e7 --- /dev/null +++ b/rojo-test/serve-test-snapshots/serve_test__add_folder_info.snap @@ -0,0 +1,9 @@ +--- +source: rojo-test/src/serve_test.rs +expression: redactions.redacted_yaml(info) +--- +expectedPlaceIds: ~ +protocolVersion: 3 +rootInstanceId: id-2 +serverVersion: 0.6.0-dev +sessionId: id-1 diff --git a/rojo-test/serve-test-snapshots/serve_test__add_folder_subscribe.snap b/rojo-test/serve-test-snapshots/serve_test__add_folder_subscribe.snap new file mode 100644 index 00000000..bfdfeadf --- /dev/null +++ b/rojo-test/serve-test-snapshots/serve_test__add_folder_subscribe.snap @@ -0,0 +1,19 @@ +--- +source: rojo-test/src/serve_test.rs +expression: "subscribe_response.intern_and_redact(&mut redactions, ())" +--- +messageCursor: 1 +messages: + - removedInstances: [] + addedInstances: + id-3: + Id: id-3 + Parent: id-2 + Name: my-new-folder + ClassName: Folder + Properties: {} + Children: [] + Metadata: + ignoreUnknownInstances: false + updatedInstances: [] +sessionId: id-1 diff --git a/rojo-test/src/internable.rs b/rojo-test/src/internable.rs new file mode 100644 index 00000000..9c1cbb57 --- /dev/null +++ b/rojo-test/src/internable.rs @@ -0,0 +1,104 @@ +use std::collections::HashMap; + +use rbx_dom_weak::RbxId; +use serde::Serialize; + +use librojo::web_interface::{Instance, InstanceUpdate, ReadResponse, SubscribeResponse}; +use rojo_insta_ext::RedactionMap; + +/// A convenience method to store all of the redactable data from a piece of +/// data, then immediately redact it and return a serde_yaml Value. +pub trait InternAndRedact { + fn intern_and_redact(&self, redactions: &mut RedactionMap, extra: T) -> serde_yaml::Value; +} + +impl InternAndRedact for I +where + I: Serialize + Internable, +{ + fn intern_and_redact(&self, redactions: &mut RedactionMap, extra: T) -> serde_yaml::Value { + self.intern(redactions, extra); + redactions.redacted_yaml(self) + } +} + +/// A trait to describe how to discover redactable data from an type. +/// +/// The 'extra' parameter is a kludge to support types like Instance or +/// ReadResponse that need some additional information in order to be +/// deterministic. +pub trait Internable { + fn intern(&self, redactions: &mut RedactionMap, extra: T); +} + +impl Internable for ReadResponse<'_> { + fn intern(&self, redactions: &mut RedactionMap, root_id: RbxId) { + redactions.intern(root_id); + + let root_instance = self.instances.get(&root_id).unwrap(); + + for &child_id in root_instance.children.iter() { + self.intern(redactions, child_id); + } + } +} + +impl<'a> Internable<&'a HashMap>> for Instance<'a> { + fn intern( + &self, + redactions: &mut RedactionMap, + other_instances: &HashMap>, + ) { + redactions.intern(self.id); + + for child_id in self.children.iter() { + let child = &other_instances[child_id]; + child.intern(redactions, other_instances); + } + } +} + +impl Internable<()> for SubscribeResponse<'_> { + fn intern(&self, redactions: &mut RedactionMap, _extra: ()) { + for message in &self.messages { + intern_instance_updates(redactions, &message.updated_instances); + intern_instance_additions(redactions, &message.added_instances); + } + } +} + +fn intern_instance_updates(redactions: &mut RedactionMap, updates: &[InstanceUpdate]) { + for update in updates { + redactions.intern(update.id); + } +} + +fn intern_instance_additions( + redactions: &mut RedactionMap, + additions: &HashMap>, +) { + // This method redacts in a deterministic order from a HashMap by collecting + // all of the instances that are direct children of instances we've already + // interned. + let mut added_roots = Vec::new(); + + for (id, added) in additions { + let parent_id = added.parent.unwrap(); + let parent_redacted = redactions.get_redacted_value(parent_id); + + // Here, we assume that instances are only added to other instances that + // we've already interned. If that's not true, then we'll have some + // dangling unredacted IDs. + if let Some(parent_redacted) = parent_redacted { + added_roots.push((id, parent_redacted)); + } + } + + // Sort the input by the redacted key, which should match the traversal + // order we need for the tree. + added_roots.sort_unstable_by(|a, b| a.1.cmp(&b.1)); + + for (root_id, _redacted_id) in added_roots { + additions[root_id].intern(redactions, additions); + } +} diff --git a/rojo-test/src/lib.rs b/rojo-test/src/lib.rs index 29258e3b..cd4e7461 100644 --- a/rojo-test/src/lib.rs +++ b/rojo-test/src/lib.rs @@ -1,8 +1,7 @@ #![cfg(test)] -#[macro_use] -mod serve_util; - mod build_test; +mod internable; mod serve_test; +mod serve_util; mod util; diff --git a/rojo-test/src/serve_test.rs b/rojo-test/src/serve_test.rs index 7a322e53..cbb0167f 100644 --- a/rojo-test/src/serve_test.rs +++ b/rojo-test/src/serve_test.rs @@ -2,7 +2,7 @@ use std::fs; use insta::assert_yaml_snapshot; -use crate::serve_util::{run_serve_test, InternAndRedact}; +use crate::{internable::InternAndRedact, serve_util::run_serve_test}; #[test] fn empty() { @@ -39,7 +39,7 @@ fn scripts() { let subscribe_response = session.get_api_subscribe(0).unwrap(); assert_yaml_snapshot!( "scripts_subscribe", - redactions.redacted_yaml(subscribe_response) + subscribe_response.intern_and_redact(&mut redactions, ()) ); let read_response = session.get_api_read(root_id).unwrap(); @@ -70,9 +70,7 @@ fn just_txt() { }); } -// TODO: Rojo doesn't currently handle files/folders being added. #[test] -#[ignore] fn add_folder() { run_serve_test("add_folder", |session, mut redactions| { let info = session.get_api_rojo().unwrap(); @@ -91,7 +89,7 @@ fn add_folder() { let subscribe_response = session.get_api_subscribe(0).unwrap(); assert_yaml_snapshot!( "add_folder_subscribe", - redactions.redacted_yaml(subscribe_response) + subscribe_response.intern_and_redact(&mut redactions, ()) ); let read_response = session.get_api_read(root_id).unwrap(); diff --git a/rojo-test/src/serve_util.rs b/rojo-test/src/serve_util.rs index 95695053..c0c67c1f 100644 --- a/rojo-test/src/serve_util.rs +++ b/rojo-test/src/serve_util.rs @@ -3,10 +3,12 @@ use std::{ path::{Path, PathBuf}, process::Command, sync::atomic::{AtomicUsize, Ordering}, + thread, + time::Duration, }; use rbx_dom_weak::RbxId; -use serde::Serialize; + use tempfile::{tempdir, TempDir}; use librojo::web_interface::{ReadResponse, ServerInfoResponse, SubscribeResponse}; @@ -16,6 +18,13 @@ use crate::util::{ copy_recursive, get_rojo_path, get_serve_tests_path, get_working_dir_path, KillOnDrop, }; +/// Convenience method to run a `rojo serve` test. +/// +/// Test projects should be defined in the `serve-tests` folder; their filename +/// should be given as the first parameter. +/// +/// The passed in callback is where the actual test body should go. Setup and +/// cleanup happens automatically. pub fn run_serve_test(test_name: &str, callback: impl FnOnce(TestServeSession, RedactionMap)) { let _ = env_logger::try_init(); @@ -36,42 +45,7 @@ pub fn run_serve_test(test_name: &str, callback: impl FnOnce(TestServeSession, R settings.bind(move || callback(session, redactions)); } -pub trait Internable { - fn intern(&self, redactions: &mut RedactionMap, extra: T); -} - -impl Internable for ReadResponse<'_> { - fn intern(&self, redactions: &mut RedactionMap, root_id: RbxId) { - redactions.intern(root_id); - - let root_instance = self.instances.get(&root_id).unwrap(); - - for &child_id in root_instance.children.iter() { - self.intern(redactions, child_id); - } - } -} - -pub trait InternAndRedact { - fn intern_and_redact(&self, redactions: &mut RedactionMap, extra: T) -> serde_yaml::Value; -} - -impl InternAndRedact for I -where - I: Serialize + Internable, -{ - fn intern_and_redact(&self, redactions: &mut RedactionMap, extra: T) -> serde_yaml::Value { - self.intern(redactions, extra); - redactions.redacted_yaml(self) - } -} - -fn get_port_number() -> usize { - static NEXT_PORT_NUMBER: AtomicUsize = AtomicUsize::new(35103); - - NEXT_PORT_NUMBER.fetch_add(1, Ordering::SeqCst) -} - +/// Represents a running Rojo serve session running in a temporary directory. pub struct TestServeSession { // Drop order is important here: we want the process to be killed before the // directory it's operating on is destroyed. @@ -129,19 +103,28 @@ impl TestServeSession { &self.project_path } + /// Waits for the `rojo serve` server to come online with expontential + /// backoff. pub fn wait_to_come_online(&mut self) -> ServerInfoResponse { - loop { + const BASE_DURATION_MS: f32 = 30.0; + const EXP_BACKOFF_FACTOR: f32 = 1.3; + const MAX_TRIES: u32 = 5; + + for i in 1..=MAX_TRIES { match self.rojo_process.0.try_wait() { Ok(Some(status)) => panic!("Rojo process exited with status {}", status), - Ok(None) => { /* process is still running */ } + Ok(None) => { /* The process is still running, as expected */ } Err(err) => panic!("Failed to wait on Rojo process: {}", err), } let info = match self.get_api_rojo() { Ok(info) => info, Err(err) => { - log::debug!("Server error, retrying: {}", err); - std::thread::sleep(std::time::Duration::from_millis(30)); + let retry_time_ms = BASE_DURATION_MS * (i as f32).powf(EXP_BACKOFF_FACTOR); + let retry_time = Duration::from_millis(retry_time_ms as u64); + + log::info!("Server error, retrying in {:?}: {}", retry_time, err); + thread::sleep(retry_time); continue; } }; @@ -150,6 +133,8 @@ impl TestServeSession { return info; } + + panic!("Rojo server did not respond after {} tries.", MAX_TRIES); } pub fn get_api_rojo(&self) -> Result { @@ -175,3 +160,15 @@ impl TestServeSession { reqwest::get(&url)?.json() } } + +/// Probably-okay way to generate random enough port numbers for running the +/// Rojo live server. +/// +/// If this method ends up having problems, we should add an option for Rojo to +/// use a random port chosen by the operating system and figure out a good way +/// to get that port back to the test CLI. +fn get_port_number() -> usize { + static NEXT_PORT_NUMBER: AtomicUsize = AtomicUsize::new(35103); + + NEXT_PORT_NUMBER.fetch_add(1, Ordering::SeqCst) +}