From c3c79b6e2255bd10a10ea7d305a43522d8ed43de Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Thu, 27 Aug 2026 22:37:26 +0200 Subject: [PATCH 1/2] Add failing specs for the two sides reading the same, and for sort-axes X is compared as written, and the sides have opposite signs, so the left side is walked from the centre outwards and the right side from the outside in. The fixture holds one mirrored pair of columns, and the spec says only that the two sides agree, not which way they run, so it cannot be satisfied by picking a direction. The two pending ones say what the sort-axes key has to do once it exists, since a config naming it is accepted and ignored today. --- .../mirrored-columns-repro.jbeam | 28 +++++++++++++++++ test-extra/transformation/Spec.hs | 1 + test-extra/transformation/Spec/Config.hs | 22 +++++++++++++ test-extra/transformation/Spec/Regression.hs | 31 +++++++++++++++++++ 4 files changed, 82 insertions(+) create mode 100644 examples/regression_jbeam/mirrored-columns-repro.jbeam diff --git a/examples/regression_jbeam/mirrored-columns-repro.jbeam b/examples/regression_jbeam/mirrored-columns-repro.jbeam new file mode 100644 index 00000000..2f7ea004 --- /dev/null +++ b/examples/regression_jbeam/mirrored-columns-repro.jbeam @@ -0,0 +1,28 @@ +{ +"testpart":{ + "nodes":[ + ["id", "posX", "posY", "posZ"], + // Synthetic regression-test fixture, not vetted by the jbeam + // maintainer and not intended as a demo/example. + // + // Two columns per side, mirrored across the centre line, at one + // station so only the columns can decide the order. X is compared + // raw, so the left side runs from the centre outwards while the + // right side runs from the outside in, and no existing fixture has + // both sides in columns to show it. + ["nl0", 0.40, -1.00, 0.10], + ["nl1", 0.40, -1.00, 0.50], + ["nl2", 0.80, -1.00, 0.10], + ["nl3", 0.80, -1.00, 0.50], + ["nr0", -0.40, -1.00, 0.10], + ["nr1", -0.40, -1.00, 0.50], + ["nr2", -0.80, -1.00, 0.10], + ["nr3", -0.80, -1.00, 0.50], + ], + "beams":[ + ["id1:", "id2:"], + ["nl0", "nl1"], + ["nr0", "nr1"], + ], +}, +} diff --git a/test-extra/transformation/Spec.hs b/test-extra/transformation/Spec.hs index eda6c24b..a2240838 100644 --- a/test-extra/transformation/Spec.hs +++ b/test-extra/transformation/Spec.hs @@ -162,6 +162,7 @@ main = hspec $ do letterEndingNodesSpec ySortingBandingSpec xColumnSortingSpec + mirroredColumnsSpec metadataAcrossTreesSpec metadataPreservedSpec triangleMetadataSpec diff --git a/test-extra/transformation/Spec/Config.hs b/test-extra/transformation/Spec/Config.hs index 5394401f..e7e5d520 100644 --- a/test-extra/transformation/Spec/Config.hs +++ b/test-extra/transformation/Spec/Config.hs @@ -14,6 +14,7 @@ import Data.Text (Text) import Data.Text qualified as T import Data.Text.Encoding (encodeUtf8) import JbeamEdit.Transformation.Config +import JbeamEdit.Transformation.Types (Axis (..), SortAxes (..)) import Test.Hspec parseField :: (TransformationConfig -> a) -> Text -> Either Text a @@ -76,6 +77,27 @@ configParsingSpec = describe "the transformation config parser" $ do it "rejects a support-threshold below 1" $ parseField supportThreshold "support-threshold: 0.8\n" `shouldSatisfy` isLeft + it "reads a permutation of the three axes" $ do + pendingWith + "sort-axes has no key in the parser yet, so a config naming one is \ + \accepted and then ignored, and the file sorts by the default axes \ + \without saying so. Issue #243." + parseField sortAxes (olderThresholds <> "sort-axes: [X, Y, Z]\n") + `shouldBe` Right (SortAxes AxisX AxisY AxisZ) + + it "rejects a list that is not three distinct axes" $ do + pendingWith + "the same missing key, from the other side: a repeated or short list \ + \has to be refused where it is written, not silently replaced by the \ + \default. Issue #243." + parseField sortAxes (olderThresholds <> "sort-axes: [X, X, Y]\n") + `shouldSatisfy` isLeft + parseField sortAxes (olderThresholds <> "sort-axes: [X, Y]\n") + `shouldSatisfy` isLeft + + it "gives the axes the tool has always used when the key is absent" $ + parseField sortAxes olderThresholds `shouldBe` Right defaultSortAxes + it "gives every default for an empty file" $ do parseField ySortingThreshold "" `shouldBe` Right defaultSortingThreshold parseField supportThreshold "" `shouldBe` Right defaultSupportThreshold diff --git a/test-extra/transformation/Spec/Regression.hs b/test-extra/transformation/Spec/Regression.hs index 71aaf418..9790adaf 100644 --- a/test-extra/transformation/Spec/Regression.hs +++ b/test-extra/transformation/Spec/Regression.hs @@ -10,6 +10,7 @@ module Spec.Regression ( metadataAcrossTreesSpec, metadataPreservedSpec, xColumnSortingSpec, + mirroredColumnsSpec, ) where import Data.Map qualified as M @@ -220,3 +221,33 @@ xColumnSortingSpec = case transform M.empty columnSortingConfig node of Left err -> expectationFailure ("transform failed: " ++ T.unpack err) Right (_, _, _, resultNode) -> assert resultNode + +{- | Both sides of a car hold the same shape mirrored, so the order the +transform writes them in should mirror too. `compareAV` and the column pass +compare X as written, and the sides have opposite signs, so the left side is +walked from the centre outwards and the right side from the outside in. The +maintainer wants the outer column first, which is what the right side already +does by accident. + +The assertion says only that the two sides agree, not which way they run, so +it cannot be satisfied by the current code whichever direction is chosen. +-} +mirroredColumnsFixture :: FilePath +mirroredColumnsFixture = + "examples/regression_jbeam/mirrored-columns-repro.jbeam" + +mirroredColumnsSpec :: Spec +mirroredColumnsSpec = + describe "two sides holding mirrored columns" + . it "writes them back in mirrored order" + $ do + topNode <- parseJbeamFile mirroredColumnsFixture + case transform M.empty newTransformationConfig topNode of + Left err -> expectationFailure ("transform failed: " ++ T.unpack err) + Right (_, _, _, resultNode) -> do + let coordinates = vertexCoordinatesInOrder resultNode + left = filter (\(x, _, _) -> x > 0) coordinates + right = filter (\(x, _, _) -> x < 0) coordinates + mirror (x, y, z) = (negate x, y, z) + left `shouldNotBe` [] + map mirror right `shouldBe` left From 9e92acfa644e96baba450aad632db24b332c2dff Mon Sep 17 00:00:00 2001 From: webdevred <148627186+webdevred@users.noreply.github.com> Date: Fri, 28 Aug 2026 17:39:59 +0200 Subject: [PATCH 2/2] Cut the direction the issue owns from the fixture comment --- test-extra/transformation/Spec/Regression.hs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/test-extra/transformation/Spec/Regression.hs b/test-extra/transformation/Spec/Regression.hs index 9790adaf..373e0630 100644 --- a/test-extra/transformation/Spec/Regression.hs +++ b/test-extra/transformation/Spec/Regression.hs @@ -225,12 +225,10 @@ xColumnSortingSpec = {- | Both sides of a car hold the same shape mirrored, so the order the transform writes them in should mirror too. `compareAV` and the column pass compare X as written, and the sides have opposite signs, so the left side is -walked from the centre outwards and the right side from the outside in. The -maintainer wants the outer column first, which is what the right side already -does by accident. +walked from the centre outwards and the right side from the outside in. -The assertion says only that the two sides agree, not which way they run, so -it cannot be satisfied by the current code whichever direction is chosen. +Which direction both should take is still open, so the assertion says only +that they agree. -} mirroredColumnsFixture :: FilePath mirroredColumnsFixture =