From 93fd943d41885defeb7c8748f6d1b169e5681ed6 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 29 Aug 2023 11:41:15 -0500 Subject: [PATCH] chore: generalize snapshot tests (#288) --- src/lib.rs | 81 -------------- .../{type_tracing/tests.rs => helpers/mod.rs} | 102 ++++++++---------- .../{type_tracing => helpers}/test_builder.rs | 88 +++++++++------ tests/integration_test.rs | 85 ++++++++++++++- .../specs/graph/DynamicJsonIgnoresAssert.txt | 46 ++++++++ tests/type_tracing/in_memory_loader.rs | 66 ------------ tests/type_tracing/mod.rs | 3 - 7 files changed, 227 insertions(+), 244 deletions(-) rename tests/{type_tracing/tests.rs => helpers/mod.rs} (51%) rename tests/{type_tracing => helpers}/test_builder.rs (68%) create mode 100644 tests/specs/graph/DynamicJsonIgnoresAssert.txt delete mode 100644 tests/type_tracing/in_memory_loader.rs delete mode 100644 tests/type_tracing/mod.rs diff --git a/src/lib.rs b/src/lib.rs index e98629a1d..ca1156f78 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -391,87 +391,6 @@ mod tests { ); } - #[tokio::test] - async fn test_build_graph_dynamic_json_ignores_assert() { - let mut loader = setup( - vec![ - ( - "file:///a/test.js", - Source::Module { - specifier: "file:///a/test.js", - maybe_headers: None, - content: r#" - const a = await import("./a.json"); - "#, - }, - ), - ( - "file:///a/a.json", - Source::Module { - specifier: "file:///a/a.json", - maybe_headers: None, - content: r#"{"a":"b"}"#, - }, - ), - ], - vec![], - ); - let roots = vec![ModuleSpecifier::parse("file:///a/test.js").unwrap()]; - let mut graph = ModuleGraph::new(GraphKind::All); - graph - .build( - roots.clone(), - &mut loader, - BuildOptions { - is_dynamic: true, - ..Default::default() - }, - ) - .await; - assert_eq!( - json!(graph), - json!({ - "roots": [ - "file:///a/test.js", - ], - "modules": [ - { - "kind": "asserted", - "size": 9, - "mediaType": "Json", - "specifier": "file:///a/a.json" - }, - { - "dependencies": [ - { - "specifier": "./a.json", - "code": { - "specifier": "file:///a/a.json", - "span": { - "start": { - "line": 1, - "character": 31 - }, - "end": { - "line": 1, - "character": 41 - } - } - }, - "isDynamic": true - } - ], - "kind": "esm", - "mediaType": "JavaScript", - "size": 53, - "specifier": "file:///a/test.js" - } - ], - "redirects": {} - }) - ); - } - #[tokio::test] async fn test_valid_type_missing() { let mut loader = setup( diff --git a/tests/type_tracing/tests.rs b/tests/helpers/mod.rs similarity index 51% rename from tests/type_tracing/tests.rs rename to tests/helpers/mod.rs index d4534a41e..6d42c6999 100644 --- a/tests/type_tracing/tests.rs +++ b/tests/helpers/mod.rs @@ -1,54 +1,15 @@ use std::path::Path; use std::path::PathBuf; -use pretty_assertions::assert_eq; +mod test_builder; -use super::test_builder::TestBuilder; -use deno_graph::type_tracer::TypeTraceDiagnostic; +pub use test_builder::*; +use url::Url; -#[tokio::test] -async fn test_type_tracing_specs() { - for (test_file_path, spec) in - get_specs_in_dir(&PathBuf::from("./tests/specs/type_tracing")) - { - eprintln!("Running {}", test_file_path.display()); - let mut builder = TestBuilder::new(); - builder.with_loader(|loader| { - for file in &spec.files { - loader.add_file(&file.specifier, &file.text); - } - }); - - let result = builder.trace().await.unwrap(); - let update_var = std::env::var("UPDATE"); - let spec = if update_var.as_ref().map(|v| v.as_str()) == Ok("1") { - let mut spec = spec; - spec.output_file.text = result.output.clone(); - spec.diagnostics = result.diagnostics.clone(); - std::fs::write(&test_file_path, spec.emit()).unwrap(); - spec - } else { - spec - }; - assert_eq!( - result.output, - spec.output_file.text, - "Should be same for {}", - test_file_path.display() - ); - assert_eq!( - result.diagnostics, - spec.diagnostics, - "Should be same for {}", - test_file_path.display() - ); - } -} - -struct Spec { - files: Vec, - output_file: File, - diagnostics: Vec, +pub struct Spec { + pub files: Vec, + pub output_file: SpecFile, + pub diagnostics: Vec, } impl Spec { @@ -59,6 +20,9 @@ impl Spec { text.push('\n'); } text.push_str(&self.output_file.emit()); + if !text.ends_with('\n') { + text.push('\n'); + } if !self.diagnostics.is_empty() { text.push_str("\n# diagnostics\n"); text.push_str(&serde_json::to_string_pretty(&self.diagnostics).unwrap()); @@ -68,33 +32,45 @@ impl Spec { } } -struct File { - specifier: String, - text: String, +#[derive(Debug)] +pub struct SpecFile { + pub specifier: String, + pub text: String, } -impl File { +impl SpecFile { pub fn emit(&self) -> String { format!("# {}\n{}", self.specifier, self.text) } + + pub fn url(&self) -> Url { + let specifier = &self.specifier; + if !specifier.starts_with("http") && !specifier.starts_with("file") { + Url::parse(&format!("file:///{}", specifier)).unwrap() + } else { + Url::parse(specifier).unwrap() + } + } } -fn get_specs_in_dir(path: &Path) -> Vec<(PathBuf, Spec)> { - let files = get_files_in_dir_recursive(path); +pub fn get_specs_in_dir(path: &Path) -> Vec<(PathBuf, Spec)> { + let files = collect_files_in_dir_recursive(path); let files = if files .iter() - .any(|(s, _)| s.to_string_lossy().to_lowercase().contains("_only")) + .any(|file| file.path.to_string_lossy().to_lowercase().contains("_only")) { files .into_iter() - .filter(|(s, _)| s.to_string_lossy().to_lowercase().contains("_only")) + .filter(|file| { + file.path.to_string_lossy().to_lowercase().contains("_only") + }) .collect() } else { files }; files .into_iter() - .map(|(file_path, text)| (file_path, parse_spec(text))) + .map(|file| (file.path, parse_spec(file.text))) .collect() } @@ -106,7 +82,7 @@ fn parse_spec(text: String) -> Spec { if let Some(file) = current_file.take() { files.push(file); } - current_file = Some(File { + current_file = Some(SpecFile { specifier: specifier.to_string(), text: String::new(), }); @@ -136,16 +112,24 @@ fn parse_spec(text: String) -> Spec { } } -fn get_files_in_dir_recursive(path: &Path) -> Vec<(PathBuf, String)> { +struct CollectedFile { + pub path: PathBuf, + pub text: String, +} + +fn collect_files_in_dir_recursive(path: &Path) -> Vec { let mut result = Vec::new(); for entry in path.read_dir().unwrap().flatten() { let entry_path = entry.path(); if entry_path.is_file() { let text = std::fs::read_to_string(&entry_path).unwrap(); - result.push((entry_path, text)); + result.push(CollectedFile { + path: entry_path, + text, + }); } else { - result.extend(get_files_in_dir_recursive(&entry_path)); + result.extend(collect_files_in_dir_recursive(&entry_path)); } } diff --git a/tests/type_tracing/test_builder.rs b/tests/helpers/test_builder.rs similarity index 68% rename from tests/type_tracing/test_builder.rs rename to tests/helpers/test_builder.rs index f44213149..59db132e0 100644 --- a/tests/type_tracing/test_builder.rs +++ b/tests/helpers/test_builder.rs @@ -1,63 +1,67 @@ -use std::cell::RefCell; -use std::collections::BTreeMap; - use anyhow::Result; use deno_ast::ModuleSpecifier; use deno_ast::SourceRanged; -use deno_graph::type_tracer::trace_public_types; -use deno_graph::type_tracer::ModuleSymbol; -use deno_graph::type_tracer::RootSymbol; -use deno_graph::type_tracer::SymbolId; -use deno_graph::type_tracer::TypeTraceDiagnostic; -use deno_graph::type_tracer::TypeTraceHandler; +use deno_graph::source::MemoryLoader; use deno_graph::CapturingModuleAnalyzer; use deno_graph::DefaultModuleParser; use deno_graph::GraphKind; use deno_graph::ModuleGraph; +use std::collections::BTreeMap; -use super::in_memory_loader::InMemoryLoader; +#[cfg(feature = "type_tracing")] +pub mod tracing { + use std::cell::RefCell; -#[derive(Default)] -struct TestTypeTraceHandler { - diagnostics: RefCell>, -} + use deno_graph::type_tracer::RootSymbol; + use deno_graph::type_tracer::TypeTraceDiagnostic; + use deno_graph::type_tracer::TypeTraceHandler; + use deno_graph::ModuleGraph; -impl TestTypeTraceHandler { - pub fn diagnostics(self) -> Vec { - self.diagnostics.take() + #[derive(Default)] + pub struct TestTypeTraceHandler { + diagnostics: RefCell>, + } + + impl TestTypeTraceHandler { + pub fn diagnostics(self) -> Vec { + self.diagnostics.take() + } + } + + impl TypeTraceHandler for TestTypeTraceHandler { + fn diagnostic(&self, diagnostic: TypeTraceDiagnostic) { + self.diagnostics.borrow_mut().push(diagnostic); + } } -} -impl TypeTraceHandler for TestTypeTraceHandler { - fn diagnostic(&self, diagnostic: TypeTraceDiagnostic) { - self.diagnostics.borrow_mut().push(diagnostic); + pub struct TypeTraceResult { + pub graph: ModuleGraph, + pub root_symbol: RootSymbol, + pub output: String, + pub diagnostics: Vec, } } -pub struct TypeTraceResult { +pub struct BuildResult { pub graph: ModuleGraph, - pub root_symbol: RootSymbol, - pub output: String, - pub diagnostics: Vec, } pub struct TestBuilder { - loader: InMemoryLoader, + loader: MemoryLoader, entry_point: String, } impl TestBuilder { pub fn new() -> Self { - let loader = InMemoryLoader::default(); Self { - loader, + loader: Default::default(), entry_point: "file:///mod.ts".to_string(), } } pub fn with_loader( &mut self, - mut action: impl FnMut(&mut InMemoryLoader), + mut action: impl FnMut(&mut MemoryLoader), ) -> &mut Self { action(&mut self.loader); self @@ -69,8 +73,23 @@ impl TestBuilder { self } - pub async fn trace(&mut self) -> Result { - let handler = TestTypeTraceHandler::default(); + pub async fn build(&mut self) -> BuildResult { + let mut graph = deno_graph::ModuleGraph::new(GraphKind::All); + let entry_point_url = ModuleSpecifier::parse(&self.entry_point).unwrap(); + let roots = vec![entry_point_url.clone()]; + graph + .build( + roots.clone(), + &mut self.loader, + deno_graph::BuildOptions::default(), + ) + .await; + BuildResult { graph } + } + + #[cfg(feature = "type_tracing")] + pub async fn trace(&mut self) -> Result { + let handler = tracing::TestTypeTraceHandler::default(); let mut graph = deno_graph::ModuleGraph::new(GraphKind::All); let entry_point_url = ModuleSpecifier::parse(&self.entry_point).unwrap(); let roots = vec![entry_point_url.clone()]; @@ -84,13 +103,13 @@ impl TestBuilder { let source_parser = DefaultModuleParser::new_for_analysis(); let capturing_analyzer = CapturingModuleAnalyzer::new(Some(Box::new(source_parser)), None); - let root_symbol = trace_public_types( + let root_symbol = deno_graph::type_tracer::trace_public_types( &graph, &roots, &capturing_analyzer.as_capturing_parser(), &handler, )?; - Ok(TypeTraceResult { + Ok(tracing::TypeTraceResult { graph: graph.clone(), root_symbol: root_symbol.clone(), output: { @@ -102,7 +121,8 @@ impl TestBuilder { output_text.push_str(&format!("{}: {:#?}\n", k.as_str(), v)); } let get_symbol_text = - |module_symbol: &ModuleSymbol, symbol_id: SymbolId| { + |module_symbol: &deno_graph::type_tracer::ModuleSymbol, + symbol_id: deno_graph::type_tracer::SymbolId| { let symbol = module_symbol.symbol(symbol_id).unwrap(); let definitions = root_symbol.go_to_definitions(&graph, module_symbol, symbol); diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 45ff65d47..8f3e0ed02 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -5,6 +5,7 @@ // out of deno_graph that should be public use std::cell::RefCell; +use std::path::PathBuf; use std::rc::Rc; use anyhow::anyhow; @@ -20,9 +21,91 @@ use deno_semver::package::PackageNv; use deno_semver::package::PackageReq; use deno_semver::Version; use futures::future::LocalBoxFuture; +use pretty_assertions::assert_eq; + +use crate::helpers::get_specs_in_dir; +use crate::helpers::TestBuilder; + +mod helpers; + +#[tokio::test] +async fn test_graph_specs() { + for (test_file_path, spec) in + get_specs_in_dir(&PathBuf::from("./tests/specs/graph")) + { + eprintln!("Running {}", test_file_path.display()); + let mut builder = TestBuilder::new(); + builder.with_loader(|loader| { + for file in &spec.files { + loader.add_source_with_text(file.url(), &file.text); + } + }); + + let result = builder.build().await; + let update_var = std::env::var("UPDATE"); + let mut output_text = serde_json::to_string_pretty(&result.graph).unwrap(); + output_text.push('\n'); + let spec = if update_var.as_ref().map(|v| v.as_str()) == Ok("1") { + let mut spec = spec; + spec.output_file.text = output_text.clone(); + std::fs::write(&test_file_path, spec.emit()).unwrap(); + spec + } else { + spec + }; + assert_eq!( + output_text, + spec.output_file.text, + "Should be same for {}", + test_file_path.display() + ); + } +} #[cfg(feature = "type_tracing")] -mod type_tracing; +#[tokio::test] +async fn test_type_tracing_specs() { + for (test_file_path, spec) in + get_specs_in_dir(&PathBuf::from("./tests/specs/type_tracing")) + { + eprintln!("Running {}", test_file_path.display()); + let mut builder = TestBuilder::new(); + builder.with_loader(|loader| { + for file in &spec.files { + loader.add_source_with_text(file.url(), &file.text); + } + }); + + let result = builder.trace().await.unwrap(); + let update_var = std::env::var("UPDATE"); + let diagnostics = result + .diagnostics + .iter() + .map(|d| serde_json::to_value(d.clone()).unwrap()) + .collect::>(); + let spec = if update_var.as_ref().map(|v| v.as_str()) == Ok("1") { + let mut spec = spec; + spec.output_file.text = result.output.clone(); + spec.diagnostics = diagnostics.clone(); + std::fs::write(&test_file_path, spec.emit()).unwrap(); + spec + } else { + spec + }; + assert_eq!( + result.output, + spec.output_file.text, + "Should be same for {}", + test_file_path.display() + ); + assert_eq!( + diagnostics, + spec.diagnostics, + "Should be same for {}", + test_file_path.display() + ); + } +} #[tokio::test] async fn test_npm_version_not_found_then_found() { diff --git a/tests/specs/graph/DynamicJsonIgnoresAssert.txt b/tests/specs/graph/DynamicJsonIgnoresAssert.txt new file mode 100644 index 000000000..befbc071f --- /dev/null +++ b/tests/specs/graph/DynamicJsonIgnoresAssert.txt @@ -0,0 +1,46 @@ +# mod.ts +const a = await import("./a.json"); + +# a.json +{"a":"b"} + +# output +{ + "roots": [ + "file:///mod.ts" + ], + "modules": [ + { + "kind": "asserted", + "specifier": "file:///a.json", + "size": 10, + "mediaType": "Json" + }, + { + "kind": "esm", + "dependencies": [ + { + "specifier": "./a.json", + "code": { + "specifier": "file:///a.json", + "span": { + "start": { + "line": 0, + "character": 23 + }, + "end": { + "line": 0, + "character": 33 + } + } + }, + "isDynamic": true + } + ], + "size": 36, + "mediaType": "TypeScript", + "specifier": "file:///mod.ts" + } + ], + "redirects": {} +} diff --git a/tests/type_tracing/in_memory_loader.rs b/tests/type_tracing/in_memory_loader.rs deleted file mode 100644 index a908ce2b7..000000000 --- a/tests/type_tracing/in_memory_loader.rs +++ /dev/null @@ -1,66 +0,0 @@ -// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. - -use std::collections::HashMap; -use std::pin::Pin; -use std::sync::Arc; - -use anyhow::anyhow; -use anyhow::Result; -use deno_ast::ModuleSpecifier; -use deno_graph::source::LoadResponse; -use deno_graph::source::Loader; -use futures::Future; - -type RemoteFileText = Arc; -type RemoteFileHeaders = Option>; -type RemoteFileResult = Result<(RemoteFileText, RemoteFileHeaders), String>; - -#[derive(Clone, Default)] -pub struct InMemoryLoader { - modules: HashMap, -} - -impl InMemoryLoader { - pub fn add_file( - &mut self, - specifier: impl AsRef, - text: impl AsRef, - ) -> &mut Self { - let specifier = specifier.as_ref(); - let specifier = - if !specifier.starts_with("http") && !specifier.starts_with("file") { - ModuleSpecifier::parse(&format!("file:///{}", specifier)).unwrap() - } else { - ModuleSpecifier::parse(specifier).unwrap() - }; - self.modules.insert( - ModuleSpecifier::parse(specifier.as_ref()).unwrap(), - Ok((text.as_ref().into(), None)), - ); - self - } -} - -impl Loader for InMemoryLoader { - fn load( - &mut self, - specifier: &ModuleSpecifier, - _is_dynamic: bool, - ) -> Pin>> + 'static>> { - let specifier = specifier.clone(); - let result = self.modules.get(&specifier).map(|result| match result { - Ok(result) => Ok(LoadResponse::Module { - specifier, // todo: test a re-direct - content: result.0.clone(), - maybe_headers: result.1.clone(), - }), - Err(err) => Err(err), - }); - let result = match result { - Some(Ok(result)) => Ok(Some(result)), - Some(Err(err)) => Err(anyhow!("{}", err)), - None => Ok(None), - }; - Box::pin(futures::future::ready(result)) - } -} diff --git a/tests/type_tracing/mod.rs b/tests/type_tracing/mod.rs deleted file mode 100644 index 0d5097072..000000000 --- a/tests/type_tracing/mod.rs +++ /dev/null @@ -1,3 +0,0 @@ -mod in_memory_loader; -mod test_builder; -mod tests;