GNOME.org
 

[librsvg/refactor-test-utils: 2/2] Refactor test_utils


commit 969de9571e2185017e1d22f14b3857ab21c9c140
Author: Sven Neumann <sven svenfoo org>
Date:   Mon Jan 25 17:58:30 2021 +0100

    Refactor test_utils

 src/test_utils.rs                 | 169 ++++++++++++++++++--------------------
 tests/src/api.rs                  |  26 +++---
 tests/src/bugs.rs                 |  58 ++++++-------
 tests/src/intrinsic_dimensions.rs |  59 ++++++-------
 tests/src/primitives.rs           |  73 ++++++++--------
 tests/src/reference.rs            |   8 +-
 6 files changed, 188 insertions(+), 205 deletions(-)
---
diff --git a/src/test_utils.rs b/src/test_utils.rs
index 2890fc8e..cd5fd62f 100644
--- a/src/test_utils.rs
+++ b/src/test_utils.rs
@@ -3,6 +3,7 @@
 //! This module has utility functions that are used in the test suite
 //! to compare rendered surfaces to reference images.
 
+use crate::error::RenderingError;
 use crate::surface_utils::compare_surfaces::{compare_surfaces, BufferDiff, Diff};
 use crate::surface_utils::shared_surface::{SharedImageSurface, SurfaceType};
 
@@ -10,7 +11,7 @@ use std::convert::TryFrom;
 use std::env;
 use std::fs::{self, File};
 use std::io::BufReader;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use std::sync::Once;
 
 fn tolerable_difference() -> u8 {
@@ -45,104 +46,68 @@ impl Deviation for Diff {
     }
 }
 
-/// Creates a directory for test output and returns its path.
-///
-/// The location for the output directory is taken from the `OUT_DIR` environment
-/// variable if that is set. Otherwise std::env::temp_dir() will be used, which is
-/// a platform dependent location for temporary files.
-///
-/// # Panics
-///
-/// Will panic if the output directory can not be created.
-pub fn output_dir() -> PathBuf {
-    let tempdir = || {
-        let mut path = env::temp_dir();
-        path.push("rsvg-test-output");
-        path
-    };
-    let path = env::var_os("OUT_DIR").map_or_else(tempdir, PathBuf::from);
-
-    fs::create_dir_all(&path).expect("could not create output directory for tests");
-
-    path
-}
-
-// FIXME: proper errors?
-fn load_png_as_argb(path: &PathBuf) -> Result<cairo::ImageSurface, ()> {
-    let file = File::open(path).map_err(|_| ())?;
-
-    let mut reference_file = BufReader::new(file);
-
-    let png = cairo::ImageSurface::create_from_png(&mut reference_file).map_err(|_| ())?;
-    let argb =
-        cairo::ImageSurface::create(cairo::Format::ARgb32, png.get_width(), png.get_height())
-            .map_err(|_| ())?;
+pub struct Reference(SharedImageSurface);
+
+impl Reference {
+    pub fn from_png<P: AsRef<Path>>(path: P) -> Result<Self, cairo::IoError> {
+        let file = File::open(path).map_err(|e| cairo::IoError::Io(e))?;
+        let mut reader = BufReader::new(file);
+        let png = cairo::ImageSurface::create_from_png(&mut reader)?;
+        let argb =
+            cairo::ImageSurface::create(cairo::Format::ARgb32, png.get_width(), png.get_height())?;
+        {
+            // convert to ARGB; the PNG may come as Rgb24
+            let cr = cairo::Context::new(&argb);
+            cr.set_source_surface(&png, 0.0, 0.0);
+            cr.paint();
+        }
 
-    {
-        // convert to ARGB; the PNG may come as Rgb24
-        let cr = cairo::Context::new(&argb);
-        cr.set_source_surface(&png, 0.0, 0.0);
-        cr.paint();
+        Self::from_surface(argb)
     }
 
-    Ok(argb)
-}
+    pub fn from_surface(surface: cairo::ImageSurface) -> Result<Self, cairo::IoError> {
+        let shared = SharedImageSurface::wrap(surface, SurfaceType::SRgb)?;
+        Ok(Self(shared))
+    }
 
-/// Compares `output_surf` to the reference image from `reference_path`.
-///
-/// Loads the image stored at `reference_path` and uses `compare_to_surface` to
-/// do the comparison.  See that function for details.
-///
-/// # Panics
-///
-/// See `compare_to_surface` for information; this function compares the images and panics in the
-/// same way as that function upon encountering differences.
-pub fn compare_to_file(
-    output_surf: &SharedImageSurface,
-    output_base_name: &str,
-    reference_path: &PathBuf,
-) {
-    let png = load_png_as_argb(reference_path).unwrap();
-    let reference_surf = SharedImageSurface::wrap(png, SurfaceType::SRgb).unwrap();
-
-    compare_to_surface(output_surf, &reference_surf, output_base_name);
+    pub fn compare_to(&self, surface: &SharedImageSurface) -> Result<BufferDiff, RenderingError> {
+        compare_surfaces(&self.0, surface)
+    }
 }
 
-/// Compares two surfaces and panics if they are too different.
-///
-/// The `output_base_name` is used to write test results if the
-/// surfaces are different.  If this is `foo`, this will write
-/// `foo-out.png` with the `output_surf` and `foo-diff.png` with a
-/// visual diff between `output_surf` and `reference_surf`.
-///
-/// # Panics
-///
-/// Will panic if the surfaces are too different to be acceptable.
-pub fn compare_to_surface(
-    output_surf: &SharedImageSurface,
-    reference_surf: &SharedImageSurface,
-    output_base_name: &str,
-) {
-    let diff = compare_surfaces(output_surf, reference_surf).unwrap();
-    evaluate_diff(&diff, output_surf, output_base_name);
+pub trait Evaluation {
+    fn evaluate(&self, output_surface: &SharedImageSurface, output_base_name: &str);
 }
 
-fn evaluate_diff(diff: &BufferDiff, output_surf: &SharedImageSurface, output_base_name: &str) {
-    match diff {
-        BufferDiff::DifferentSizes => unreachable!("surfaces should be of the same size"),
-
-        BufferDiff::Diff(diff) => {
-            if diff.distinguishable() {
-                println!(
-                    "{}: {} pixels changed with maximum difference of {}",
-                    output_base_name, diff.num_pixels_changed, diff.max_diff,
-                );
-
-                write_to_file(output_surf, output_base_name, "out");
-                write_to_file(&diff.surface, output_base_name, "diff");
-
-                if diff.inacceptable() {
-                    panic!("surfaces are too different");
+impl Evaluation for BufferDiff {
+    /// Evaluates a BufferDiff and panics if there are relevant differences
+    ///
+    /// The `output_base_name` is used to write test results if the
+    /// surfaces are different.  If this is `foo`, this will write
+    /// `foo-out.png` with the `output_surf` and `foo-diff.png` with a
+    /// visual diff between `output_surf` and the `Reference` that this
+    /// diff was created from.
+    ///
+    /// # Panics
+    ///
+    /// Will panic if the surfaces are too different to be acceptable.
+    fn evaluate(&self, output_surf: &SharedImageSurface, output_base_name: &str) {
+        match self {
+            BufferDiff::DifferentSizes => unreachable!("surfaces should be of the same size"),
+
+            BufferDiff::Diff(diff) => {
+                if diff.distinguishable() {
+                    println!(
+                        "{}: {} pixels changed with maximum difference of {}",
+                        output_base_name, diff.num_pixels_changed, diff.max_diff,
+                    );
+
+                    write_to_file(output_surf, output_base_name, "out");
+                    write_to_file(&diff.surface, output_base_name, "diff");
+
+                    if diff.inacceptable() {
+                        panic!("surfaces are too different");
+                    }
                 }
             }
         }
@@ -160,3 +125,25 @@ fn write_to_file(input: &SharedImageSurface, output_base_name: &str, suffix: &st
         .write_to_png(&mut output_file)
         .unwrap();
 }
+
+/// Creates a directory for test output and returns its path.
+///
+/// The location for the output directory is taken from the `OUT_DIR` environment
+/// variable if that is set. Otherwise std::env::temp_dir() will be used, which is
+/// a platform dependent location for temporary files.
+///
+/// # Panics
+///
+/// Will panic if the output directory can not be created.
+pub fn output_dir() -> PathBuf {
+    let tempdir = || {
+        let mut path = env::temp_dir();
+        path.push("rsvg-test-output");
+        path
+    };
+    let path = env::var_os("OUT_DIR").map_or_else(tempdir, PathBuf::from);
+
+    fs::create_dir_all(&path).expect("could not create output directory for tests");
+
+    path
+}
diff --git a/tests/src/api.rs b/tests/src/api.rs
index 764492b8..adb7fcb4 100644
--- a/tests/src/api.rs
+++ b/tests/src/api.rs
@@ -3,7 +3,7 @@ use librsvg::{CairoRenderer, RenderingError};
 
 use librsvg::{
     surface_utils::shared_surface::{SharedImageSurface, SurfaceType},
-    test_utils::compare_to_surface,
+    test_utils::{Evaluation, Reference},
 };
 
 use crate::utils::load_svg;
@@ -82,9 +82,11 @@ fn render_layer() {
         cr.fill();
     }
 
-    let reference_surf = SharedImageSurface::wrap(reference_surf, SurfaceType::SRgb).unwrap();
-
-    compare_to_surface(&output_surf, &reference_surf, "render_layer");
+    Reference::from_surface(reference_surf)
+        .unwrap()
+        .compare_to(&output_surf)
+        .unwrap()
+        .evaluate(&output_surf, "renderer_layer");
 }
 
 #[test]
@@ -168,9 +170,11 @@ fn untransformed_element() {
         cr.stroke();
     }
 
-    let reference_surf = SharedImageSurface::wrap(reference_surf, SurfaceType::SRgb).unwrap();
-
-    compare_to_surface(&output_surf, &reference_surf, "untransformed_element");
+    Reference::from_surface(reference_surf)
+        .unwrap()
+        .compare_to(&output_surf)
+        .unwrap()
+        .evaluate(&output_surf, "untransformed_element");
 }
 
 #[test]
@@ -218,7 +222,9 @@ fn set_stylesheet() {
         cr.fill();
     }
 
-    let reference_surf = SharedImageSurface::wrap(reference_surf, SurfaceType::SRgb).unwrap();
-
-    compare_to_surface(&output_surf, &reference_surf, "set_stylesheet");
+    Reference::from_surface(reference_surf)
+        .unwrap()
+        .compare_to(&output_surf)
+        .unwrap()
+        .evaluate(&output_surf, "set_stylesheet");
 }
diff --git a/tests/src/bugs.rs b/tests/src/bugs.rs
index 2f45f39b..522e415f 100644
--- a/tests/src/bugs.rs
+++ b/tests/src/bugs.rs
@@ -1,7 +1,6 @@
 use cairo;
 use librsvg::{
-    surface_utils::shared_surface::{SharedImageSurface, SurfaceType},
-    test_utils::compare_to_surface,
+    test_utils::{Evaluation, Reference},
     LoadingError, SvgHandle,
 };
 use matches::matches;
@@ -76,13 +75,11 @@ fn nonexistent_image_shouldnt_cancel_rendering() {
         cr.fill();
     }
 
-    let reference_surf = SharedImageSurface::wrap(reference_surf, SurfaceType::SRgb).unwrap();
-
-    compare_to_surface(
-        &output_surf,
-        &reference_surf,
-        "nonexistent_image_shouldnt_cancel_rendering",
-    );
+    Reference::from_surface(reference_surf)
+        .unwrap()
+        .compare_to(&output_surf)
+        .unwrap()
+        .evaluate(&output_surf, "nonexistent_image_shouldnt_cancel_rendering");
 }
 
 // https://gitlab.gnome.org/GNOME/librsvg/-/issues/568
@@ -129,13 +126,11 @@ fn href_attribute_overrides_xlink_href() {
         cr.fill();
     }
 
-    let reference_surf = SharedImageSurface::wrap(reference_surf, SurfaceType::SRgb).unwrap();
-
-    compare_to_surface(
-        &output_surf,
-        &reference_surf,
-        "href_attribute_overrides_xlink_href",
-    );
+    Reference::from_surface(reference_surf)
+        .unwrap()
+        .compare_to(&output_surf)
+        .unwrap()
+        .evaluate(&output_surf, "href_attribute_overrides_xlink_href");
 }
 
 // https://gitlab.gnome.org/GNOME/librsvg/-/issues/560
@@ -174,13 +169,11 @@ fn nonexistent_filter_leaves_object_unfiltered() {
         cr.fill();
     }
 
-    let reference_surf = SharedImageSurface::wrap(reference_surf, SurfaceType::SRgb).unwrap();
-
-    compare_to_surface(
-        &output_surf,
-        &reference_surf,
-        "nonexistent_filter_leaves_object_unfiltered",
-    );
+    Reference::from_surface(reference_surf)
+        .unwrap()
+        .compare_to(&output_surf)
+        .unwrap()
+        .evaluate(&output_surf, "nonexistent_filter_leaves_object_unfiltered");
 }
 
 // https://www.w3.org/TR/SVG2/painting.html#SpecifyingPaint says this:
@@ -243,13 +236,11 @@ fn recursive_paint_servers_fallback_to_color() {
         cr.fill();
     }
 
-    let reference_surf = SharedImageSurface::wrap(reference_surf, SurfaceType::SRgb).unwrap();
-
-    compare_to_surface(
-        &output_surf,
-        &reference_surf,
-        "recursive_paint_servers_fallback_to_color",
-    );
+    Reference::from_surface(reference_surf)
+        .unwrap()
+        .compare_to(&output_surf)
+        .unwrap()
+        .evaluate(&output_surf, "recursive_paint_servers_fallback_to_color");
 }
 
 fn test_renders_as_empty(svg: &SvgHandle, test_name: &str) {
@@ -267,9 +258,12 @@ fn test_renders_as_empty(svg: &SvgHandle, test_name: &str) {
     .unwrap();
 
     let reference_surf = cairo::ImageSurface::create(cairo::Format::ARgb32, 100, 100).unwrap();
-    let reference_surf = SharedImageSurface::wrap(reference_surf, SurfaceType::SRgb).unwrap();
 
-    compare_to_surface(&output_surf, &reference_surf, test_name);
+    Reference::from_surface(reference_surf)
+        .unwrap()
+        .compare_to(&output_surf)
+        .unwrap()
+        .evaluate(&output_surf, test_name);
 }
 
 // https://gitlab.gnome.org/GNOME/librsvg/-/issues/308
diff --git a/tests/src/intrinsic_dimensions.rs b/tests/src/intrinsic_dimensions.rs
index 68bbf0c6..55934685 100644
--- a/tests/src/intrinsic_dimensions.rs
+++ b/tests/src/intrinsic_dimensions.rs
@@ -1,8 +1,7 @@
 use cairo;
 
 use librsvg::{
-    surface_utils::shared_surface::{SharedImageSurface, SurfaceType},
-    test_utils::compare_to_surface,
+    test_utils::{Evaluation, Reference},
     CairoRenderer, IntrinsicDimensions, Length, LengthUnit, RenderingError,
 };
 
@@ -461,13 +460,11 @@ fn render_to_viewport_with_different_size() {
         cr.fill();
     }
 
-    let reference_surf = SharedImageSurface::wrap(reference_surf, SurfaceType::SRgb).unwrap();
-
-    compare_to_surface(
-        &output_surf,
-        &reference_surf,
-        "render_to_viewport_with_different_size",
-    );
+    Reference::from_surface(reference_surf)
+        .unwrap()
+        .compare_to(&output_surf)
+        .unwrap()
+        .evaluate(&output_surf, "render_to_viewport_with_different_size");
 }
 
 #[test]
@@ -506,9 +503,11 @@ fn render_to_offsetted_viewport() {
         cr.fill();
     }
 
-    let reference_surf = SharedImageSurface::wrap(reference_surf, SurfaceType::SRgb).unwrap();
-
-    compare_to_surface(&output_surf, &reference_surf, "render_to_offseted_viewport");
+    Reference::from_surface(reference_surf)
+        .unwrap()
+        .compare_to(&output_surf)
+        .unwrap()
+        .evaluate(&output_surf, "render_to_offsetted_viewport");
 }
 
 #[test]
@@ -550,13 +549,11 @@ fn render_to_viewport_with_transform() {
         cr.fill();
     }
 
-    let reference_surf = SharedImageSurface::wrap(reference_surf, SurfaceType::SRgb).unwrap();
-
-    compare_to_surface(
-        &output_surf,
-        &reference_surf,
-        "render_to_viewport_with_transform",
-    );
+    Reference::from_surface(reference_surf)
+        .unwrap()
+        .compare_to(&output_surf)
+        .unwrap()
+        .evaluate(&output_surf, "render_to_viewport_with_transform");
 }
 
 #[test]
@@ -620,13 +617,11 @@ fn clip_on_transformed_viewport() {
         cr.paint();
     }
 
-    let reference_surf = SharedImageSurface::wrap(reference_surf, SurfaceType::SRgb).unwrap();
-
-    compare_to_surface(
-        &output_surf,
-        &reference_surf,
-        "clip_on_transformed_viewport",
-    );
+    Reference::from_surface(reference_surf)
+        .unwrap()
+        .compare_to(&output_surf)
+        .unwrap()
+        .evaluate(&output_surf, "clip_on_transformed_viewport");
 }
 
 #[test]
@@ -690,11 +685,9 @@ fn mask_on_transformed_viewport() {
         cr.paint();
     }
 
-    let reference_surf = SharedImageSurface::wrap(reference_surf, SurfaceType::SRgb).unwrap();
-
-    compare_to_surface(
-        &output_surf,
-        &reference_surf,
-        "mask_on_transformed_viewport",
-    );
+    Reference::from_surface(reference_surf)
+        .unwrap()
+        .compare_to(&output_surf)
+        .unwrap()
+        .evaluate(&output_surf, "mask_on_transformed_viewport");
 }
diff --git a/tests/src/primitives.rs b/tests/src/primitives.rs
index acc682f2..088dff90 100644
--- a/tests/src/primitives.rs
+++ b/tests/src/primitives.rs
@@ -1,9 +1,6 @@
 use cairo;
 
-use librsvg::{
-    surface_utils::shared_surface::{SharedImageSurface, SurfaceType},
-    test_utils::compare_to_surface,
-};
+use librsvg::test_utils::{Evaluation, Reference};
 
 use crate::utils::{load_svg, render_document, SurfaceSize};
 
@@ -44,13 +41,11 @@ fn simple_opacity_with_transform() {
         cr.fill();
     }
 
-    let reference_surf = SharedImageSurface::wrap(reference_surf, SurfaceType::SRgb).unwrap();
-
-    compare_to_surface(
-        &output_surf,
-        &reference_surf,
-        "simple_opacity_with_transform",
-    );
+    Reference::from_surface(reference_surf)
+        .unwrap()
+        .compare_to(&output_surf)
+        .unwrap()
+        .evaluate(&output_surf, "simple_opacity_with_transform");
 }
 
 #[test]
@@ -90,13 +85,11 @@ fn simple_opacity_with_offset_viewport() {
         cr.fill();
     }
 
-    let reference_surf = SharedImageSurface::wrap(reference_surf, SurfaceType::SRgb).unwrap();
-
-    compare_to_surface(
-        &output_surf,
-        &reference_surf,
-        "simple_opacity_with_offset_viewport",
-    );
+    Reference::from_surface(reference_surf)
+        .unwrap()
+        .compare_to(&output_surf)
+        .unwrap()
+        .evaluate(&output_surf, "simple_opacity_with_offset_viewport");
 }
 
 #[test]
@@ -141,9 +134,11 @@ fn simple_opacity_with_scale() {
         cr.fill();
     }
 
-    let reference_surf = SharedImageSurface::wrap(reference_surf, SurfaceType::SRgb).unwrap();
-
-    compare_to_surface(&output_surf, &reference_surf, "simple_opacity_with_scale");
+    Reference::from_surface(reference_surf)
+        .unwrap()
+        .compare_to(&output_surf)
+        .unwrap()
+        .evaluate(&output_surf, "simple_opacity_with_scale");
 }
 
 #[test]
@@ -200,9 +195,11 @@ fn markers_with_scale() {
         }
     }
 
-    let reference_surf = SharedImageSurface::wrap(reference_surf, SurfaceType::SRgb).unwrap();
-
-    compare_to_surface(&output_surf, &reference_surf, "markers_with_scale");
+    Reference::from_surface(reference_surf)
+        .unwrap()
+        .compare_to(&output_surf)
+        .unwrap()
+        .evaluate(&output_surf, "markers_with_scale");
 }
 
 #[test]
@@ -242,13 +239,11 @@ fn opacity_inside_transformed_group() {
         cr.fill();
     }
 
-    let reference_surf = SharedImageSurface::wrap(reference_surf, SurfaceType::SRgb).unwrap();
-
-    compare_to_surface(
-        &output_surf,
-        &reference_surf,
-        "opacity_inside_transformed_group",
-    );
+    Reference::from_surface(reference_surf)
+        .unwrap()
+        .compare_to(&output_surf)
+        .unwrap()
+        .evaluate(&output_surf, "opacity_inside_transformed_group");
 }
 
 #[test]
@@ -303,9 +298,11 @@ fn compound_opacity() {
         cr.paint_with_alpha(0.5);
     }
 
-    let reference_surf = SharedImageSurface::wrap(reference_surf, SurfaceType::SRgb).unwrap();
-
-    compare_to_surface(&output_surf, &reference_surf, "compound_opacity");
+    Reference::from_surface(reference_surf)
+        .unwrap()
+        .compare_to(&output_surf)
+        .unwrap()
+        .evaluate(&output_surf, "compound_opacity");
 }
 
 #[test]
@@ -370,7 +367,9 @@ fn nested_masks() {
         cr.fill();
     }
 
-    let reference_surf = SharedImageSurface::wrap(reference_surf, SurfaceType::SRgb).unwrap();
-
-    compare_to_surface(&output_surf, &reference_surf, "nested_masks");
+    Reference::from_surface(reference_surf)
+        .unwrap()
+        .compare_to(&output_surf)
+        .unwrap()
+        .evaluate(&output_surf, "nested_masks");
 }
diff --git a/tests/src/reference.rs b/tests/src/reference.rs
index 5dade027..a0fda2ab 100644
--- a/tests/src/reference.rs
+++ b/tests/src/reference.rs
@@ -12,7 +12,7 @@ use test_generator::test_resources;
 use cairo;
 use librsvg::{
     surface_utils::shared_surface::{SharedImageSurface, SurfaceType},
-    test_utils::compare_to_file,
+    test_utils::{Evaluation, Reference},
     CairoRenderer, IntrinsicDimensions, Length, Loader,
 };
 use std::path::PathBuf;
@@ -83,7 +83,11 @@ fn reference_test(path: &str) {
 
     let output_surf = SharedImageSurface::wrap(surface, SurfaceType::SRgb).unwrap();
 
-    compare_to_file(&output_surf, &path_base_name, &reference);
+    Reference::from_png(&reference)
+        .unwrap()
+        .compare_to(&output_surf)
+        .unwrap()
+        .evaluate(&output_surf, &path_base_name);
 }
 
 /// Turns `/foo/bar/baz.svg` into `/foo/bar/baz-ref.svg`.
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]