tests: Refactor code so all testing goes through Test::test(), allows for approximations with custom hooks

tests/tests/external_interface/tests.rs

@@ -1,25 +1,22 @@
 use crate::external_interface::ExternalInterfaceTestProvider;
 use crate::set_logger;
 use crate::util::options::TestOptions;
-use crate::util::runner::test_swf_with_hooks;
 use crate::util::test::Test;
-use anyhow::Result;
 use ruffle_core::external::Value as ExternalValue;
 use std::collections::BTreeMap;
 use std::path::Path;
 
-pub fn external_interface_avm1() -> Result<()> {
+pub fn external_interface_avm1() -> Result<(), libtest_mimic::Failed> {
     set_logger();
-    let test = Test::from_options(
+    Test::from_options(
         TestOptions {
             num_frames: 1,
             ..Default::default()
         },
         Path::new("tests/swfs/avm1/external_interface/"),
         Path::new("tests/swfs"),
-    )?;
+    )?
-    test_swf_with_hooks(
+    .run(
-        &test,
         |player| {
             player
                 .lock()
@@ -65,18 +62,17 @@ pub fn external_interface_avm1() -> Result<()> {
     )
 }
 
-pub fn external_interface_avm2() -> Result<()> {
+pub fn external_interface_avm2() -> Result<(), libtest_mimic::Failed> {
     set_logger();
-    let test = Test::from_options(
+    Test::from_options(
         TestOptions {
             num_frames: 1,
             ..Default::default()
         },
         Path::new("tests/swfs/avm2/external_interface/"),
         Path::new("tests/swfs"),
-    )?;
+    )?
-    test_swf_with_hooks(
+    .run(
-        &test,
         |player| {
             player
                 .lock()

tests/tests/regression_tests.rs

@@ -34,7 +34,7 @@ fn main() {
                 .context("Couldn't create test")
                 .unwrap();
             let ignore = !test.should_run();
-            let mut trial = Trial::test(test.name.to_string(), || test.run());
+            let mut trial = Trial::test(test.name.to_string(), || test.run(|_| Ok(()), |_| Ok(())));
             if ignore {
                 trial = trial.with_ignored_flag(true);
             }
@@ -43,17 +43,13 @@ fn main() {
         .collect();
 
     // Manual tests here, since #[test] doesn't work once we use our own test harness
-    tests.push(Trial::test("shared_object_avm1", || {
+    tests.push(Trial::test("shared_object_avm1", || shared_object_avm1()));
-        shared_object_avm1().map_err(|e| e.to_string().into())
+    tests.push(Trial::test("shared_object_avm2", || shared_object_avm2()));
-    }));
-    tests.push(Trial::test("shared_object_avm2", || {
-        shared_object_avm2().map_err(|e| e.to_string().into())
-    }));
     tests.push(Trial::test("external_interface_avm1", || {
-        external_interface_avm1().map_err(|e| e.to_string().into())
+        external_interface_avm1()
     }));
     tests.push(Trial::test("external_interface_avm2", || {
-        external_interface_avm2().map_err(|e| e.to_string().into())
+        external_interface_avm2()
     }));
 
     tests.sort_unstable_by(|a, b| a.name().cmp(b.name()));

tests/tests/shared_object/mod.rs

@@ -1,12 +1,10 @@
 use crate::set_logger;
 use crate::util::options::TestOptions;
-use crate::util::runner::test_swf_with_hooks;
 use crate::util::test::Test;
-use anyhow::Result;
 use ruffle_core::backend::storage::{MemoryStorageBackend, StorageBackend};
 use std::path::Path;
 
-pub fn shared_object_avm1() -> Result<()> {
+pub fn shared_object_avm1() -> Result<(), libtest_mimic::Failed> {
     set_logger();
     // Test SharedObject persistence. Run an SWF that saves data
     // to a shared object twice and verify that the data is saved.
@@ -14,17 +12,17 @@ pub fn shared_object_avm1() -> Result<()> {
         Box::<MemoryStorageBackend>::default();
 
     // Initial run; no shared object data.
-    test_swf_with_hooks(
+    Test::from_options(
-        &Test::from_options(
+        TestOptions {
-            TestOptions {
+            num_frames: 1,
-                num_frames: 1,
+            output_path: "output1.txt".into(),
-                output_path: "output1.txt".into(),
+            ..Default::default()
-                ..Default::default()
+        },
-            },
+        Path::new("tests/swfs/avm1/shared_object/"),
-            Path::new("tests/swfs/avm1/shared_object/"),
+        Path::new("tests/swfs"),
-            Path::new("tests/swfs"),
+    )?
-        )?,
+    .run(
-        |_player| Ok(()),
+        |_| Ok(()),
         |player| {
             // Save the storage backend for next run.
             let mut player = player.lock().unwrap();
@@ -43,29 +41,29 @@ pub fn shared_object_avm1() -> Result<()> {
     );
 
     // Re-run the SWF, verifying that the shared object persists.
-    test_swf_with_hooks(
+    Test::from_options(
-        &Test::from_options(
+        TestOptions {
-            TestOptions {
+            num_frames: 1,
-                num_frames: 1,
+            output_path: "output2.txt".into(),
-                output_path: "output2.txt".into(),
+            ..Default::default()
-                ..Default::default()
+        },
-            },
+        Path::new("tests/swfs/avm1/shared_object/"),
-            Path::new("tests/swfs/avm1/shared_object/"),
+        Path::new("tests/swfs"),
-            Path::new("tests/swfs"),
+    )?
-        )?,
+    .run(
         |player| {
             // Swap in the previous storage backend.
             let mut player = player.lock().unwrap();
             std::mem::swap(player.storage_mut(), &mut memory_storage_backend);
             Ok(())
         },
-        |_player| Ok(()),
+        |_| Ok(()),
     )?;
 
     Ok(())
 }
 
-pub fn shared_object_avm2() -> Result<()> {
+pub fn shared_object_avm2() -> Result<(), libtest_mimic::Failed> {
     set_logger();
     // Test SharedObject persistence. Run an SWF that saves data
     // to a shared object twice and verify that the data is saved.
@@ -73,16 +71,16 @@ pub fn shared_object_avm2() -> Result<()> {
         Box::<MemoryStorageBackend>::default();
 
     // Initial run; no shared object data.
-    test_swf_with_hooks(
+    Test::from_options(
-        &Test::from_options(
+        TestOptions {
-            TestOptions {
+            num_frames: 1,
-                num_frames: 1,
+            output_path: "output1.txt".into(),
-                output_path: "output1.txt".into(),
+            ..Default::default()
-                ..Default::default()
+        },
-            },
+        Path::new("tests/swfs/avm2/shared_object/"),
-            Path::new("tests/swfs/avm2/shared_object/"),
+        Path::new("tests/swfs"),
-            Path::new("tests/swfs"),
+    )?
-        )?,
+    .run(
         |_player| Ok(()),
         |player| {
             // Save the storage backend for next run.
@@ -102,16 +100,16 @@ pub fn shared_object_avm2() -> Result<()> {
     );
 
     // Re-run the SWF, verifying that the shared object persists.
-    test_swf_with_hooks(
+    Test::from_options(
-        &Test::from_options(
+        TestOptions {
-            TestOptions {
+            num_frames: 1,
-                num_frames: 1,
+            output_path: "output2.txt".into(),
-                output_path: "output2.txt".into(),
+            ..Default::default()
-                ..Default::default()
+        },
-            },
+        Path::new("tests/swfs/avm2/shared_object/"),
-            Path::new("tests/swfs/avm2/shared_object/"),
+        Path::new("tests/swfs"),
-            Path::new("tests/swfs"),
+    )?
-        )?,
+    .run(
         |player| {
             // Swap in the previous storage backend.
             let mut player = player.lock().unwrap();

tests/tests/util/options.rs

@@ -85,7 +85,7 @@ pub struct PlayerOptions {
 }
 
 impl PlayerOptions {
-    pub fn setup(&self, player: Arc<Mutex<Player>>) {
+    pub fn setup(&self, player: &Arc<Mutex<Player>>) {
         if let Some(max_execution_duration) = self.max_execution_duration {
             player
                 .lock()

tests/tests/util/runner.rs

@@ -1,7 +1,5 @@
-use crate::assert_eq;
 use crate::util::test::Test;
 use anyhow::{anyhow, Result};
-use regex::Regex;
 use ruffle_core::backend::log::LogBackend;
 use ruffle_core::backend::navigator::{NullExecutor, NullNavigatorBackend};
 use ruffle_core::events::MouseButton as RuffleMouseButton;
@@ -41,8 +39,8 @@ impl LogBackend for TestLogBackend {
 /// Tests that the trace output matches the given expected output.
 pub fn run_swf(
     test: &Test,
-    before_start: impl FnOnce(Arc<Mutex<Player>>) -> Result<()>,
     mut injector: InputInjector,
+    before_start: impl FnOnce(Arc<Mutex<Player>>) -> Result<()>,
     before_end: impl FnOnce(Arc<Mutex<Player>>) -> Result<()>,
 ) -> Result<String> {
     let base_path = Path::new(&test.output_path).parent().unwrap();
@@ -100,6 +98,10 @@ pub fn run_swf(
         .with_movie(movie)
         .build();
 
+    if let Some(options) = &test.options.player_options {
+        options.setup(&player);
+    }
+
     before_start(player.clone())?;
 
     for _ in 0..test.options.num_frames {
@@ -207,120 +209,3 @@ pub fn run_swf(
     let trace = trace_output.borrow().join("\n");
     Ok(trace)
 }
-
-/// Loads an SWF and runs it through the Ruffle core for a number of frames.
-/// Tests that the trace output matches the given expected output.
-/// If a line has a floating point value, it will be compared approxinmately using the given epsilon.
-pub fn test_swf_approx(
-    test: &Test,
-    num_patterns: &[Regex],
-    approx_assert_fn: impl Fn(f64, f64),
-) -> Result<()> {
-    let injector =
-        InputInjector::from_file(&test.input_path).unwrap_or_else(|_| InputInjector::empty());
-    let trace_log = run_swf(&test, |_| Ok(()), injector, |_| Ok(()))?;
-    let mut expected_data = std::fs::read_to_string(&test.output_path)?;
-
-    // Strip a trailing newline if it has one.
-    if expected_data.ends_with('\n') {
-        expected_data = expected_data[0..expected_data.len() - "\n".len()].to_string();
-    }
-
-    std::assert_eq!(
-        trace_log.lines().count(),
-        expected_data.lines().count(),
-        "# of lines of output didn't match"
-    );
-
-    for (actual, expected) in trace_log.lines().zip(expected_data.lines()) {
-        // If these are numbers, compare using approx_eq.
-        if let (Ok(actual), Ok(expected)) = (actual.parse::<f64>(), expected.parse::<f64>()) {
-            // NaNs should be able to pass in an approx test.
-            if actual.is_nan() && expected.is_nan() {
-                continue;
-            }
-
-            // TODO: Lower this epsilon as the accuracy of the properties improves.
-            // if let Some(relative_epsilon) = relative_epsilon {
-            //     assert_relative_eq!(
-            //         actual,
-            //         expected,
-            //         epsilon = absolute_epsilon,
-            //         max_relative = relative_epsilon
-            //     );
-            // } else {
-            //     assert_abs_diff_eq!(actual, expected, epsilon = absolute_epsilon);
-            // }
-            approx_assert_fn(actual, expected);
-        } else {
-            let mut found = false;
-            // Check each of the user-provided regexes for a match
-            for pattern in num_patterns {
-                if let (Some(actual_captures), Some(expected_captures)) =
-                    (pattern.captures(actual), pattern.captures(expected))
-                {
-                    found = true;
-                    std::assert_eq!(
-                        actual_captures.len(),
-                        expected_captures.len(),
-                        "Differing numbers of regex captures"
-                    );
-
-                    // Each capture group (other than group 0, which is always the entire regex
-                    // match) represents a floating-point value
-                    for (actual_val, expected_val) in actual_captures
-                        .iter()
-                        .skip(1)
-                        .zip(expected_captures.iter().skip(1))
-                    {
-                        let actual_num = actual_val
-                            .expect("Missing capture gruop value for 'actual'")
-                            .as_str()
-                            .parse::<f64>()
-                            .expect("Failed to parse 'actual' capture group as float");
-                        let expected_num = expected_val
-                            .expect("Missing capture gruop value for 'expected'")
-                            .as_str()
-                            .parse::<f64>()
-                            .expect("Failed to parse 'expected' capture group as float");
-                        approx_assert_fn(actual_num, expected_num);
-                    }
-                    let modified_actual = pattern.replace(actual, "");
-                    let modified_expected = pattern.replace(expected, "");
-                    assert_eq!(modified_actual, modified_expected);
-                    break;
-                }
-            }
-            if !found {
-                assert_eq!(actual, expected);
-            }
-        }
-    }
-    Ok(())
-}
-
-/// Loads an SWF and runs it through the Ruffle core for a number of frames.
-/// Tests that the trace output matches the given expected output.
-#[allow(clippy::too_many_arguments)]
-pub fn test_swf_with_hooks(
-    test: &Test,
-    before_start: impl FnOnce(Arc<Mutex<Player>>) -> Result<()>,
-    before_end: impl FnOnce(Arc<Mutex<Player>>) -> Result<()>,
-) -> Result<()> {
-    let injector =
-        InputInjector::from_file(&test.input_path).unwrap_or_else(|_| InputInjector::empty());
-    let mut expected_output = std::fs::read_to_string(&test.output_path)?.replace("\r\n", "\n");
-
-    // Strip a trailing newline if it has one.
-    if expected_output.ends_with('\n') {
-        expected_output = expected_output[0..expected_output.len() - "\n".len()].to_string();
-    }
-
-    let trace_log = run_swf(&test, before_start, injector, before_end)?;
-    assert_eq!(
-        trace_log, expected_output,
-        "ruffle output != flash player output"
-    );
-
-    Ok(())
-}

tests/tests/util/test.rs

@@ -1,8 +1,12 @@
+use crate::assert_eq;
 use crate::set_logger;
 use crate::util::options::TestOptions;
-use crate::util::runner::{test_swf_approx, test_swf_with_hooks, RUN_IMG_TESTS};
+use crate::util::runner::{run_swf, RUN_IMG_TESTS};
 use anyhow::{Context, Result};
+use ruffle_core::Player;
+use ruffle_input_format::InputInjector;
 use std::path::{Path, PathBuf};
+use std::sync::{Arc, Mutex};
 
 pub struct Test {
     pub options: TestOptions,
@@ -41,29 +45,20 @@ impl Test {
         )
     }
 
-    pub fn run(self) -> Result<(), libtest_mimic::Failed> {
+    pub fn run(
+        self,
+        before_start: impl FnOnce(Arc<Mutex<Player>>) -> Result<()>,
+        before_end: impl FnOnce(Arc<Mutex<Player>>) -> Result<()>,
+    ) -> std::result::Result<(), libtest_mimic::Failed> {
         set_logger();
-
+        let injector = if self.input_path.is_file() {
-        if let Some(approximations) = &self.options.approximations {
+            InputInjector::from_file(&self.input_path)?
-            test_swf_approx(
-                &self,
-                &approximations.number_patterns(),
-                |actual, expected| approximations.compare(actual, expected),
-            )
-            .map_err(|e| e.to_string().into())
         } else {
-            test_swf_with_hooks(
+            InputInjector::empty()
-                &self,
+        };
-                |player| {
+        let output = run_swf(&self, injector, before_start, before_end)?;
-                    if let Some(player_options) = &self.options.player_options {
+        self.compare_output(&output)?;
-                        player_options.setup(player);
+        Ok(())
-                    }
-                    Ok(())
-                },
-                |_| Ok(()),
-            )
-            .map_err(|e| e.to_string().into())
-        }
     }
 
     pub fn should_run(&self) -> bool {
@@ -75,4 +70,86 @@ impl Test {
         }
         return true;
     }
+
+    pub fn compare_output(&self, actual_output: &str) -> Result<()> {
+        let mut expected_output = std::fs::read_to_string(&self.output_path)?.replace("\r\n", "\n");
+
+        // Strip a trailing newline if it has one.
+        if expected_output.ends_with('\n') {
+            expected_output = expected_output[0..expected_output.len() - "\n".len()].to_string();
+        }
+
+        if let Some(approximations) = &self.options.approximations {
+            std::assert_eq!(
+                actual_output.lines().count(),
+                expected_output.lines().count(),
+                "# of lines of output didn't match"
+            );
+
+            for (actual, expected) in actual_output.lines().zip(expected_output.lines()) {
+                // If these are numbers, compare using approx_eq.
+                if let (Ok(actual), Ok(expected)) = (actual.parse::<f64>(), expected.parse::<f64>())
+                {
+                    // NaNs should be able to pass in an approx test.
+                    if actual.is_nan() && expected.is_nan() {
+                        continue;
+                    }
+
+                    approximations.compare(actual, expected);
+                } else {
+                    let mut found = false;
+
+                    // Check each of the user-provided regexes for a match
+                    for pattern in approximations.number_patterns() {
+                        if let (Some(actual_captures), Some(expected_captures)) =
+                            (pattern.captures(actual), pattern.captures(expected))
+                        {
+                            found = true;
+                            std::assert_eq!(
+                                actual_captures.len(),
+                                expected_captures.len(),
+                                "Differing numbers of regex captures"
+                            );
+
+                            // Each capture group (other than group 0, which is always the entire regex
+                            // match) represents a floating-point value
+                            for (actual_val, expected_val) in actual_captures
+                                .iter()
+                                .skip(1)
+                                .zip(expected_captures.iter().skip(1))
+                            {
+                                let actual_num = actual_val
+                                    .expect("Missing capture group value for 'actual'")
+                                    .as_str()
+                                    .parse::<f64>()
+                                    .expect("Failed to parse 'actual' capture group as float");
+                                let expected_num = expected_val
+                                    .expect("Missing capture group value for 'expected'")
+                                    .as_str()
+                                    .parse::<f64>()
+                                    .expect("Failed to parse 'expected' capture group as float");
+                                approximations.compare(actual_num, expected_num);
+                            }
+                            let modified_actual = pattern.replace(actual, "");
+                            let modified_expected = pattern.replace(expected, "");
+
+                            assert_eq!(modified_actual, modified_expected);
+                            break;
+                        }
+                    }
+
+                    if !found {
+                        assert_eq!(actual, expected);
+                    }
+                }
+            }
+        } else {
+            assert_eq!(
+                actual_output, expected_output,
+                "ruffle output != flash player output"
+            );
+        }
+
+        Ok(())
+    }
 }