test the euler characteristic in the test suite, and add a mechanism …#499
Merged
julialongtin merged 2 commits intomasterfrom Mar 19, 2026
Merged
test the euler characteristic in the test suite, and add a mechanism …#499julialongtin merged 2 commits intomasterfrom
julialongtin merged 2 commits intomasterfrom
Conversation
…for a test to specify a resolution.
There was a problem hiding this comment.
Pull request overview
Adds support for inline primitive-module tests to specify a rendering/discretization resolution, and wires those inline tests into the Hspec suite by checking mesh Euler characteristics.
Changes:
- Extend
ArgParser’sAPTestnode to carry an optionalℝresolution and update consumers (e.g.,docgen). - Add
atResolutionandcollectTestshelpers for inline tests inUtil.ArgParser. - Run collected inline primitive tests in
tests/ImplicitSpec.hsby executing SCAD and validating invariants (Euler characteristic) on the discretized mesh.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tests/ImplicitSpec.hs |
Collects inline tests from primitiveModules, runs them via runOpenscad, and checks Euler characteristic invariants. |
programs/docgen.hs |
Updates APTest pattern match to reflect the new constructor shape. |
implicit.cabal |
Exposes Graphics.Implicit.ExtOpenScad.Util.ArgParser so the test suite can import collectTests. |
Graphics/Implicit/ExtOpenScad/Util/ArgParser.hs |
Adds per-test resolution support (atResolution) and extraction of inline tests (collectTests); updates APTest constructor usage. |
Graphics/Implicit/ExtOpenScad/Primitives.hs |
Moves/adds inline tests near the start of primitive argparsers; temporarily comments out broken/unsupported ones. |
Graphics/Implicit/ExtOpenScad/Definitions.hs |
Extends APTest to include Maybe ℝ resolution and updates the Monad instance accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+85
to
+86
| | isJust maybeRes = error "tried to set a resolution of a test twice." | ||
| | otherwise = APTest str (Just res) tests child |
| collectTests :: ArgParser a -> [(Text, Maybe ℝ, [TestInvariant])] | ||
| collectTests (APTest str maybeRes tests child) = (str, maybeRes, tests) : collectTests child | ||
| collectTests (APExample _ child) = collectTests child | ||
| collectTests (APBranch branches) = concatMap collectTests branches |
Comment on lines
+417
to
+419
| f = length triangles | ||
| e = length $ nub $ concatMap edges triangles | ||
| v = length $ nub $ concatMap vertices triangles |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…for a test to specify a resolution.