Fix MacOS C++ tests compatibility issues by nycrat · Pull Request #3677 · UBC-Thunderbots/Software

nycrat · 2026-04-05T00:45:46Z

Description

This PR makes unit tests and c++ simulated tests able to run on macos, and resolved issues caused by differences between mac and ubuntu (clang & gcc).

Remove fedisableexcept which is no longer an issue: Fix Floating Point Errors with Er Force Simulator #2419
Add portable wrapper for feenableexcept. Note: on MacOS ARM, it is not possible to crash on floating-point exceptions. So, there is a warning logged when unit tests are ran that floating point exceptions may not be caught.
Resolve issue with nanopb: (14:40:33) ERROR: /Users/avah/2_school/team/thunderbots/src/proto/BUILD:64:21: Linking proto/liblink_nanopb_outputs.dylib failed: (Exit 1): cc_wrapper.sh failed: error exe cuting CppLink command (from target //proto:tbots_nanopb_proto) by adding linker flags to use dynamic lookup
Fix failing tests (math_functions, typename, thunderscope requirements). More details below.

Screen.Recording.2026-04-06.at.13.55.44.mov

Testing Done

Ran simulated gameplay test suite and everything works. Ran software tests and there were three test failures and how they were fixed:

math_functions_test fails due to floating-point rounding error (use EXPECT_NEAR with abs_error of 1e-15 instead of EXPECT_EQ)
thunderscope requirements_lock was outdated (just updated it)
typename_test did not work since clang (libc++) uses internal namespace std::__1 (fixed by adding macro to the test case to check for macos, could possibly add to functionality of demangleTypeId function to handle this internal namespace)

Resolved Issues

There was no issue made, but in #3496, it was stated that unit tests do not work on macos: #3496 (comment).

Length Justification and Key Files to Review

Review Checklist

It is the reviewers responsibility to also make sure every item here has been covered

Function & Class comments: All function definitions (usually in the .h file) should have a javadoc style comment at the start of them. For examples, see the functions defined in thunderbots/software/geom. Similarly, all classes should have an associated Javadoc comment explaining the purpose of the class.
Remove all commented out code
Remove extra print statements: for example, those just used for testing
Resolve all TODO's: All TODO (or similar) statements should either be completed or associated with a github issue

Andrewyx

Left some comments

Andrewyx · 2026-04-07T23:16:55Z

@@ -63,7 +89,8 @@ int main(int argc, char **argv)
        {
            // disable floating point errors when using visualizer due to potential
            // floating point errors in QT
-            fedisableexcept(FE_INVALID | FE_OVERFLOW);
+            // TODO #(2510) Remove this once we port over to simulated pytests entirely
+            // fedisableexcept(FE_INVALID | FE_OVERFLOW);


Why not remove this now?

Oh true I'll remove them now

Andrewyx · 2026-04-07T23:18:41Z

 {
    double out = linear(0, 0, 2);
-    EXPECT_EQ(out, 0.5);
+    EXPECT_NEAR(out, 0.5, 1e-15);


Could we use std::numeric_limits<double>::epsilon() here instead?

Sure that should work

Andrewyx · 2026-04-07T23:19:32Z

+qtawesome==1.4.0 \
+    --hash=sha256:783e414d1317f3e978bf67ea8e8a1b1498bad9dbd305dec814027e3b50521be6 \
+    --hash=sha256:a4d689fa071c595aa6184171ce1f0f847677cb8d2db45382c43129f1d72a3d93
+    # via -r software/thunderscope/requirements.in


Are these intended dep updates?

Andrewyx · 2026-04-07T23:20:32Z

+    # Linker flags required for macos, doesn't affect linux
+    link_flags = ["-Wl,-undefined,dynamic_lookup"]


Does this affect the robots/cross-compile toolchain? They run on ARM.

I'll look into it a bit more, from what I understand its a difference between apple's llvm ld and gnu ld

Lmh-java · 2026-04-08T21:52:23Z

+#if defined(__linux__) && defined(__GNUC__)
+    feenableexcept(excepts);
+    return true;
+#else
+    // Unsupported platform
+    return false;


This looks fine, but just curious:
Is there any reason that we did not use polyfills such as
https://github.com/adobe/lagrange/blob/771d85889dd052b545de1fa4a66fe4b3ff2c5e91/modules/core/src/fpe.cpp#L94-L113

Not sure if polyfills like this will work, but I think if it does, it will enable more consistency behaviours across different platforms?

Oh yea I tested it and it should technically be possible since I got it working for a minimal example https://gist.github.com/nycrat/3bfd574a7b3d328e51ae1b3705de9ebc. But for some reason it doesn't work with gtest. I'll keep trying to make it work since that would be optimal

nycrat added 2 commits April 4, 2026 17:44

Add portable wrapper around feenableexcept

f1a5999

Add specific linker flags for macos

3f999c4

nycrat requested review from Andrewyx, sauravbanna and williamckha as code owners April 5, 2026 00:45

nycrat changed the title ~~Resolve MacOS c++ test compatibility issues~~ Resolve MacOS c++ unit tests compatibility issues Apr 5, 2026

nycrat changed the title ~~Resolve MacOS c++ unit tests compatibility issues~~ Fix MacOS C++ unit tests compatibility issues Apr 5, 2026

nycrat marked this pull request as draft April 5, 2026 00:50

nycrat changed the title ~~Fix MacOS C++ unit tests compatibility issues~~ [DRAFT] Fix MacOS C++ unit tests compatibility issues Apr 5, 2026

nycrat added 7 commits April 4, 2026 18:08

Remove other calls to feenableexcept

13405a6

Update enable_fp_exceptions function

d822e30

Remove macos floating point exception trap after testing

58d7e18

Formatting

779d584

Update thunderscope requirements_lock.darwin.txt

28fcbe4

Fix and add comment explaining differences in typename test

e6ed708

Use EXPECT_NEAR for floating point calculations

793110c

nycrat changed the title ~~[DRAFT] Fix MacOS C++ unit tests compatibility issues~~ [DRAFT] Fix MacOS C++ tests compatibility issues Apr 6, 2026

nycrat changed the title ~~[DRAFT] Fix MacOS C++ tests compatibility issues~~ Fix MacOS C++ tests compatibility issues Apr 6, 2026

nycrat marked this pull request as ready for review April 6, 2026 22:32

nycrat requested review from Lmh-java and maggiettu April 6, 2026 22:33

Andrewyx reviewed Apr 7, 2026

View reviewed changes

Remove fedisableexcept

031ab83

nycrat force-pushed the avah/resolve_macos_cpp_test_issues branch from 9db98e8 to 031ab83 Compare April 8, 2026 03:00

Use numeric_limits epsilon instead of 1e-15

bbf6240

nycrat force-pushed the avah/resolve_macos_cpp_test_issues branch from 914b755 to bbf6240 Compare April 8, 2026 03:03

nycrat requested a review from Andrewyx April 8, 2026 18:51

Lmh-java reviewed Apr 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix MacOS C++ tests compatibility issues#3677

Fix MacOS C++ tests compatibility issues#3677
nycrat wants to merge 11 commits intoUBC-Thunderbots:masterfrom
nycrat:avah/resolve_macos_cpp_test_issues

nycrat commented Apr 5, 2026 •

edited

Loading

Uh oh!

Andrewyx left a comment

Uh oh!

Andrewyx Apr 7, 2026

Uh oh!

nycrat Apr 7, 2026

Uh oh!

Andrewyx Apr 7, 2026

Uh oh!

nycrat Apr 7, 2026

Uh oh!

Andrewyx Apr 7, 2026

Uh oh!

nycrat Apr 7, 2026

Uh oh!

Andrewyx Apr 7, 2026

Uh oh!

nycrat Apr 7, 2026

Uh oh!

Lmh-java Apr 8, 2026

Uh oh!

nycrat Apr 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		# Linker flags required for macos, doesn't affect linux
		link_flags = ["-Wl,-undefined,dynamic_lookup"]

Conversation

nycrat commented Apr 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing Done

Resolved Issues

Length Justification and Key Files to Review

Review Checklist

Uh oh!

Andrewyx left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nycrat commented Apr 5, 2026 •

edited

Loading