Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion src/shared/test_util/tbots_gtest_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,37 @@

std::string runtime_dir = "/tmp/tbots/yellow_test";

/**
* Portable wrapper for feenableexcept. Use to specify which floating-point
* exceptions should crash the program when they occur.
*
* @note On MacOS ARM, there are no floating-point exception traps, so tests may
* pass on MacOS that wouldn't pass on other platforms if floating-point
* exceptions occur.
*
* @param excepts A bitmask of floating-point exceptions to be enabled.
* @return True on success, false on failure to set floating-point exceptions.
*/
bool enable_fp_exceptions(unsigned int excepts)
{
#if defined(__linux__) && defined(__GNUC__)
feenableexcept(excepts);
return true;
#else
// Unsupported platform
return false;
Comment on lines +21 to +26
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea I can't seem to get it working, something with the gtest environment I guess?

#endif
}

int main(int argc, char** argv)
{
testing::InitGoogleTest(&argc, argv);
feenableexcept(FE_INVALID | FE_OVERFLOW);

// Crash on invalid operations like sqrt of negative and floating-point overflow
if (!enable_fp_exceptions(FE_INVALID | FE_OVERFLOW))
{
std::cerr << "Warning: Could not enable floating-point exceptions." << std::endl;
}

LoggerSingleton::initializeLogger(runtime_dir, nullptr);

Expand Down
42 changes: 22 additions & 20 deletions src/software/math/math_functions_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,69 +2,71 @@

#include <gtest/gtest.h>

#include <limits>

TEST(LinearUtilFunctionTest, testZeroCase)
{
double out = linear(0, 0, 2);
EXPECT_EQ(out, 0.5);
EXPECT_NEAR(out, 0.5, std::numeric_limits<double>::epsilon());
}

TEST(LinearUtilFunctionTest, testOneQuarter)
{
double out = linear(-1, 0, 4);
EXPECT_EQ(out, 0.25);
EXPECT_NEAR(out, 0.25, std::numeric_limits<double>::epsilon());
}

TEST(LinearUtilFunctionTest, testTwoThirds)
{
double out = linear(0.75, 0, 4.5);
EXPECT_EQ(out, 2.0 / 3.0);
EXPECT_NEAR(out, 2.0 / 3.0, std::numeric_limits<double>::epsilon());
}

TEST(LinearUtilFunctionTest, testMinimumNoOffset)
{
double out = linear(-1.5, 0, 3);
EXPECT_EQ(out, 0.0);
EXPECT_NEAR(out, 0.0, std::numeric_limits<double>::epsilon());
}

TEST(LinearUtilFunctionTest, testMaximumNoOffset)
{
double out = linear(2.5, 0, 5.0);
EXPECT_EQ(out, 1.0);
}

TEST(LinearUtilFunctionTest, testClampBelowNoOffset)
{
double out = linear(-2, 0, 1);
EXPECT_EQ(out, 0.0);
}

TEST(LinearUtilFunctionTest, testClampAboveNoOffset)
{
double out = linear(4.2, 0, 6);
EXPECT_EQ(out, 1.0);
EXPECT_NEAR(out, 1.0, std::numeric_limits<double>::epsilon());
}

TEST(LinearUtilFunctionTest, testMinimumNegativeOffset)
{
double out = linear(-4, -2, 4);
EXPECT_EQ(out, 0.0);
EXPECT_NEAR(out, 0.0, std::numeric_limits<double>::epsilon());
}

TEST(LinearUtilFunctionTest, testMaximumNegativeOffset)
{
double out = linear(1.5, -1, 5.0);
EXPECT_EQ(out, 1.0);
EXPECT_NEAR(out, 1.0, std::numeric_limits<double>::epsilon());
}

TEST(LinearUtilFunctionTest, testMinimumPositiveOffset)
{
double out = linear(0, 3, 6);
EXPECT_EQ(out, 0.0);
EXPECT_NEAR(out, 0.0, std::numeric_limits<double>::epsilon());
}

TEST(LinearUtilFunctionTest, testMaximumPositiveOffset)
{
double out = linear(6, 1.5, 9);
EXPECT_NEAR(out, 1.0, std::numeric_limits<double>::epsilon());
}

TEST(LinearUtilFunctionTest, testClampBelowNoOffset)
{
double out = linear(-2, 0, 1);
EXPECT_EQ(out, 0.0);
}

TEST(LinearUtilFunctionTest, testClampAboveNoOffset)
{
double out = linear(4.2, 0, 6);
EXPECT_EQ(out, 1.0);
}

Expand Down
4 changes: 0 additions & 4 deletions src/software/simulation/er_force_simulator_test.cpp
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
#include "software/simulation/er_force_simulator.h"

#include <gtest/gtest.h>
// TODO (#2419): remove this
#include <fenv.h>

#include "proto/message_translation/er_force_world.h"
#include "proto/message_translation/tbots_protobuf.h"
Expand All @@ -15,8 +13,6 @@ class ErForceSimulatorTest : public ::testing::Test
protected:
void SetUp() override
{
// TODO (#2419): remove this to re-enable sigfpe checks
fedisableexcept(FE_INVALID | FE_OVERFLOW);
auto realism_config = ErForceSimulator::createDefaultRealismConfig();
simulator = std::make_shared<ErForceSimulator>(TbotsProto::FieldType::DIV_B,
robot_constants, realism_config);
Expand Down
8 changes: 7 additions & 1 deletion src/software/thunderscope/requirements_lock.darwin.txt
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,13 @@ pyqtgraph==0.13.7 \
--hash=sha256:64f84f1935c6996d0e09b1ee66fe478a7771e3ca6f3aaa05f00f6e068321d9e3 \
--hash=sha256:7754edbefb6c367fa0dfb176e2d0610da3ada20aa7a5318516c74af5fb72bf7a
# via -r software/thunderscope/requirements.in
qtawesome==1.4.0 \
--hash=sha256:783e414d1317f3e978bf67ea8e8a1b1498bad9dbd305dec814027e3b50521be6 \
--hash=sha256:a4d689fa071c595aa6184171ce1f0f847677cb8d2db45382c43129f1d72a3d93
# via -r software/thunderscope/requirements.in
Comment on lines +123 to +126
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these intended dep updates?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do they need to be done, seems to be something fit for another pr (or for #3542, I promise I'll finish that soon)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the software tests that failed was for the requirements; I just updated the requirements cause it was supposed to be done way earlier

qtpy==2.4.2 \
--hash=sha256:5a696b1dd7a354cb330657da1d17c20c2190c72d4888ba923f8461da67aa1a1c \
--hash=sha256:9d6ec91a587cc1495eaebd23130f7619afa5cdd34a277acb87735b4ad7c65156
# via pyqt-toast-notification
# via
# pyqt-toast-notification
# qtawesome
6 changes: 6 additions & 0 deletions src/software/util/typename/typename_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,13 @@ class TestTypeA : public TestType
TEST(TypeNameTest, abstract_base_class_concrete_subtype)
{
std::shared_ptr<TestType> test_type = std::make_shared<TestTypeA>();
#ifdef __APPLE__
// With clang / libc++, the standard library uses an inline namespace '__1' for
// ABI versioning, which is reflected in the demangled type name.
EXPECT_EQ("std::__1::shared_ptr<TestType>", objectTypeName(test_type));
#else
EXPECT_EQ("std::shared_ptr<TestType>", objectTypeName(test_type));
#endif
EXPECT_EQ("TestTypeA", objectTypeName(*test_type));
}

Expand Down
6 changes: 6 additions & 0 deletions src/starlark/nanopb/nanopb.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,11 @@ def _construct_cc_info(
user_compile_flags = copts,
)

# Flags required for macos linker which allow symbols to be resolved at runtime
link_flags = []
if "darwin" in cc_toolchain.cpu:
link_flags = ["-Wl,-undefined,dynamic_lookup"]

(linking_context, linking_outputs) = \
cc_common.create_linking_context_from_compilation_outputs(
name = "link_nanopb_outputs",
Expand All @@ -235,6 +240,7 @@ def _construct_cc_info(
feature_configuration = feature_configuration,
cc_toolchain = cc_toolchain,
linking_contexts = nanopb_linking_contexts,
user_link_flags = link_flags,
)

extra_context = cc_common.create_compilation_context(
Expand Down
Loading