Migrate tests to use MockWarningHandler for warning interception.

PiperOrigin-RevId: 933656759
Change-Id: Ia21514c302799034bf598445933dd90427284737
This commit is contained in:
Yuval Tassa
2026-06-17 05:06:15 -07:00
committed by Copybara-Service
parent 38c3388046
commit cedaa47d61
9 changed files with 72 additions and 122 deletions
+14 -17
View File
@@ -31,7 +31,6 @@
#include <mujoco/mjtype.h> #include <mujoco/mjtype.h>
#include <mujoco/mujoco.h> #include <mujoco/mujoco.h>
#include <mujoco/mjxmacro.h> #include <mujoco/mjxmacro.h>
#include "src/cc/array_safety.h"
#include "src/engine/engine_callback.h" #include "src/engine/engine_callback.h"
#include "src/engine/engine_core_util.h" #include "src/engine/engine_core_util.h"
#include "src/engine/engine_io.h" #include "src/engine/engine_io.h"
@@ -68,10 +67,11 @@ static const char* const kTendonForceClamp =
using ::testing::Pointwise; using ::testing::Pointwise;
using ::testing::Ne; using ::testing::_;
using ::testing::HasSubstr;
using ::testing::NotNull;
using ::testing::Gt; using ::testing::Gt;
using ::testing::HasSubstr;
using ::testing::Ne;
using ::testing::NotNull;
// --------------------------- activation limits ------------------------------- // --------------------------- activation limits -------------------------------
@@ -997,32 +997,28 @@ TEST_F(ForwardTest, ControlClamping) {
// data->ctrl[1] remains pristine // data->ctrl[1] remains pristine
EXPECT_EQ(data->ctrl[1], 2); EXPECT_EQ(data->ctrl[1], 2);
// install warning handler MockWarningHandler warning_handler;
static char warning[1024];
warning[0] = '\0';
mju_user_warning = [](const char* msg) {
util::strcpy_arr(warning, msg);
};
// for the unclamped actuator, huge raises warning // for the unclamped actuator, huge raises warning
warning_handler.ExpectWarnings(
"Nan, Inf or huge value in CTRL at ACTUATOR 0");
data->ctrl[0] = 10*mjMAXVAL; data->ctrl[0] = 10*mjMAXVAL;
mj_forward(model, data); mj_forward(model, data);
EXPECT_THAT(warning, testing::Mock::VerifyAndClearExpectations(&warning_handler);
HasSubstr("Nan, Inf or huge value in CTRL at ACTUATOR 0"));
// for the clamped actuator, huge does not raise warning // for the clamped actuator, huge does not raise warning
EXPECT_CALL(warning_handler, Warn(_)).Times(0);
mj_resetData(model, data); mj_resetData(model, data);
warning[0] = '\0';
data->ctrl[1] = 10*mjMAXVAL; data->ctrl[1] = 10*mjMAXVAL;
mj_forward(model, data); mj_forward(model, data);
EXPECT_EQ(warning[0], '\0'); testing::Mock::VerifyAndClearExpectations(&warning_handler);
// for the clamped actuator, NaN raises warning // for the clamped actuator, NaN raises warning
warning_handler.ExpectWarnings(
"Nan, Inf or huge value in CTRL at ACTUATOR 1");
mj_resetData(model, data); mj_resetData(model, data);
data->ctrl[1] = std::numeric_limits<double>::quiet_NaN(); data->ctrl[1] = std::numeric_limits<double>::quiet_NaN();
mj_forward(model, data); mj_forward(model, data);
EXPECT_THAT(warning,
HasSubstr("Nan, Inf or huge value in CTRL at ACTUATOR 1"));
mj_deleteData(data); mj_deleteData(data);
mj_deleteModel(model); mj_deleteModel(model);
@@ -2865,7 +2861,8 @@ TEST_F(ActEarlyTest, RemovesOneStepDelay) {
mj_step(model, data); mj_step(model, data);
for (int j = 0; j < model->nu / 2; j++) { for (int j = 0; j < model->nu / 2; j++) {
// this is true for torque actuators // this is true for torque actuators
EXPECT_NEAR(last_qfrc[2 * j], data->qfrc_actuator[2 * j + 1], MjTol(1e-3, 1e-1)) EXPECT_NEAR(last_qfrc[2 * j], data->qfrc_actuator[2 * j + 1],
MjTol(1e-3, 1e-1))
<< "there should be a 1 step delay between qfrc for " << "there should be a 1 step delay between qfrc for "
<< mj_id2name(model, mjOBJ_ACTUATOR, 2 * j); << mj_id2name(model, mjOBJ_ACTUATOR, 2 * j);
} }
+5 -14
View File
@@ -31,7 +31,6 @@
#include <absl/strings/str_format.h> #include <absl/strings/str_format.h>
#include <mujoco/mjxmacro.h> #include <mujoco/mjxmacro.h>
#include <mujoco/mujoco.h> #include <mujoco/mujoco.h>
#include "src/engine/engine_util_errmem.h"
#include "src/engine/engine_thread.h" #include "src/engine/engine_thread.h"
#include "test/fixture.h" #include "test/fixture.h"
@@ -130,14 +129,10 @@ TEST_F(EngineIoTest, MakeDataReturnsNullOnFailure) {
// fail mj_makeData intentionally with a bad size // fail mj_makeData intentionally with a bad size
model->nbody = -1; model->nbody = -1;
static bool warning; MockWarningHandler warning_handler;
warning = false; warning_handler.ExpectWarnings();
mju_user_warning = [](const char* error) {
warning = true;
};
mjData* data = mj_makeData(model); mjData* data = mj_makeData(model);
EXPECT_THAT(data, IsNull()); EXPECT_THAT(data, IsNull());
EXPECT_TRUE(warning) << "Expecting warning to be triggered.";
mj_deleteData(data); mj_deleteData(data);
mj_deleteModel(model); mj_deleteModel(model);
@@ -964,18 +959,14 @@ TEST_F(EngineIoTest, LoadModelBufferRejectsOverflowingSizes) {
// nbuffer mismatch check — but the overflow should be caught earlier // nbuffer mismatch check — but the overflow should be caught earlier
// in safeAddToBufferSize/mj_makeModel before we reach that check. // in safeAddToBufferSize/mj_makeModel before we reach that check.
// intercept mju_warning because the test framework translates it to ADD_FAILURE // Intercept warnings to prevent them from failing the test.
static bool warning_triggered = false; MockWarningHandler warning_handler;
warning_triggered = false; warning_handler.ExpectWarnings();
mju_user_warning = [](const char* msg) {
warning_triggered = true;
};
// attempt to load — should return NULL, not crash // attempt to load — should return NULL, not crash
mjModel* bad_model = mj_loadModelBuffer(buffer.data(), bufsize); mjModel* bad_model = mj_loadModelBuffer(buffer.data(), bufsize);
EXPECT_THAT(bad_model, IsNull()) EXPECT_THAT(bad_model, IsNull())
<< "Expected mj_loadModelBuffer to reject overflow-inducing sizes"; << "Expected mj_loadModelBuffer to reject overflow-inducing sizes";
EXPECT_TRUE(warning_triggered) << "Expected a warning about invalid sizes";
// clean up if somehow it succeeded // clean up if somehow it succeeded
if (bad_model) { if (bad_model) {
+16 -9
View File
@@ -83,9 +83,16 @@ MockWarningHandler::MockWarningHandler() {
// Restores the previously active warning handler. // Restores the previously active warning handler.
MockWarningHandler::~MockWarningHandler() { active_handler = prev_; } MockWarningHandler::~MockWarningHandler() { active_handler = prev_; }
// Configures the mock warning handler to ignore all warnings. // Configures the mock warning handler to ignore all warnings (if empty) or
void MockWarningHandler::ExpectWarnings() { // expect at least one warning containing substring (if non-empty).
EXPECT_CALL(*this, Warn(_)).WillRepeatedly(Return()); void MockWarningHandler::ExpectWarnings(std::string_view substring) {
if (substring.empty()) {
EXPECT_CALL(*this, Warn(_)).WillRepeatedly(Return());
} else {
EXPECT_CALL(*this, Warn(::testing::HasSubstr(std::string(substring))))
.Times(::testing::AtLeast(1))
.WillRepeatedly(Return());
}
} }
// Returns the active warning handler. // Returns the active warning handler.
@@ -102,10 +109,12 @@ static mjfLogHandler prev_log_handler ABSL_GUARDED_BY(handlers_mutex) = nullptr;
void default_mj_log_handler(const mjLogMessage* msg) { void default_mj_log_handler(const mjLogMessage* msg) {
std::string subject = msg->subject; std::string subject = msg->subject;
if (msg->func) { if (msg->func) {
subject = std::string(msg->func) + ": " + msg->subject; subject = absl::StrCat(msg->func, ": ", msg->subject);
} }
if (msg->level == mjLOG_ERROR) { if (msg->level == mjLOG_ERROR) {
// legacy fallback: some tests still install mju_user_error to intercept
// errors with longjmp-based capture
if (mju_user_error) { if (mju_user_error) {
mju_user_error(subject.c_str()); mju_user_error(subject.c_str());
} else { } else {
@@ -114,14 +123,12 @@ void default_mj_log_handler(const mjLogMessage* msg) {
} else if (msg->level == mjLOG_WARNING) { } else if (msg->level == mjLOG_WARNING) {
std::string full_msg = subject; std::string full_msg = subject;
if (msg->body) { if (msg->body) {
full_msg += "\n" + std::string(msg->body); absl::StrAppend(&full_msg, "\n", msg->body);
} }
if (mju_user_warning) { if (auto* handler = MockWarningHandler::GetActive()) {
mju_user_warning(full_msg.c_str());
} else if (auto* handler = MockWarningHandler::GetActive()) {
handler->Warn(full_msg); handler->Warn(full_msg);
} else { } else {
ADD_FAILURE() << "mju_user_warning: " << full_msg; ADD_FAILURE() << "Unexpected warning: " << full_msg;
} }
} }
} }
+3 -2
View File
@@ -117,8 +117,9 @@ class MockWarningHandler {
// Mock method called when a warning is intercepted. // Mock method called when a warning is intercepted.
MOCK_METHOD(void, Warn, (const std::string& msg)); MOCK_METHOD(void, Warn, (const std::string& msg));
// Allow any number of warnings without triggering test failure. // Allow any number of warnings (if empty) or expect at least one warning
void ExpectWarnings(); // containing the specified substring (if non-empty).
void ExpectWarnings(std::string_view substring = "");
// Returns the thread-local active mock warning handler. // Returns the thread-local active mock warning handler.
static MockWarningHandler* GetActive(); static MockWarningHandler* GetActive();
+10 -5
View File
@@ -24,9 +24,9 @@
#include <string> #include <string>
#include <vector> #include <vector>
#include <absl/strings/str_format.h>
#include <gmock/gmock.h> #include <gmock/gmock.h>
#include <gtest/gtest.h> #include <gtest/gtest.h>
#include "src/cc/array_safety.h"
#include <mujoco/mujoco.h> #include <mujoco/mujoco.h>
#include <mujoco/mjspec.h> #include <mujoco/mjspec.h>
#include <mujoco/mjplugin.h> #include <mujoco/mjplugin.h>
@@ -38,9 +38,8 @@ namespace mujoco {
namespace { namespace {
using ::testing::HasSubstr; using ::testing::HasSubstr;
using ::testing::NotNull;
using ::testing::IsNull; using ::testing::IsNull;
using ::testing::NotNull;
// -------------------------- test model manipulation ------------------------- // -------------------------- test model manipulation -------------------------
@@ -243,7 +242,8 @@ int open_mock(mjResource* resource) {
</mujoco> </mujoco>
)"; )";
resource->data = mju_malloc(sizeof(parent_xml)); resource->data = mju_malloc(sizeof(parent_xml));
std::strcpy((char*)resource->data, parent_xml); absl::SNPrintF(static_cast<char*>(resource->data), sizeof(parent_xml), "%s",
parent_xml);
return 1; return 1;
} }
@@ -2668,6 +2668,12 @@ void AttachNestedKeyframe(bool compile) {
// compile required before further attachment // compile required before further attachment
mjModel* m_child = compile ? mj_compile(child, 0) : nullptr; mjModel* m_child = compile ? mj_compile(child, 0) : nullptr;
// check warning is issued, empty for a compiled model
MockWarningHandler warning_handler;
if (!compile) {
warning_handler.ExpectWarnings("model has pending keyframes");
}
// attach child to parent // attach child to parent
mjs_attach(mjs_findFrame(parent, "frame")->element, mjs_attach(mjs_findFrame(parent, "frame")->element,
mjs_findBody(child, "body")->element, "child-", ""); mjs_findBody(child, "body")->element, "child-", "");
@@ -2679,7 +2685,6 @@ void AttachNestedKeyframe(bool compile) {
EXPECT_THAT(mjs_getWarning(parent, 0), EXPECT_THAT(mjs_getWarning(parent, 0),
HasSubstr("model has pending keyframes")); HasSubstr("model has pending keyframes"));
} }
// compare models // compare models
mjtNum tol = 0; mjtNum tol = 0;
std::string field = ""; std::string field = "";
+2 -6
View File
@@ -30,7 +30,6 @@
#include <mujoco/mjmodel.h> #include <mujoco/mjmodel.h>
#include <mujoco/mjtype.h> #include <mujoco/mjtype.h>
#include <mujoco/mujoco.h> #include <mujoco/mujoco.h>
#include "src/cc/array_safety.h"
#include "test/fixture.h" #include "test/fixture.h"
namespace mujoco { namespace mujoco {
@@ -1310,11 +1309,8 @@ TEST_F(MjCMeshTest, NaNConvexHullDisallowed) {
</asset> </asset>
</mujoco> </mujoco>
)"; )";
static char warning[1024]; MockWarningHandler warning_handler;
warning[0] = '\0'; warning_handler.ExpectWarnings();
mju_user_warning = [](const char* msg) {
util::strcpy_arr(warning, msg);
};
std::array<char, 1024> error; std::array<char, 1024> error;
mjModel* model = LoadModelFromString(xml, error.data(), error.size()); mjModel* model = LoadModelFromString(xml, error.data(), error.size());
EXPECT_THAT(model, testing::IsNull()); EXPECT_THAT(model, testing::IsNull());
+18 -60
View File
@@ -24,7 +24,7 @@
#include <gmock/gmock.h> #include <gmock/gmock.h>
#include <gtest/gtest.h> #include <gtest/gtest.h>
#include "src/cc/array_safety.h" #include <absl/strings/str_format.h>
#include <mujoco/mjplugin.h> #include <mujoco/mjplugin.h>
#include <mujoco/mujoco.h> #include <mujoco/mujoco.h>
#include "src/engine/engine_plugin.h" #include "src/engine/engine_plugin.h"
@@ -51,8 +51,9 @@ int open_str(mjResource* resource) {
return 0; return 0;
} }
resource->data = mju_malloc(100*sizeof(char)); const std::size_t kBufferSize = 100;
std::strcpy((char*) resource->data, "Hello World"); resource->data = mju_malloc(kBufferSize * sizeof(char));
absl::SNPrintF((char*) resource->data, kBufferSize, "Hello World");
return 1; return 1;
} }
@@ -128,17 +129,10 @@ TEST_F(ResourceTest, RegisterProviderMissingCallbacks) {
.prefix = "myprefix", .prefix = "myprefix",
}; };
// install warning handler MockWarningHandler warning_handler;
static char warning[1024]; warning_handler.ExpectWarnings("callback");
warning[0] = '\0';
mju_user_warning = [](const char* msg) {
util::strcpy_arr(warning, msg);
};
int i = mjp_registerResourceProvider(&provider); int i = mjp_registerResourceProvider(&provider);
// warning message related to missing callbacks
EXPECT_THAT(warning, HasSubstr("callback"));
EXPECT_LT(i, 1); EXPECT_LT(i, 1);
} }
@@ -150,17 +144,10 @@ TEST_F(ResourceTest, RegisterProviderMissingPrefix) {
.close = close_nop, .close = close_nop,
}; };
// install warning handler MockWarningHandler warning_handler;
static char warning[1024]; warning_handler.ExpectWarnings("prefix");
warning[0] = '\0';
mju_user_warning = [](const char* msg) {
util::strcpy_arr(warning, msg);
};
int i = mjp_registerResourceProvider(&provider); int i = mjp_registerResourceProvider(&provider);
// warning message related to missing prefix
EXPECT_THAT(warning, HasSubstr("prefix"));
EXPECT_LT(i, 1); EXPECT_LT(i, 1);
} }
@@ -172,17 +159,10 @@ TEST_F(ResourceTest, RegisterProviderInvalidPrefix1) {
.close = close_nop, .close = close_nop,
}; };
// install warning handler MockWarningHandler warning_handler;
static char warning[1024]; warning_handler.ExpectWarnings("prefix");
warning[0] = '\0';
mju_user_warning = [](const char* msg) {
util::strcpy_arr(warning, msg);
};
int i = mjp_registerResourceProvider(&provider); int i = mjp_registerResourceProvider(&provider);
// warning message related to missing prefix
EXPECT_THAT(warning, HasSubstr("prefix"));
EXPECT_LT(i, 1); EXPECT_LT(i, 1);
} }
@@ -194,17 +174,10 @@ TEST_F(ResourceTest, RegisterProviderInvalidPrefix2) {
.close = close_nop, .close = close_nop,
}; };
// install warning handler MockWarningHandler warning_handler;
static char warning[1024]; warning_handler.ExpectWarnings("prefix");
warning[0] = '\0';
mju_user_warning = [](const char* msg) {
util::strcpy_arr(warning, msg);
};
int i = mjp_registerResourceProvider(&provider); int i = mjp_registerResourceProvider(&provider);
// warning message related to missing prefix
EXPECT_THAT(warning, HasSubstr("prefix"));
EXPECT_LT(i, 1); EXPECT_LT(i, 1);
} }
@@ -316,13 +289,8 @@ TEST_F(ResourceTest, NameWithValidPrefix) {
int i = mjp_registerResourceProvider(&provider); int i = mjp_registerResourceProvider(&provider);
EXPECT_GT(i, 0); EXPECT_GT(i, 0);
MockWarningHandler warning_handler;
// install warning handler warning_handler.ExpectWarnings();
static char warning[1024];
warning[0] = '\0';
mju_user_warning = [](const char* msg) {
util::strcpy_arr(warning, msg);
};
// open resource // open resource
mjResource* resource = mju_openResource("", "nop:found", nullptr, nullptr, 0); mjResource* resource = mju_openResource("", "nop:found", nullptr, nullptr, 0);
@@ -342,13 +310,8 @@ TEST_F(ResourceTest, NameWithUpperCasePrefix) {
int i = mjp_registerResourceProvider(&provider); int i = mjp_registerResourceProvider(&provider);
EXPECT_GT(i, 0); EXPECT_GT(i, 0);
MockWarningHandler warning_handler;
// install warning handler warning_handler.ExpectWarnings();
static char warning[1024];
warning[0] = '\0';
mju_user_warning = [](const char* msg) {
util::strcpy_arr(warning, msg);
};
// open resource // open resource
mjResource* resource = mju_openResource("", "NOP:found", nullptr, nullptr, 0); mjResource* resource = mju_openResource("", "NOP:found", nullptr, nullptr, 0);
@@ -368,13 +331,8 @@ TEST_F(ResourceTest, NameWithInvalidPrefix) {
int i = mjp_registerResourceProvider(&provider); int i = mjp_registerResourceProvider(&provider);
EXPECT_GT(i, 0); EXPECT_GT(i, 0);
MockWarningHandler warning_handler;
// install warning handler warning_handler.ExpectWarnings();
static char warning[1024];
warning[0] = '\0';
mju_user_warning = [](const char* msg) {
util::strcpy_arr(warning, msg);
};
// open resource // open resource
mjResource* resource = mju_openResource("", "nopfound", nullptr, nullptr, 0); mjResource* resource = mju_openResource("", "nopfound", nullptr, nullptr, 0);
+2 -2
View File
@@ -24,7 +24,6 @@
#include <gtest/gtest.h> #include <gtest/gtest.h>
#include <gmock/gmock.h> #include <gmock/gmock.h>
#include <mujoco/mujoco.h>
#include "test/fixture.h" #include "test/fixture.h"
namespace mujoco { namespace mujoco {
@@ -160,7 +159,8 @@ TEST_F(UserUtilTest, StringToVectorInvalidNumber) {
} }
TEST_F(UserUtilTest, StringToVectorNan) { TEST_F(UserUtilTest, StringToVectorNan) {
mju_user_warning = nullptr; MockWarningHandler warning_handler;
warning_handler.ExpectWarnings();
auto v = StringToVector<double>("1 2 nan 3.21"); auto v = StringToVector<double>("1 2 nan 3.21");
EXPECT_THAT(v[2], IsNan()); EXPECT_THAT(v[2], IsNan());
EXPECT_EQ(v[3], 3.21); EXPECT_EQ(v[3], 3.21);
+2 -7
View File
@@ -28,7 +28,6 @@
#include <mujoco/mjspec.h> #include <mujoco/mjspec.h>
#include <mujoco/mujoco.h> #include <mujoco/mujoco.h>
#include "src/cc/array_safety.h" #include "src/cc/array_safety.h"
#include "src/engine/engine_util_errmem.h"
#include "src/user/user_api.h" #include "src/user/user_api.h"
#include "src/xml/xml_api.h" #include "src/xml/xml_api.h"
#include "test/compare_model.h" #include "test/compare_model.h"
@@ -381,14 +380,10 @@ TEST_F(XMLReaderTest, CanParseNanAndRaisesWarning) {
</mujoco> </mujoco>
)"; )";
std::array<char, 1024> error; std::array<char, 1024> error;
static char warning[1024]; MockWarningHandler warning_handler;
warning[0] = '\0'; warning_handler.ExpectWarnings("XML contains a 'NaN'");
mju_user_warning = [](const char* msg) {
util::strcpy_arr(warning, msg);
};
mjModel* model = LoadModelFromString(xml, error.data(), error.size()); mjModel* model = LoadModelFromString(xml, error.data(), error.size());
ASSERT_THAT(model, NotNull()); ASSERT_THAT(model, NotNull());
EXPECT_THAT(warning, HasSubstr("XML contains a 'NaN'"));
EXPECT_THAT(model->geom_pos[0], IsNan()); EXPECT_THAT(model->geom_pos[0], IsNan());
EXPECT_THAT(model->geom_pos[1], IsNan()); EXPECT_THAT(model->geom_pos[1], IsNan());
EXPECT_THAT(model->geom_pos[2], IsNan()); EXPECT_THAT(model->geom_pos[2], IsNan());