Skip to content

Commit

Permalink
Merge pull request #1637 from hannobraun/validate
Browse files Browse the repository at this point in the history
Make validation unit tests more explicit
  • Loading branch information
hannobraun authored Mar 1, 2023
2 parents 5fcb7a1 + 6bc79bd commit 82d5c7a
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 102 deletions.
16 changes: 13 additions & 3 deletions crates/fj-kernel/src/validate/cycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ mod tests {
objects::Cycle,
partial::{Partial, PartialCycle, PartialObject},
services::Services,
validate::Validate,
validate::{CycleValidationError, Validate, ValidationError},
};

#[test]
Expand Down Expand Up @@ -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(())
}
Expand Down Expand Up @@ -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(())
}
Expand Down
40 changes: 18 additions & 22 deletions crates/fj-kernel/src/validate/edge.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use fj_math::{Point, Scalar};

use crate::{
objects::{GlobalEdge, GlobalVertex, HalfEdge, Surface},
objects::{GlobalEdge, GlobalVertex, HalfEdge},
storage::Handle,
};

Expand Down Expand Up @@ -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\
Expand Down Expand Up @@ -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(),
);
}
Expand All @@ -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(),
);
}
Expand All @@ -147,7 +133,7 @@ mod tests {
objects::HalfEdge,
partial::{Partial, PartialCycle},
services::Services,
validate::Validate,
validate::{HalfEdgeValidationError, Validate, ValidationError},
};

#[test]
Expand Down Expand Up @@ -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(())
}
Expand Down Expand Up @@ -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(())
}
Expand Down
93 changes: 19 additions & 74 deletions crates/fj-kernel/src/validate/face.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use fj_math::{Point, Scalar, Winding};

use crate::{
objects::{Cycle, Face, Surface, SurfaceVertex},
objects::{Face, SurfaceVertex},
storage::Handle,
};

Expand All @@ -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\
Expand Down Expand Up @@ -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(),
);
}
Expand Down Expand Up @@ -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(),
);
}
Expand All @@ -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();
Expand Down Expand Up @@ -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(())
}
Expand Down Expand Up @@ -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(())
}
Expand Down
5 changes: 3 additions & 2 deletions crates/fj-kernel/src/validate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion crates/fj-operations/src/shape_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down

0 comments on commit 82d5c7a

Please sign in to comment.