From 0ee526a9baab3085f0f8ae538aed3f07779eb656 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Agust=C3=ADn=20Borgna?=
 <121866228+aborgna-q@users.noreply.github.com>
Date: Mon, 17 Jun 2024 09:23:02 +0100
Subject: [PATCH] chore: Fix failing unsoundness checks (#1196)

The unsoundness check was [failing on
`main`](https://github.com/CQCL/hugr/actions/runs/9512637937/job/26221038735).

- Skips the hugr-cli tests when running miri, as it does not support
calling external processes.
- Move `impl Arbitrary for CustomSerialized` out of a block of
miri-skipped tests.
- Adds a `just miri` command with the correct flags to get proptests
working with miri.

CI run:
https://github.com/CQCL/hugr/actions/runs/9518163988/job/26238422543

---------

Co-authored-by: doug-q <141026920+doug-q@users.noreply.github.com>
---
 hugr-cli/tests/cli.rs                |  6 +++
 hugr-core/src/ops/constant/custom.rs | 75 ++++++++++++++--------------
 justfile                             |  4 ++
 3 files changed, 48 insertions(+), 37 deletions(-)

diff --git a/hugr-cli/tests/cli.rs b/hugr-cli/tests/cli.rs
index beb1e19f2..1cd8b5438 100644
--- a/hugr-cli/tests/cli.rs
+++ b/hugr-cli/tests/cli.rs
@@ -1,3 +1,9 @@
+//! Tests for the CLI
+//!
+//! Miri is globally disabled for these tests because they mostly involve
+//! calling the CLI binary, which Miri doesn't support.
+#![cfg(all(test, not(miri)))]
+
 use assert_cmd::Command;
 use assert_fs::{fixture::FileWriteStr, NamedTempFile};
 use hugr_cli::VALID_PRINT;
diff --git a/hugr-core/src/ops/constant/custom.rs b/hugr-core/src/ops/constant/custom.rs
index eb46a2bb7..e3df87b45 100644
--- a/hugr-core/src/ops/constant/custom.rs
+++ b/hugr-core/src/ops/constant/custom.rs
@@ -476,44 +476,45 @@ mod test {
             serde_yaml::from_value(serde_yaml::to_value(&ev).unwrap()).unwrap()
         );
     }
+}
 
-    mod proptest {
-        use ::proptest::prelude::*;
-
-        use crate::{
-            extension::ExtensionSet,
-            ops::constant::CustomSerialized,
-            proptest::{any_serde_yaml_value, any_string},
-            types::Type,
-        };
-
-        impl Arbitrary for CustomSerialized {
-            type Parameters = ();
-            type Strategy = BoxedStrategy<Self>;
-            fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
-                let typ = any::<Type>();
-                let extensions = any::<ExtensionSet>();
-                // here we manually construct a serialized `dyn CustomConst`.
-                // The "c" and "v" come from the `typetag::serde` annotation on
-                // `trait CustomConst`.
-                // TODO This is not ideal, if we were to accidentally
-                // generate a valid tag(e.g. "ConstInt") then things will
-                // go wrong: the serde::Deserialize impl for that type will
-                // interpret "v" and fail.
-                let value = (any_serde_yaml_value(), any_string()).prop_map(|(content, tag)| {
-                    [("c".into(), tag.into()), ("v".into(), content)]
-                        .into_iter()
-                        .collect::<serde_yaml::Mapping>()
-                        .into()
-                });
-                (typ, value, extensions)
-                    .prop_map(|(typ, value, extensions)| CustomSerialized {
-                        typ,
-                        value,
-                        extensions,
-                    })
-                    .boxed()
-            }
+#[cfg(test)]
+mod proptest {
+    use ::proptest::prelude::*;
+
+    use crate::{
+        extension::ExtensionSet,
+        ops::constant::CustomSerialized,
+        proptest::{any_serde_yaml_value, any_string},
+        types::Type,
+    };
+
+    impl Arbitrary for CustomSerialized {
+        type Parameters = ();
+        type Strategy = BoxedStrategy<Self>;
+        fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
+            let typ = any::<Type>();
+            let extensions = any::<ExtensionSet>();
+            // here we manually construct a serialized `dyn CustomConst`.
+            // The "c" and "v" come from the `typetag::serde` annotation on
+            // `trait CustomConst`.
+            // TODO This is not ideal, if we were to accidentally
+            // generate a valid tag(e.g. "ConstInt") then things will
+            // go wrong: the serde::Deserialize impl for that type will
+            // interpret "v" and fail.
+            let value = (any_serde_yaml_value(), any_string()).prop_map(|(content, tag)| {
+                [("c".into(), tag.into()), ("v".into(), content)]
+                    .into_iter()
+                    .collect::<serde_yaml::Mapping>()
+                    .into()
+            });
+            (typ, value, extensions)
+                .prop_map(|(typ, value, extensions)| CustomSerialized {
+                    typ,
+                    value,
+                    extensions,
+                })
+                .boxed()
         }
     }
 }
diff --git a/justfile b/justfile
index f11bee80d..f7f52f2f0 100644
--- a/justfile
+++ b/justfile
@@ -42,6 +42,10 @@ coverage language="[rust|python]": (_run_lang language \
         "poetry run pytest --cov=./ --cov-report=html"
     )
 
+# Run unsoundness checks using miri
+miri:
+    PROPTEST_DISABLE_FAILURE_PERSISTENCE=true MIRIFLAGS='-Zmiri-env-forward=PROPTEST_DISABLE_FAILURE_PERSISTENCE' cargo +nightly miri test
+
 # Load a shell with all the dependencies installed
 shell:
     poetry shell