Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Evaluate fixed precision C tests #670

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Evaluate fixed precision C tests #670

wants to merge 3 commits into from

Conversation

drinckes
Copy link
Contributor

@drinckes drinckes commented Dec 30, 2024

This is a test PR only - not for merging. It relates to #669.

This changes the C encoding test to only test the fixed precision (integer) encoding implementation.

It splits the fixed precision part of encoding in the C implementation into a separate function, and modifies the encoding tests to directly test it. It also adds a test runner for MacOS to see if we can still get encoding differences.

@drinckes drinckes self-assigned this Dec 30, 2024
@drinckes
Copy link
Contributor Author

drinckes commented Dec 30, 2024

A previous PR that added a macos-latest test runner encountered C encoding failures with (among others):

59.32,-157.21,12,93F48QCR+2222

Instead of the desired code, it generated 93F48QCQ+2X55. This must be down to some very small difference in the conversion of the longitude (since the difference is on even digits "Q" and "X"). macos-latest uses arm64/M1 (ubuntu hardware isn't specified.)

Changing the tests to use fixed precision lat/lngs (computed on ubuntu) gives us:

3733000000,186695680,12,93F48QCR+2222

Running the tests on macos-latest all pass. So this is one way for us to have reliable cross-platform tests.

UPDATE: I mean yes the tests fail but that's because of the formatting check. The encoding unit tests pass.

@drinckes drinckes added the c label Dec 30, 2024
@drinckes drinckes linked an issue Dec 30, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve encoding/decoding consistency in tests
1 participant