GNOME.org
 

[librsvg/refactor-test-utils: 1/3] Refactor test_utils and move it to the tests


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

    Refactor test_utils and move it to the tests
    
    librsvg::test_utils lives in the tests crate now as reference_utils
    and features a nicer API.

 Makefile.am                           |   1 -
 src/lib.rs                            |   1 -
 src/surface_utils/compare_surfaces.rs |   3 +-
 src/test_utils.rs                     | 162 --------------------------------
 tests/Makefile.am                     |   1 +
 tests/src/api.rs                      |  25 +++--
 tests/src/bugs.rs                     |  52 ++++-------
 tests/src/intrinsic_dimensions.rs     |  53 ++++-------
 tests/src/main.rs                     |   3 +
 tests/src/primitives.rs               |  60 +++++-------
 tests/src/reference.rs                |   6 +-
 tests/src/reference_utils.rs          | 169 ++++++++++++++++++++++++++++++++++
 12 files changed, 245 insertions(+), 291 deletions(-)
---
diff --git a/Makefile.am b/Makefile.am
index 287a5c24..15c39266 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -100,7 +100,6 @@ LIBRSVG_SRC =                                       \
        src/surface_utils/mod.rs                \
        src/surface_utils/shared_surface.rs     \
        src/surface_utils/srgb.rs               \
-       src/test_utils.rs                       \
        src/text.rs                             \
        src/transform.rs                        \
        src/ua.css                              \
diff --git a/src/lib.rs b/src/lib.rs
index 53dc5c63..3ac4c261 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -130,7 +130,6 @@ mod space;
 mod structure;
 mod style;
 pub mod surface_utils;
-pub mod test_utils;
 mod text;
 mod transform;
 mod unit_interval;
diff --git a/src/surface_utils/compare_surfaces.rs b/src/surface_utils/compare_surfaces.rs
index 5f893858..47be0661 100644
--- a/src/surface_utils/compare_surfaces.rs
+++ b/src/surface_utils/compare_surfaces.rs
@@ -3,7 +3,6 @@ use super::{
     shared_surface::{SharedImageSurface, SurfaceType},
     ImageSurfaceDataExt, Pixel, PixelOps,
 };
-use crate::error::RenderingError;
 
 use rgb::{ComponentMap, RGB};
 
@@ -35,7 +34,7 @@ fn emphasize(p: &Pixel) -> Pixel {
 pub fn compare_surfaces(
     surf_a: &SharedImageSurface,
     surf_b: &SharedImageSurface,
-) -> Result<BufferDiff, RenderingError> {
+) -> Result<BufferDiff, cairo::Status> {
     let a_width = surf_a.width();
     let a_height = surf_a.height();
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index a74e5ae8..08499c7e 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -16,6 +16,7 @@ test_sources =                                \
        src/predicates/png.rs           \
        src/primitives.rs               \
        src/reference.rs                \
+       src/reference_utils.rs          \
        src/render_crash.rs             \
        src/utils.rs                    \
        $(NULL)
diff --git a/tests/src/api.rs b/tests/src/api.rs
index 764492b8..1ea0b3c6 100644
--- a/tests/src/api.rs
+++ b/tests/src/api.rs
@@ -1,11 +1,8 @@
 use cairo;
+use librsvg::surface_utils::shared_surface::{SharedImageSurface, SurfaceType};
 use librsvg::{CairoRenderer, RenderingError};
 
-use librsvg::{
-    surface_utils::shared_surface::{SharedImageSurface, SurfaceType},
-    test_utils::compare_to_surface,
-};
-
+use crate::reference_utils::{Compare, Evaluate, Reference};
 use crate::utils::load_svg;
 
 #[test]
@@ -82,9 +79,9 @@ 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)
+        .compare(&output_surf)
+        .evaluate(&output_surf, "renderer_layer");
 }
 
 #[test]
@@ -168,9 +165,9 @@ 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)
+        .compare(&output_surf)
+        .evaluate(&output_surf, "untransformed_element");
 }
 
 #[test]
@@ -218,7 +215,7 @@ 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)
+        .compare(&output_surf)
+        .evaluate(&output_surf, "set_stylesheet");
 }
diff --git a/tests/src/bugs.rs b/tests/src/bugs.rs
index 2f45f39b..c94588ea 100644
--- a/tests/src/bugs.rs
+++ b/tests/src/bugs.rs
@@ -1,11 +1,8 @@
 use cairo;
-use librsvg::{
-    surface_utils::shared_surface::{SharedImageSurface, SurfaceType},
-    test_utils::compare_to_surface,
-    LoadingError, SvgHandle,
-};
+use librsvg::{LoadingError, SvgHandle};
 use matches::matches;
 
+use crate::reference_utils::{Compare, Evaluate, Reference};
 use crate::utils::{load_svg, render_document, SurfaceSize};
 
 // https://gitlab.gnome.org/GNOME/librsvg/issues/335
@@ -76,13 +73,9 @@ 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)
+        .compare(&output_surf)
+        .evaluate(&output_surf, "nonexistent_image_shouldnt_cancel_rendering");
 }
 
 // https://gitlab.gnome.org/GNOME/librsvg/-/issues/568
@@ -129,13 +122,9 @@ 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)
+        .compare(&output_surf)
+        .evaluate(&output_surf, "href_attribute_overrides_xlink_href");
 }
 
 // https://gitlab.gnome.org/GNOME/librsvg/-/issues/560
@@ -174,13 +163,9 @@ 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)
+        .compare(&output_surf)
+        .evaluate(&output_surf, "nonexistent_filter_leaves_object_unfiltered");
 }
 
 // https://www.w3.org/TR/SVG2/painting.html#SpecifyingPaint says this:
@@ -243,13 +228,9 @@ 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)
+        .compare(&output_surf)
+        .evaluate(&output_surf, "recursive_paint_servers_fallback_to_color");
 }
 
 fn test_renders_as_empty(svg: &SvgHandle, test_name: &str) {
@@ -267,9 +248,10 @@ 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)
+        .compare(&output_surf)
+        .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..9b6298b8 100644
--- a/tests/src/intrinsic_dimensions.rs
+++ b/tests/src/intrinsic_dimensions.rs
@@ -1,11 +1,8 @@
 use cairo;
 
-use librsvg::{
-    surface_utils::shared_surface::{SharedImageSurface, SurfaceType},
-    test_utils::compare_to_surface,
-    CairoRenderer, IntrinsicDimensions, Length, LengthUnit, RenderingError,
-};
+use librsvg::{CairoRenderer, IntrinsicDimensions, Length, LengthUnit, RenderingError};
 
+use crate::reference_utils::{Compare, Evaluate, Reference};
 use crate::utils::{load_svg, render_document, SurfaceSize};
 
 #[test]
@@ -461,13 +458,9 @@ 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)
+        .compare(&output_surf)
+        .evaluate(&output_surf, "render_to_viewport_with_different_size");
 }
 
 #[test]
@@ -506,9 +499,9 @@ 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)
+        .compare(&output_surf)
+        .evaluate(&output_surf, "render_to_offsetted_viewport");
 }
 
 #[test]
@@ -550,13 +543,9 @@ 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)
+        .compare(&output_surf)
+        .evaluate(&output_surf, "render_to_viewport_with_transform");
 }
 
 #[test]
@@ -620,13 +609,9 @@ 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)
+        .compare(&output_surf)
+        .evaluate(&output_surf, "clip_on_transformed_viewport");
 }
 
 #[test]
@@ -690,11 +675,7 @@ 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)
+        .compare(&output_surf)
+        .evaluate(&output_surf, "mask_on_transformed_viewport");
 }
diff --git a/tests/src/main.rs b/tests/src/main.rs
index adfb52f2..f56696ad 100644
--- a/tests/src/main.rs
+++ b/tests/src/main.rs
@@ -29,6 +29,9 @@ mod primitives;
 #[cfg(test)]
 mod reference;
 
+#[cfg(test)]
+mod reference_utils;
+
 #[cfg(test)]
 mod render_crash;
 
diff --git a/tests/src/primitives.rs b/tests/src/primitives.rs
index acc682f2..ed5f9d91 100644
--- a/tests/src/primitives.rs
+++ b/tests/src/primitives.rs
@@ -1,10 +1,6 @@
 use cairo;
 
-use librsvg::{
-    surface_utils::shared_surface::{SharedImageSurface, SurfaceType},
-    test_utils::compare_to_surface,
-};
-
+use crate::reference_utils::{Compare, Evaluate, Reference};
 use crate::utils::{load_svg, render_document, SurfaceSize};
 
 #[test]
@@ -44,13 +40,9 @@ 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)
+        .compare(&output_surf)
+        .evaluate(&output_surf, "simple_opacity_with_transform");
 }
 
 #[test]
@@ -90,13 +82,9 @@ 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)
+        .compare(&output_surf)
+        .evaluate(&output_surf, "simple_opacity_with_offset_viewport");
 }
 
 #[test]
@@ -141,9 +129,9 @@ 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)
+        .compare(&output_surf)
+        .evaluate(&output_surf, "simple_opacity_with_scale");
 }
 
 #[test]
@@ -200,9 +188,9 @@ 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)
+        .compare(&output_surf)
+        .evaluate(&output_surf, "markers_with_scale");
 }
 
 #[test]
@@ -242,13 +230,9 @@ 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)
+        .compare(&output_surf)
+        .evaluate(&output_surf, "opacity_inside_transformed_group");
 }
 
 #[test]
@@ -303,9 +287,9 @@ 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)
+        .compare(&output_surf)
+        .evaluate(&output_surf, "compound_opacity");
 }
 
 #[test]
@@ -370,7 +354,7 @@ 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)
+        .compare(&output_surf)
+        .evaluate(&output_surf, "nested_masks");
 }
diff --git a/tests/src/reference.rs b/tests/src/reference.rs
index 5dade027..5237911e 100644
--- a/tests/src/reference.rs
+++ b/tests/src/reference.rs
@@ -12,11 +12,11 @@ use test_generator::test_resources;
 use cairo;
 use librsvg::{
     surface_utils::shared_surface::{SharedImageSurface, SurfaceType},
-    test_utils::compare_to_file,
     CairoRenderer, IntrinsicDimensions, Length, Loader,
 };
 use std::path::PathBuf;
 
+use crate::reference_utils::{Compare, Evaluate, Reference};
 use crate::utils::{setup_font_map, setup_language};
 
 // The original reference images from the SVG1.1 test suite are at 72 DPI.
@@ -83,7 +83,9 @@ 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)
+        .compare(&output_surf)
+        .evaluate(&output_surf, &path_base_name);
 }
 
 /// Turns `/foo/bar/baz.svg` into `/foo/bar/baz-ref.svg`.
diff --git a/tests/src/reference_utils.rs b/tests/src/reference_utils.rs
new file mode 100644
index 00000000..f059186c
--- /dev/null
+++ b/tests/src/reference_utils.rs
@@ -0,0 +1,169 @@
+//! Utilities for the reference image test suite.
+//!
+//! This module has utility functions that are used in the test suite
+//! to compare rendered surfaces to reference images.
+
+use std::convert::TryFrom;
+use std::env;
+use std::fs::{self, File};
+use std::io::BufReader;
+use std::path::{Path, PathBuf};
+use std::sync::Once;
+
+use librsvg::surface_utils::compare_surfaces::{compare_surfaces, BufferDiff, Diff};
+use librsvg::surface_utils::shared_surface::{SharedImageSurface, SurfaceType};
+
+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();
+        }
+
+        Self::from_surface(argb)
+    }
+
+    pub fn from_surface(surface: cairo::ImageSurface) -> Result<Self, cairo::IoError> {
+        let shared = SharedImageSurface::wrap(surface, SurfaceType::SRgb)?;
+        Ok(Self(shared))
+    }
+}
+
+pub trait Compare {
+    fn compare(self, surface: &SharedImageSurface) -> Result<BufferDiff, cairo::IoError>;
+}
+
+impl Compare for Reference {
+    fn compare(self, surface: &SharedImageSurface) -> Result<BufferDiff, cairo::IoError> {
+        compare_surfaces(&self.0, surface).map_err(cairo::IoError::from)
+    }
+}
+
+impl Compare for Result<Reference, cairo::IoError> {
+    fn compare(self, surface: &SharedImageSurface) -> Result<BufferDiff, cairo::IoError> {
+        self.map(|reference| reference.compare(surface))
+            .and_then(std::convert::identity)
+    }
+}
+
+pub trait Evaluate {
+    fn evaluate(&self, output_surface: &SharedImageSurface, output_base_name: &str);
+}
+
+impl Evaluate 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");
+                    }
+                }
+            }
+        }
+    }
+}
+
+impl Evaluate for Result<BufferDiff, cairo::IoError> {
+    fn evaluate(&self, output_surface: &SharedImageSurface, output_base_name: &str) {
+        self.as_ref()
+            .map(|diff| diff.evaluate(output_surface, output_base_name))
+            .unwrap();
+    }
+}
+
+fn write_to_file(input: &SharedImageSurface, output_base_name: &str, suffix: &str) {
+    let path = output_dir().join(&format!("{}-{}.png", output_base_name, suffix));
+    println!("{}: {}", suffix, path.to_string_lossy());
+    let mut output_file = File::create(path).unwrap();
+    input
+        .clone()
+        .into_image_surface()
+        .unwrap()
+        .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
+}
+
+fn tolerable_difference() -> u8 {
+    static mut TOLERANCE: u8 = 2;
+
+    static ONCE: Once = Once::new();
+    ONCE.call_once(|| unsafe {
+        if let Ok(str) = env::var("RSVG_TEST_TOLERANCE") {
+            let value: usize = str
+                .parse()
+                .expect("Can not parse RSVG_TEST_TOLERANCE as a number");
+            TOLERANCE =
+                u8::try_from(value).expect("RSVG_TEST_TOLERANCE should be between 0 and 255");
+        }
+    });
+
+    unsafe { TOLERANCE }
+}
+
+trait Deviation {
+    fn distinguishable(&self) -> bool;
+    fn inacceptable(&self) -> bool;
+}
+
+impl Deviation for Diff {
+    fn distinguishable(&self) -> bool {
+        self.max_diff > 2
+    }
+
+    fn inacceptable(&self) -> bool {
+        self.max_diff > tolerable_difference()
+    }
+}
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]