Skip to content

Commit 57c86e6

Browse files
facontidavideclaude
andcommitted
test: Add regression tests for P0/P1 bug fixes
- BuiltinArrayTypeNotMistakenForNestedType: verifies float64[9] is recognized as a builtin (not a nested message type) - ImuSchemaContainsExpectedFields: verifies all IMU fields including fixed-size array covariance fields are present - NonExistentNestedTypeReturnsEmpty: verifies fabricated types fail - SubscribeFailureResponseContainsTopicAndReason: verifies each failure entry in the subscribe response has topic and reason fields Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Entire-Checkpoint: 00acef786069
1 parent 072f7ed commit 57c86e6

2 files changed

Lines changed: 91 additions & 0 deletions

File tree

tests/unit/test_bridge_server.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1338,3 +1338,37 @@ TEST_F(BridgeServerTest, RegressionTest_PausedClientDisconnectNoDoubleDecrement)
13381338
mock_->simulate_disconnect(client2);
13391339
EXPECT_EQ(server_->get_active_session_count(), 0u);
13401340
}
1341+
1342+
// ---------------------------------------------------------------------------
1343+
// Regression: Subscribe to a topic that exists but has an empty schema should
1344+
// report a failure (not silently succeed with an empty definition).
1345+
//
1346+
// In the unit test environment, we can't easily mock SchemaExtractor, but we
1347+
// can verify the failure-reporting path by subscribing to a non-existent topic
1348+
// which produces the "Topic does not exist" failure, confirming the failure
1349+
// response structure is correct.
1350+
// ---------------------------------------------------------------------------
1351+
TEST_F(BridgeServerTest, SubscribeFailureResponseContainsTopicAndReason) {
1352+
ASSERT_TRUE(server_->initialize());
1353+
1354+
json req;
1355+
req["command"] = "subscribe";
1356+
req["topics"] = json::array({"/nonexistent_topic_1", "/nonexistent_topic_2"});
1357+
mock_->push_request("client_fail_multi", req.dump());
1358+
server_->process_requests();
1359+
1360+
json reply = mock_->pop_reply("client_fail_multi");
1361+
ASSERT_FALSE(reply.is_discarded());
1362+
EXPECT_EQ(reply["status"], "error");
1363+
EXPECT_EQ(reply["error_code"], "ALL_SUBSCRIPTIONS_FAILED");
1364+
ASSERT_TRUE(reply.contains("failures"));
1365+
ASSERT_EQ(reply["failures"].size(), 2u);
1366+
1367+
// Each failure entry must have topic and reason
1368+
for (const auto& failure : reply["failures"]) {
1369+
EXPECT_TRUE(failure.contains("topic"));
1370+
EXPECT_TRUE(failure.contains("reason"));
1371+
EXPECT_FALSE(failure["topic"].get<std::string>().empty());
1372+
EXPECT_FALSE(failure["reason"].get<std::string>().empty());
1373+
}
1374+
}

tests/unit/test_schema_extractor.cpp

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,3 +201,60 @@ TEST_F(SchemaExtractorTest, RemoveCommentsOnlyComments) {
201201
}
202202
EXPECT_TRUE(is_empty_or_whitespace) << "Result should be empty or whitespace-only, got: '" << result << "'";
203203
}
204+
205+
// ---------------------------------------------------------------------------
206+
// Regression: Builtin types with array brackets (e.g., float64[9]) must be
207+
// recognised as builtins, not treated as nested message types.
208+
// ---------------------------------------------------------------------------
209+
TEST_F(SchemaExtractorTest, BuiltinArrayTypeNotMistakenForNestedType) {
210+
// sensor_msgs/msg/Imu contains "float64[9] orientation_covariance".
211+
// Before the fix, float64[9] was not recognized as builtin because
212+
// brackets were stripped after the builtin check, causing the schema
213+
// extraction to try to find "sensor_msgs/msg/float64.msg" which fails.
214+
std::string schema = extractor_->get_message_definition("sensor_msgs/msg/Imu");
215+
ASSERT_FALSE(schema.empty()) << "Imu schema should not be empty (float64[9] must be recognised as builtin)";
216+
217+
// Verify the covariance fields are present in the output
218+
EXPECT_NE(schema.find("orientation_covariance"), std::string::npos);
219+
EXPECT_NE(schema.find("angular_velocity_covariance"), std::string::npos);
220+
EXPECT_NE(schema.find("linear_acceleration_covariance"), std::string::npos);
221+
}
222+
223+
// ---------------------------------------------------------------------------
224+
// Non-existent nested type should cause the whole schema to fail
225+
// (not silently omit the nested type).
226+
// ---------------------------------------------------------------------------
227+
TEST_F(SchemaExtractorTest, NonExistentNestedTypeReturnsEmpty) {
228+
// We can't easily create a message type with a fake nested type,
229+
// but we can verify that a completely fabricated package returns empty.
230+
std::string schema = extractor_->get_message_definition("fake_nonexistent_pkg/msg/FakeType");
231+
EXPECT_TRUE(schema.empty()) << "Fabricated nested type should produce empty schema";
232+
}
233+
234+
// ---------------------------------------------------------------------------
235+
// Verify ImuSchemaContainsExpectedFields - specifically tests that the
236+
// schema correctly handles builtin types with fixed-size arrays.
237+
// ---------------------------------------------------------------------------
238+
TEST_F(SchemaExtractorTest, ImuSchemaContainsExpectedFields) {
239+
std::string actual = extractor_->get_message_definition("sensor_msgs/msg/Imu");
240+
ASSERT_FALSE(actual.empty()) << "Failed to extract schema for sensor_msgs/msg/Imu";
241+
242+
std::string actual_no_comments = remove_comments_from_schema(actual);
243+
244+
// Core fields
245+
EXPECT_NE(actual_no_comments.find("std_msgs/Header header"), std::string::npos);
246+
EXPECT_NE(actual_no_comments.find("geometry_msgs/Quaternion orientation"), std::string::npos);
247+
EXPECT_NE(actual_no_comments.find("geometry_msgs/Vector3 angular_velocity"), std::string::npos);
248+
EXPECT_NE(actual_no_comments.find("geometry_msgs/Vector3 linear_acceleration"), std::string::npos);
249+
250+
// Fixed-size array builtins (the regression case)
251+
EXPECT_NE(actual_no_comments.find("float64[9] orientation_covariance"), std::string::npos);
252+
EXPECT_NE(actual_no_comments.find("float64[9] angular_velocity_covariance"), std::string::npos);
253+
EXPECT_NE(actual_no_comments.find("float64[9] linear_acceleration_covariance"), std::string::npos);
254+
255+
// Nested types should be present
256+
EXPECT_NE(actual_no_comments.find("MSG: geometry_msgs/Quaternion"), std::string::npos);
257+
EXPECT_NE(actual_no_comments.find("MSG: geometry_msgs/Vector3"), std::string::npos);
258+
EXPECT_NE(actual_no_comments.find("MSG: std_msgs/Header"), std::string::npos);
259+
EXPECT_NE(actual_no_comments.find("MSG: builtin_interfaces/Time"), std::string::npos);
260+
}

0 commit comments

Comments
 (0)