[cargo-compare] fix proptests

Whoops, we weren't checking if the test actually failed.

tools/cargo-compare/Cargo.toml

@@ -15,10 +15,10 @@ either = "1.5.3"
 itertools = "0.9.0"
 guppy = { path = "../../guppy" }
 guppy-cmdlib = { path = "../../guppy-cmdlib" }
+once_cell = "1.3.1"
 structopt = "0.3"
 tempfile = "3.1.0"
 
 [dev-dependencies]
 guppy-cmdlib = { path = "../../guppy-cmdlib", features = ["proptest010"] }
-once_cell = "1.3.1"
 proptest = "0.10"

tools/cargo-compare/src/diff.rs

@@ -3,11 +3,12 @@
 
 use crate::common::GuppyCargoCommon;
 use crate::GlobalContext;
-use anyhow::Result;
+use anyhow::{bail, Result};
 use diffus::{edit, Diffable};
 use guppy::graph::PackageGraph;
 use guppy::PackageId;
 use itertools::Itertools;
+use once_cell::sync::OnceCell;
 use std::collections::{BTreeMap, BTreeSet};
 use std::fmt;
 use structopt::StructOpt;
@@ -25,36 +26,82 @@ pub struct DiffOpts {
 impl DiffOpts {
     /// Executes this command.
     pub fn exec(self, ctx: &GlobalContext) -> Result<()> {
+        let target_host_diff = self.compute_diff(ctx)?;
+        println!("{}", target_host_diff);
+
+        if target_host_diff.any_diff() {
+            bail!("non-empty diff!")
+        } else {
+            Ok(())
+        }
+    }
+
+    pub fn compute_diff<'g>(self, ctx: &'g GlobalContext) -> Result<TargetHostDiff<'g>> {
         let cargo_map = self.common.resolve_cargo(ctx)?;
         let guppy_map = self.common.resolve_guppy(ctx)?;
 
         let target_diff = FeatureDiff {
             graph: ctx.graph(),
-            a: &guppy_map.target_map,
-            b: &cargo_map.target_map,
+            a: guppy_map.target_map,
+            b: cargo_map.target_map,
             verbose: self.verbose,
         };
-        println!("** target diff (guppy -> cargo):\n{}\n", target_diff);
 
         let host_diff = FeatureDiff {
             graph: ctx.graph(),
-            a: &guppy_map.host_map,
-            b: &cargo_map.host_map,
+            a: guppy_map.host_map,
+            b: cargo_map.host_map,
             verbose: self.verbose,
         };
-        println!("** host diff (guppy -> cargo):\n{}", host_diff);
 
-        Ok(())
+        Ok(TargetHostDiff {
+            target_diff,
+            host_diff,
+            any_diff: OnceCell::new(),
+        })
     }
 }
 
-struct FeatureDiff<'g> {
+pub struct TargetHostDiff<'g> {
+    pub target_diff: FeatureDiff<'g>,
+    pub host_diff: FeatureDiff<'g>,
+    any_diff: OnceCell<bool>,
+}
+
+impl<'g> TargetHostDiff<'g> {
+    /// Returns true if there's a diff.
+    pub fn any_diff(&self) -> bool {
+        *self
+            .any_diff
+            .get_or_init(|| self.target_diff.any_diff() || self.host_diff.any_diff())
+    }
+}
+
+impl<'g> fmt::Display for TargetHostDiff<'g> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(
+            f,
+            "** target diff (guppy -> cargo):\n{}\n",
+            self.target_diff
+        )?;
+        write!(f, "** host diff (guppy -> cargo):\n{}\n", self.host_diff)
+    }
+}
+
+pub struct FeatureDiff<'g> {
     graph: &'g PackageGraph,
-    a: &'g BTreeMap<PackageId, BTreeSet<String>>,
-    b: &'g BTreeMap<PackageId, BTreeSet<String>>,
+    a: BTreeMap<PackageId, BTreeSet<String>>,
+    b: BTreeMap<PackageId, BTreeSet<String>>,
     verbose: bool,
 }
 
+impl<'g> FeatureDiff<'g> {
+    /// Returns true if there's a diff.
+    pub fn any_diff(&self) -> bool {
+        self.a.diff(&self.b).is_change()
+    }
+}
+
 impl<'g> fmt::Display for FeatureDiff<'g> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self.a.diff(&self.b) {

tools/cargo-compare/src/tests/proptest_helpers.rs

@@ -5,6 +5,8 @@ use crate::common::GuppyCargoCommon;
 use crate::diff::DiffOpts;
 use crate::GlobalContext;
 use guppy::graph::PackageGraph;
+use proptest::test_runner::{TestCaseError, TestCaseResult};
+use std::env;
 
 macro_rules! proptest_suite {
     ($name: ident) => {
@@ -20,19 +22,31 @@ macro_rules! proptest_suite {
                 proptest!(ProptestConfig::with_cases(fixture.num_proptests()), |(
                     common in fixture.common_strategy(),
                 )| {
-                    compare(fixture.graph(), common);
+                    compare(fixture.graph(), common)?;
                 });
             }
         }
     }
 }
 
 /// Test that there is no diff between guppy and cargo for the same query.
-pub(super) fn compare(graph: &PackageGraph, common: GuppyCargoCommon) {
-    let diff_opts = DiffOpts {
-        common,
-        verbose: false,
+pub(super) fn compare(graph: &PackageGraph, common: GuppyCargoCommon) -> TestCaseResult {
+    let verbose = match env::var("PROPTEST_VERBOSE")
+        .as_ref()
+        .map(|val| val.as_str())
+    {
+        Ok("true") | Ok("1") => true,
+        _ => false,
     };
+    let diff_opts = DiffOpts { common, verbose };
     let ctx = GlobalContext::new(true, graph).expect("context created");
-    diff_opts.exec(&ctx).expect("no errors and no diff found");
+    let target_host_diff = diff_opts
+        .compute_diff(&ctx)
+        .expect("compute_diff succeeded");
+    if target_host_diff.any_diff() {
+        println!("{}", target_host_diff);
+        Err(TestCaseError::fail("diff found"))
+    } else {
+        Ok(())
+    }
 }