Make validation unit tests more explicit

crates/fj-kernel/src/validate/cycle.rs

@@ -135,7 +135,7 @@ mod tests {
         objects::Cycle,
         partial::{Partial, PartialCycle, PartialObject},
         services::Services,
-        validate::Validate,
+        validate::{CycleValidationError, Validate, ValidationError},
     };
 
     #[test]
@@ -174,7 +174,12 @@ mod tests {
         };
 
         valid.validate_and_return_first_error()?;
-        assert!(invalid.validate_and_return_first_error().is_err());
+        assert!(matches!(
+            invalid.validate_and_return_first_error(),
+            Err(ValidationError::Cycle(
+                CycleValidationError::HalfEdgeConnection { .. }
+            ))
+        ));
 
         Ok(())
     }
@@ -210,7 +215,12 @@ mod tests {
         };
 
         valid.validate_and_return_first_error()?;
-        assert!(invalid.validate_and_return_first_error().is_err());
+        assert!(matches!(
+            invalid.validate_and_return_first_error(),
+            Err(ValidationError::Cycle(
+                CycleValidationError::HalfEdgeBoundaryMismatch { .. }
+            ))
+        ));
 
         Ok(())
     }

crates/fj-kernel/src/validate/edge.rs

@@ -1,7 +1,7 @@
 use fj_math::{Point, Scalar};
 
 use crate::{
-    objects::{GlobalEdge, GlobalVertex, HalfEdge, Surface},
+    objects::{GlobalEdge, GlobalVertex, HalfEdge},
     storage::Handle,
 };
 
@@ -50,20 +50,6 @@ pub enum HalfEdgeValidationError {
         half_edge: HalfEdge,
     },
 
-    /// Mismatch between the surface's of the curve and surface form
-    #[error(
-        "Surface form of vertex must be defined on same surface as curve\n\
-        - `Surface` of curve: {curve_surface:#?}\n\
-        - `Surface` of surface form: {surface_form_surface:#?}"
-    )]
-    SurfaceMismatch {
-        /// The surface of the vertex' curve
-        curve_surface: Handle<Surface>,
-
-        /// The surface of the vertex' surface form
-        surface_form_surface: Handle<Surface>,
-    },
-
     /// [`HalfEdge`]'s vertices are coincident
     #[error(
         "Vertices of `HalfEdge` on curve are coincident\n\
@@ -106,11 +92,11 @@ impl HalfEdgeValidationError {
 
         if matching_global_vertex.is_none() {
             errors.push(
-                Box::new(Self::GlobalVertexMismatch {
+                Self::GlobalVertexMismatch {
                     global_vertex_from_half_edge,
                     global_vertices_from_global_form,
                     half_edge: half_edge.clone(),
-                })
+                }
                 .into(),
             );
         }
@@ -126,12 +112,12 @@ impl HalfEdgeValidationError {
 
         if distance < config.distinct_min_distance {
             errors.push(
-                Box::new(Self::VerticesAreCoincident {
+                Self::VerticesAreCoincident {
                     back_position,
                     front_position,
                     distance,
                     half_edge: half_edge.clone(),
-                })
+                }
                 .into(),
             );
         }
@@ -147,7 +133,7 @@ mod tests {
         objects::HalfEdge,
         partial::{Partial, PartialCycle},
         services::Services,
-        validate::Validate,
+        validate::{HalfEdgeValidationError, Validate, ValidationError},
     };
 
     #[test]
@@ -195,7 +181,12 @@ mod tests {
         };
 
         valid.validate_and_return_first_error()?;
-        assert!(invalid.validate_and_return_first_error().is_err());
+        assert!(matches!(
+            invalid.validate_and_return_first_error(),
+            Err(ValidationError::HalfEdge(
+                HalfEdgeValidationError::GlobalVertexMismatch { .. }
+            ))
+        ));
 
         Ok(())
     }
@@ -231,7 +222,12 @@ mod tests {
         };
 
         valid.validate_and_return_first_error()?;
-        assert!(invalid.validate_and_return_first_error().is_err());
+        assert!(matches!(
+            invalid.validate_and_return_first_error(),
+            Err(ValidationError::HalfEdge(
+                HalfEdgeValidationError::VerticesAreCoincident { .. }
+            ))
+        ));
 
         Ok(())
     }

crates/fj-kernel/src/validate/face.rs

@@ -1,7 +1,7 @@
 use fj_math::{Point, Scalar, Winding};
 
 use crate::{
-    objects::{Cycle, Face, Surface, SurfaceVertex},
+    objects::{Face, SurfaceVertex},
     storage::Handle,
 };
 
@@ -21,24 +21,6 @@ impl Validate for Face {
 /// [`Face`] validation error
 #[derive(Clone, Debug, thiserror::Error)]
 pub enum FaceValidationError {
-    /// [`Surface`] of an interior [`Cycle`] doesn't match [`Face`]'s `Surface`
-    #[error(
-        "`Surface` of an interior `Cycle` doesn't match `Face`'s `Surface`\n\
-        - `Surface` of the `Face`: {surface:#?}\n\
-        - Invalid interior `Cycle`: {interior:#?}\n\
-        - `Face`: {face:#?}"
-    )]
-    SurfaceMismatch {
-        /// The surface of the [`Face`]
-        surface: Handle<Surface>,
-
-        /// The invalid interior cycle of the [`Face`]
-        interior: Handle<Cycle>,
-
-        /// The face
-        face: Face,
-    },
-
     /// Interior of [`Face`] has invalid winding; must be opposite of exterior
     #[error(
         "Interior of `Face` has invalid winding; must be opposite of exterior\n\
@@ -94,11 +76,11 @@ impl FaceValidationError {
 
             if exterior_winding == interior_winding {
                 errors.push(
-                    Box::new(Self::InvalidInteriorWinding {
+                    Self::InvalidInteriorWinding {
                         exterior_winding,
                         interior_winding,
                         face: face.clone(),
-                    })
+                    }
                     .into(),
                 );
             }
@@ -127,13 +109,13 @@ impl FaceValidationError {
 
                     if distance > config.identical_max_distance {
                         errors.push(
-                            Box::new(Self::VertexPositionMismatch {
+                            Self::VertexPositionMismatch {
                                 surface_position: surface_vertex.position(),
                                 surface_position_as_global,
                                 global_position,
                                 distance,
                                 surface_vertex: surface_vertex.clone(),
-                            })
+                            }
                             .into(),
                         );
                     }
@@ -153,58 +135,11 @@ mod tests {
         builder::{CycleBuilder, FaceBuilder, HalfEdgeBuilder},
         insert::Insert,
         objects::{Cycle, Face, HalfEdge, SurfaceVertex},
-        partial::{Partial, PartialCycle, PartialFace, PartialObject},
+        partial::{Partial, PartialFace, PartialObject},
         services::Services,
-        validate::Validate,
+        validate::{FaceValidationError, Validate, ValidationError},
     };
 
-    #[test]
-    fn face_surface_mismatch() -> anyhow::Result<()> {
-        let mut services = Services::new();
-
-        let valid = {
-            let mut face = PartialFace {
-                surface: Partial::from(services.objects.surfaces.xy_plane()),
-                ..Default::default()
-            };
-            face.exterior.write().update_as_polygon_from_points([
-                [0., 0.],
-                [3., 0.],
-                [0., 3.],
-            ]);
-            face.add_interior().write().update_as_polygon_from_points([
-                [1., 1.],
-                [1., 2.],
-                [2., 1.],
-            ]);
-
-            face.build(&mut services.objects)
-        };
-        let invalid = {
-            let surface = services.objects.surfaces.xz_plane();
-
-            let mut cycle = PartialCycle::default();
-            cycle.update_as_polygon_from_points([[1., 1.], [1., 2.], [2., 1.]]);
-            cycle.infer_vertex_positions_if_necessary(&surface.geometry());
-            let cycle = cycle
-                .build(&mut services.objects)
-                .insert(&mut services.objects);
-
-            let interiors = [cycle];
-            Face::new(
-                valid.surface().clone(),
-                valid.exterior().clone(),
-                interiors,
-                valid.color(),
-            )
-        };
-
-        valid.validate_and_return_first_error()?;
-        assert!(invalid.validate_and_return_first_error().is_err());
-
-        Ok(())
-    }
-
     #[test]
     fn face_invalid_interior_winding() -> anyhow::Result<()> {
         let mut services = Services::new();
@@ -242,7 +177,12 @@ mod tests {
         };
 
         valid.validate_and_return_first_error()?;
-        assert!(invalid.validate_and_return_first_error().is_err());
+        assert!(matches!(
+            invalid.validate_and_return_first_error(),
+            Err(ValidationError::Face(
+                FaceValidationError::InvalidInteriorWinding { .. }
+            ))
+        ));
 
         Ok(())
     }
@@ -317,7 +257,12 @@ mod tests {
         };
 
         valid.validate_and_return_first_error()?;
-        assert!(invalid.validate_and_return_first_error().is_err());
+        assert!(matches!(
+            invalid.validate_and_return_first_error(),
+            Err(ValidationError::Face(
+                FaceValidationError::VertexPositionMismatch { .. }
+            ))
+        ));
 
         Ok(())
     }

crates/fj-kernel/src/validate/mod.rs

@@ -23,6 +23,7 @@ use fj_math::Scalar;
 /// This trait is used automatically when inserting an object into a store.
 pub trait Validate: Sized {
     /// Validate the object using default config and return on first error
+    #[allow(clippy::result_large_err)]
     fn validate_and_return_first_error(&self) -> Result<(), ValidationError> {
         let mut errors = Vec::new();
         self.validate(&mut errors);
@@ -88,11 +89,11 @@ pub enum ValidationError {
 
     /// `Face` validation error
     #[error("`Face` validation error:\n{0}")]
-    Face(#[from] Box<FaceValidationError>),
+    Face(#[from] FaceValidationError),
 
     /// `HalfEdge` validation error
     #[error("`HalfEdge` validation error:\n{0}")]
-    HalfEdge(#[from] Box<HalfEdgeValidationError>),
+    HalfEdge(#[from] HalfEdgeValidationError),
 }
 
 impl From<Infallible> for ValidationError {

crates/fj-operations/src/shape_processor.rs

@@ -60,7 +60,7 @@ impl ShapeProcessor {
 pub enum Error {
     /// Error converting to shape
     #[error("Error converting to shape")]
-    ToShape(#[from] ValidationError),
+    ToShape(#[from] Box<ValidationError>),
 
     /// Model has zero size
     #[error("Model has zero size")]