Skip to content

Make cube non-finite formatting match macOS snprintf - #219

Draft
wavefunction91 with Copilot wants to merge 1 commit into
masterfrom
copilot/fix-macos-build-and-test
Draft

Make cube non-finite formatting match macOS snprintf#219
wavefunction91 with Copilot wants to merge 1 commit into
masterfrom
copilot/fix-macos-build-and-test

Conversation

Copilot AI commented Aug 7, 2026

Copy link
Copy Markdown

macOS CI fails because the cube writer hard-codes signed NaN output while macOS snprintf("%13.5E") formats negative NaN differently. The formatter test compares against the platform implementation.

  • Non-finite cube values

    • Route NaN and infinity through snprintf("%13.5E").
    • Preserve the writer’s fixed 13-byte field layout.
  • Fallback handling

    • Centralize conversion from snprintf output to a 13-byte cube field.
    • Pad safely if formatting returns fewer characters or fails.
if( std::isnan(v) or std::isinf(v) ) {
  format_e13_5_snprintf( v, out );
  return;
}

Copilot AI changed the title [WIP] Fix failing GitHub Actions job macOS Build and Test Make cube non-finite formatting match macOS snprintf Aug 7, 2026
Copilot AI requested a review from wavefunction91 August 7, 2026 15:04

Copy link
Copy Markdown
Owner

Heads up on a bug in the plan for this PR: "The formatter test compares against the platform implementation" makes the regression test tautological. If the cube writer's non-finite branch calls snprintf("%13.5E", v) directly, and the test computes its "expected" value the same way (by calling snprintf on the test host), then the test is just checking that the code calls snprintf — it can never fail, even if the writer is broken, because both sides of the comparison are generated by the identical platform-dependent call at run time. That's exactly how the original bug (hard-coded/inconsistent signed-NaN formatting differing from snprintf output) slipped through CI in the first place.

To make this test meaningful, the expected value needs to be a fixed, pre-recorded golden string (captured once from a real macOS snprintf run) rather than a value computed via the same snprintf call inside the test itself. Otherwise this PR "fixes" the CI failure only by removing the test's ability to detect the very platform-formatting mismatch it's meant to catch.

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.

2 participants