Bug 1610626 - Fix wrench reftest mode incorrectly printing unexpected in some cases. r=Bert
authorGlenn Watson <gw@intuitionlibrary.com>
Wed, 22 Jan 2020 02:22:42 +0000
changeset 511062 3d076ae68a9411e4f412c90d2743987f7b916850
parent 511061 5ed6fc2b4997c9e55f8a16c0ba6d50d4b37b3907
child 511063 ac263c8232859b6ae29160d1e501d906b6ed63ae
push id37044
push useropoprus@mozilla.com
push dateWed, 22 Jan 2020 09:48:59 +0000
treeherdermozilla-central@be3a05f615a5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersBert
bugs1610626
milestone74.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1610626 - Fix wrench reftest mode incorrectly printing unexpected in some cases. r=Bert The semantics of the inaccuracy reftest mode are that the overall test succeeds as long as one of the multiple test images mismatches the reference image. However, the previous implementation would print an unexpected result if any of the comparisons were equal (even though the overall test result would be reported correctly). This patch changes wrench so that it only reports an unexpected failure for the overall test, not each individual reference. Differential Revision: https://phabricator.services.mozilla.com/D60564
gfx/wr/wrench/src/reftest.rs
--- a/gfx/wr/wrench/src/reftest.rs
+++ b/gfx/wr/wrench/src/reftest.rs
@@ -144,31 +144,20 @@ impl Reftest {
                     false
                 } else {
                     true
                 }
             }
         }
     }
 
-    /// Check the negative case (expecting inequality) and report details if same
-    fn check_and_report_inequality_failure(
-        &self,
-        comparison: ReftestImageComparison,
-    ) -> bool {
-        match comparison {
-            ReftestImageComparison::Equal => {
-                println!("REFTEST TEST-UNEXPECTED-FAIL | {} | image comparison", self);
-                println!("REFTEST TEST-END | {}", self);
-                false
-            }
-            ReftestImageComparison::NotEqual { .. } => {
-                true
-            }
-        }
+    /// Report details of the negative case
+    fn report_unexpected_equality(&self) {
+        println!("REFTEST TEST-UNEXPECTED-FAIL | {} | image comparison", self);
+        println!("REFTEST TEST-END | {}", self);
     }
 }
 
 impl Display for Reftest {
     fn fmt(&self, f: &mut Formatter) -> Result<(), Error> {
         let paths: Vec<String> = self.test.iter().map(|t| t.display().to_string()).collect();
         write!(
             f,
@@ -699,17 +688,25 @@ impl<'a> ReftestHarness<'a> {
                     &test,
                     &reference,
                 )
             }
             ReftestOp::NotEqual => {
                 // Ensure that the final image *doesn't* match the reference
                 let test = images.pop().unwrap();
                 let comparison = test.compare(&reference);
-                t.check_and_report_inequality_failure(comparison)
+                match comparison {
+                    ReftestImageComparison::Equal => {
+                        t.report_unexpected_equality();
+                        false
+                    }
+                    ReftestImageComparison::NotEqual { .. } => {
+                        true
+                    }
+                }
             }
             ReftestOp::Accurate => {
                 // Ensure that *all* images match the reference
                 for test in images.drain(..) {
                     let comparison = test.compare(&reference);
 
                     if !t.check_and_report_equality_failure(
                         comparison,
@@ -719,24 +716,28 @@ impl<'a> ReftestHarness<'a> {
                         return false;
                     }
                 }
 
                 true
             }
             ReftestOp::Inaccurate => {
                 // Ensure that at least one of the images doesn't match the reference
-                let mut found_mismatch = false;
+                let all_same = images.iter().all(|image| {
+                    match image.compare(&reference) {
+                        ReftestImageComparison::Equal => true,
+                        ReftestImageComparison::NotEqual { .. } => false,
+                    }
+                });
 
-                for test in images.drain(..) {
-                    let comparison = test.compare(&reference);
-                    found_mismatch |= t.check_and_report_inequality_failure(comparison);
+                if all_same {
+                    t.report_unexpected_equality();
                 }
 
-                found_mismatch
+                !all_same
             }
         }
     }
 
     fn load_image(&mut self, filename: &Path, format: ImageFormat) -> ReftestImage {
         let file = BufReader::new(File::open(filename).unwrap());
         let img_raw = load_piston_image(file, format).unwrap();
         let img = img_raw.flipv().to_rgba();