From d0e64fd80a707d19f0e7edfe34018e176c8ad3e5 Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Tue, 19 Nov 2024 10:52:42 +0000 Subject: [PATCH] refactor(transform_conformance): remove `TestCase` trait (#7356) --- tasks/transform_conformance/src/lib.rs | 33 ++- tasks/transform_conformance/src/test_case.rs | 214 ++++++------------- 2 files changed, 76 insertions(+), 171 deletions(-) diff --git a/tasks/transform_conformance/src/lib.rs b/tasks/transform_conformance/src/lib.rs index 3620ec70f2af6..7d5a1160f3bfc 100644 --- a/tasks/transform_conformance/src/lib.rs +++ b/tasks/transform_conformance/src/lib.rs @@ -13,7 +13,7 @@ use std::{ use constants::PLUGINS; use indexmap::IndexMap; use oxc_tasks_common::{normalize_path, project_root, Snapshot}; -use test_case::TestCaseKind; +use test_case::{TestCase, TestCaseKind}; use walkdir::WalkDir; #[test] @@ -93,15 +93,15 @@ impl TestRunner { fn glob_files( root: &Path, filter: Option<&String>, - ) -> (IndexMap>, IndexMap>) { + ) -> (IndexMap>, IndexMap>) { let cwd = root.parent().unwrap_or(root); // use `IndexMap` to keep the order of the test cases the same in insert order. - let mut transform_files = IndexMap::>::new(); - let mut exec_files = IndexMap::>::new(); + let mut transform_files = IndexMap::>::new(); + let mut exec_files = IndexMap::>::new(); for case in PLUGINS { let root = root.join(case).join("test/fixtures"); - let (mut transform_paths, mut exec_paths): (Vec, Vec) = + let (mut transform_paths, mut exec_paths): (Vec, Vec) = WalkDir::new(root) .into_iter() .filter_map(Result::ok) @@ -112,12 +112,12 @@ impl TestRunner { return None; } } - TestCaseKind::new(cwd, path).filter(|test_case| !test_case.skip_test_case()) + TestCase::new(cwd, path).filter(|test_case| !test_case.skip_test_case()) }) - .partition(|p| matches!(p, TestCaseKind::Transform(_))); + .partition(|case| case.kind == TestCaseKind::Conformance); - transform_paths.sort_unstable_by(|a, b| a.path().cmp(b.path())); - exec_paths.sort_unstable_by(|a, b| a.path().cmp(b.path())); + transform_paths.sort_unstable_by(|a, b| a.path.cmp(&b.path)); + exec_paths.sort_unstable_by(|a, b| a.path.cmp(&b.path)); if !transform_paths.is_empty() { transform_files.insert((*case).to_string(), transform_paths); @@ -130,12 +130,7 @@ impl TestRunner { (transform_files, exec_files) } - fn generate_snapshot( - &self, - root: &Path, - dest: &Path, - paths: IndexMap>, - ) { + fn generate_snapshot(&self, root: &Path, dest: &Path, paths: IndexMap>) { let mut snapshot = String::new(); let mut total = 0; let mut all_passed = vec![]; @@ -147,13 +142,13 @@ impl TestRunner { total += num_of_tests; // Run the test - let (passed, failed): (Vec, Vec) = test_cases + let (passed, failed): (Vec, Vec) = test_cases .into_iter() .map(|mut test_case| { test_case.test(self.options.filter.is_some()); test_case }) - .partition(|test_case| test_case.errors().is_empty()); + .partition(|test_case| test_case.errors.is_empty()); all_passed_count += passed.len(); // Snapshot @@ -166,9 +161,9 @@ impl TestRunner { for test_case in failed { snapshot.push_str("* "); snapshot.push_str(&normalize_path( - test_case.path().strip_prefix(&case_root).unwrap(), + test_case.path.strip_prefix(&case_root).unwrap(), )); - let errors = test_case.errors(); + let errors = test_case.errors; if !errors.is_empty() { snapshot.push('\n'); for error in errors { diff --git a/tasks/transform_conformance/src/test_case.rs b/tasks/transform_conformance/src/test_case.rs index d9105e6a9214f..3b8063826c28c 100644 --- a/tasks/transform_conformance/src/test_case.rs +++ b/tasks/transform_conformance/src/test_case.rs @@ -22,83 +22,54 @@ use crate::{ }; #[derive(Debug)] +pub struct TestCase { + pub kind: TestCaseKind, + pub path: PathBuf, + pub options: BabelOptions, + pub transform_options: Result>, + pub errors: Vec, +} + +#[derive(Debug, Clone, Copy, Eq, PartialEq)] pub enum TestCaseKind { - Transform(ConformanceTestCase), - Exec(ExecTestCase), + Conformance, + Exec, } -impl TestCaseKind { +impl TestCase { pub fn new(cwd: &Path, path: &Path) -> Option { + let mut options = BabelOptions::from_test_path(path.parent().unwrap()); + options.cwd.replace(cwd.to_path_buf()); + let transform_options = TransformOptions::try_from(&options); + let path = path.to_path_buf(); + let errors = vec![]; + // in `exec` directory - if path.parent().is_some_and(|path| path.file_name().is_some_and(|n| n == "exec")) - && path.extension().is_some_and(|ext| VALID_EXTENSIONS.contains(&ext.to_str().unwrap())) + let kind = if path + .extension() + .is_some_and(|ext| VALID_EXTENSIONS.contains(&ext.to_str().unwrap())) + && (path.parent().is_some_and(|path| path.file_name().is_some_and(|n| n == "exec")) + || path.file_stem().is_some_and(|name| name == "exec")) { - return Some(Self::Exec(ExecTestCase::new(cwd, path))); + TestCaseKind::Exec } - // named `exec.[ext]` - if path.file_stem().is_some_and(|name| name == "exec") - && path.extension().is_some_and(|ext| VALID_EXTENSIONS.contains(&ext.to_str().unwrap())) - { - return Some(Self::Exec(ExecTestCase::new(cwd, path))); - } - // named `input.[ext]` or `input.d.ts` - if (path.file_stem().is_some_and(|name| name == "input") + else if (path.file_stem().is_some_and(|name| name == "input") && path .extension() .is_some_and(|ext| VALID_EXTENSIONS.contains(&ext.to_str().unwrap()))) || path.file_name().is_some_and(|name| name == "input.d.ts") { - return Some(Self::Transform(ConformanceTestCase::new(cwd, path))); - } + TestCaseKind::Conformance + } else { + return None; + }; - None + Some(Self { kind, path, options, transform_options, errors }) } pub fn skip_test_case(&self) -> bool { - match self { - Self::Transform(test_case) => test_case.skip_test_case(), - Self::Exec(exec_case) => exec_case.skip_test_case(), - } - } - - pub fn path(&self) -> &Path { - match self { - Self::Transform(test_case) => &test_case.path, - Self::Exec(exec_case) => &exec_case.path, - } - } - - pub fn test(&mut self, filter: bool) { - match self { - Self::Transform(test_case) => test_case.test(filter), - Self::Exec(test_case) => test_case.test(filter), - } - } - - pub fn errors(&self) -> &Vec { - match self { - Self::Transform(test_case) => test_case.errors(), - Self::Exec(test_case) => test_case.errors(), - } - } -} - -pub trait TestCase { - fn new(cwd: &Path, path: &Path) -> Self; - - fn options(&self) -> &BabelOptions; - - fn transform_options(&self) -> &Result>; - - fn test(&mut self, filtered: bool); - - fn errors(&self) -> &Vec; - - fn path(&self) -> &Path; - - fn skip_test_case(&self) -> bool { - let options = self.options(); + let options = &self.options; // Skip plugins we don't support yet if PLUGINS_NOT_SUPPORTED_YET @@ -111,7 +82,7 @@ pub trait TestCase { if let Some(b) = options.babel_8_breaking { if b { // Skip deprecated react options - if self.transform_options().as_ref().is_ok_and(|options| { + if self.transform_options.as_ref().is_ok_and(|options| { options.jsx.use_built_ins.is_some() || options.jsx.use_spread.is_some() }) { return true; @@ -133,7 +104,7 @@ pub trait TestCase { } // Skip some Babel tests. - if let Ok(path) = self.path().strip_prefix(packages_root()) { + if let Ok(path) = self.path.strip_prefix(packages_root()) { // babel skip test cases that in a directory starting with a dot // https://github.com/babel/babel/blob/0effd92d886b7135469d23612ceba6414c721673/packages/babel-helper-fixtures/src/index.ts#L223 if path.components().any(|c| c.as_os_str().to_str().unwrap().starts_with('.')) { @@ -146,7 +117,7 @@ pub trait TestCase { } } - let dir = self.path().parent().unwrap(); + let dir = self.path.parent().unwrap(); // Skip custom plugin.js if dir.join("plugin.js").exists() { return true; @@ -161,7 +132,7 @@ pub trait TestCase { } fn transform(&self, path: &Path) -> Result { - let transform_options = match self.transform_options() { + let transform_options = match &self.transform_options { Ok(transform_options) => transform_options, Err(json_err) => { return Err(OxcDiagnostic::error(format!("{json_err:?}"))); @@ -173,8 +144,8 @@ pub trait TestCase { // Some babel test cases have a js extension, but contain typescript code. // Therefore, if the typescript plugin exists, enable typescript. let source_type = SourceType::from_path(path).unwrap().with_typescript( - self.options().plugins.syntax_typescript.is_some() - || self.options().plugins.typescript.is_some(), + self.options.plugins.syntax_typescript.is_some() + || self.options.plugins.typescript.is_some(), ); let mut options = transform_options.clone(); @@ -182,42 +153,16 @@ pub trait TestCase { let driver = Driver::new(false, options).execute(&source_text, source_type, path); Ok(driver) } -} - -#[derive(Debug)] -pub struct ConformanceTestCase { - path: PathBuf, - options: BabelOptions, - transform_options: Result>, - errors: Vec, -} - -impl TestCase for ConformanceTestCase { - fn new(cwd: &Path, path: &Path) -> Self { - let mut options = BabelOptions::from_test_path(path.parent().unwrap()); - options.cwd.replace(cwd.to_path_buf()); - let transform_options = TransformOptions::try_from(&options); - Self { path: path.to_path_buf(), options, transform_options, errors: vec![] } - } - - fn options(&self) -> &BabelOptions { - &self.options - } - - fn transform_options(&self) -> &Result> { - &self.transform_options - } - - fn path(&self) -> &Path { - &self.path - } - fn errors(&self) -> &Vec { - &self.errors + pub fn test(&mut self, filtered: bool) { + match self.kind { + TestCaseKind::Conformance => self.test_conformance(filtered), + TestCaseKind::Exec => self.test_exec(filtered), + } } /// Test conformance by comparing the parsed babel code and transformed code. - fn test(&mut self, filtered: bool) { + fn test_conformance(&mut self, filtered: bool) { let output_path = self.path.parent().unwrap().read_dir().unwrap().find_map(|entry| { let path = entry.ok()?.path(); let file_stem = path.file_stem()?; @@ -259,7 +204,7 @@ impl TestCase for ConformanceTestCase { let mut actual_errors = None; let mut transform_options = None; - match self.transform_options() { + match &self.transform_options { Err(json_err) => { let error = json_err.iter().map(ToString::to_string).collect::>().join("\n"); actual_errors.replace(get_babel_error(&error)); @@ -285,7 +230,7 @@ impl TestCase for ConformanceTestCase { } } - let babel_options = self.options(); + let babel_options = &self.options; let output; let passed = if let Some(throws) = &babel_options.throws { @@ -372,17 +317,25 @@ impl TestCase for ConformanceTestCase { self.errors.push(OxcDiagnostic::error(actual_errors)); } } -} -#[derive(Debug)] -pub struct ExecTestCase { - path: PathBuf, - options: BabelOptions, - transform_options: Result>, - errors: Vec, -} + fn test_exec(&mut self, filtered: bool) { + if filtered { + println!("input_path: {:?}", &self.path); + println!("Input:\n{}\n", fs::read_to_string(&self.path).unwrap()); + } + + let result = match self.transform(&self.path) { + Ok(mut driver) => driver.printed(), + Err(error) => { + if filtered { + println!("Transform Errors:\n{error:?}\n",); + } + return; + } + }; + self.write_to_test_files(&result); + } -impl ExecTestCase { fn write_to_test_files(&self, content: &str) { let unprefixed_path = self .path @@ -424,49 +377,6 @@ test("exec", () => {{ } } -impl TestCase for ExecTestCase { - fn new(cwd: &Path, path: &Path) -> Self { - let mut options = BabelOptions::from_test_path(path.parent().unwrap()); - options.cwd.replace(cwd.to_path_buf()); - let transform_options = TransformOptions::try_from(&options); - Self { path: path.to_path_buf(), options, transform_options, errors: vec![] } - } - - fn options(&self) -> &BabelOptions { - &self.options - } - - fn transform_options(&self) -> &Result> { - &self.transform_options - } - - fn path(&self) -> &Path { - &self.path - } - - fn errors(&self) -> &Vec { - &self.errors - } - - fn test(&mut self, filtered: bool) { - if filtered { - println!("input_path: {:?}", &self.path); - println!("Input:\n{}\n", fs::read_to_string(&self.path).unwrap()); - } - - let result = match self.transform(&self.path) { - Ok(mut driver) => driver.printed(), - Err(error) => { - if filtered { - println!("Transform Errors:\n{error:?}\n",); - } - return; - } - }; - self.write_to_test_files(&result); - } -} - fn get_babel_error(error: &str) -> String { match error { "transform-react-jsx: unknown variant `invalidOption`, expected `classic` or `automatic`" => "Runtime must be either \"classic\" or \"automatic\".",