Skip to content

Compare floats tolerantly in round-trip test instead of ignoring them - #120

Merged
argonui merged 1 commit into
mainfrom
test/e2e-compare-numbers
Aug 4, 2026
Merged

Compare floats tolerantly in round-trip test instead of ignoring them#120
argonui merged 1 commit into
mainfrom
test/e2e-compare-numbers

Conversation

@argonui

@argonui argonui commented Aug 3, 2026

Copy link
Copy Markdown
Owner

Problem

Fixes #98

The end-to-end round-trip test tests/e2e_test.go treated every float64 as unpredictable and skipped it entirely:

if _, ok := v.(float64); ok {
    return true
}

That meant numeric corruption in the reverse→build round trip passed undetected — the very thing the round-trip property is supposed to guarantee.

Fix

Stop skipping floats. Compare them approximately with cmpopts.EquateApprox(0, 1e-4). Only Date and EpochTime remain ignored, since they are regenerated on every build by design.

The near-identical ignoreUnpredictable in root compare_test.go got the same treatment (a shared approxFloats option applied to both cmp.Diff calls).

Tolerance choice: 1e-4

Number smoothing (see objects/AGENTS.md) rounds positions to 3 decimal places, scale to 2, and colors to 5. The coarsest of those, 3dp, means legitimate values differ by at most 5e-4 from an unrounded input. An absolute tolerance of 1e-4 sits comfortably below any real change a user could make in-game (nudging a card moves it far more than 1e-4) while staying above floating-point rounding noise. It is tight enough to catch real numeric corruption yet loose enough not to flag representation jitter.

Fixture normalization

For a tolerance this tight to hold, the compared floats must be in their committed (smoothed) form — the form the tool always emits. 3_snap_points.json already stores whole-degree rotations, but two fixtures (GHE_Dev_no_objects, GHE_Dev_no_objects_no_lua) still contained raw TTS snap-point Position/Rotation values (e.g. 180.0298, -36.0014) that the tool smooths on every build and therefore never round-trip identically. Those snap-point values are smoothed in place, exactly matching the tool's own output (verified programmatically). No other fields changed. Rotations round to whole degrees, which is why raw snap-point rotations could not survive a tight tolerance untouched.

Proof the test now catches corruption

Temporarily mutated smoothPos in objects/numbersmoother.go to roundFloat(f, 3) * 2:

  • Before this change (blanket float skip): go test ./tests/ passed — corruption invisible.
  • After this change, with the same mutation: go test ./tests/ FAILS. The fixtures carrying position data (3_snap_points, GHE_Dev_no_objects, GHE_Dev_no_objects_no_lua) report the doubled coordinates; position-free fixtures still pass.

Reverting the mutation restores full green: go test ./..., go build ./..., and go vet ./... all pass.

The e2e round-trip test (and the near-identical helper in compare_test.go)
skipped every float64 value, so numeric corruption round-tripped undetected.

Replace the blanket float skip with cmpopts.EquateApprox(0, 1e-4). Only Date
and EpochTime remain ignored (regenerated on every build). The 1e-4 absolute
tolerance sits below the smallest smoothing precision (positions 3dp, scale
2dp, colors 5dp) so it absorbs rounding noise while catching real movement.

For the tolerance to hold, the compared floats must be in their committed
(smoothed) form. Two GHE_Dev fixtures still held raw TTS snap-point Positions
and Rotations that never round-tripped identically; smooth those values in
place (matching the tool's own output exactly) as 3_snap_points already is.

Fixes #98

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@argonui
argonui force-pushed the test/e2e-compare-numbers branch from f871ebd to 614a0c6 Compare August 4, 2026 03:04
@argonui
argonui merged commit bd803b4 into main Aug 4, 2026
1 check passed
@argonui
argonui deleted the test/e2e-compare-numbers branch August 4, 2026 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant