Giter VIP home page Giter VIP logo

Comments (4)

pbaughman avatar pbaughman commented on July 30, 2024

Something similar happens with test_record_all_no_discovery.cpp if I run the test in a loop in parallel with

colcon test --packages-select rcl

but that may be a different failure - I don't see messages about rosbag2 subscribing to extra topics

Running main() from gmock_main.cc
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from RecordIntegrationTestFixture
[ RUN      ] RecordIntegrationTestFixture.record_all_without_discovery_ignores_later_announced_topics
[INFO] [rosbag2_transport]: Listening for topics...
[INFO] [rosbag2_transport]: Subscribed to topic '/parameter_events'
/workspace/src/rosbag2/rosbag2_transport/test/rosbag2_transport/test_record_all_no_discovery.cpp:41: Failure
Value of: writer_->get_messages()
Expected: size is equal to 0
  Actual: { 16-byte object <10-E6 3E-E1 79-7F 00-00 00-E6 3E-E1 79-7F 00-00> }, whose size 1 doesn't match
[  FAILED  ] RecordIntegrationTestFixture.record_all_without_discovery_ignores_later_announced_topics (240 ms)
[----------] 1 test from RecordIntegrationTestFixture (240 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (240 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] RecordIntegrationTestFixture.record_all_without_discovery_ignores_later_announced_topics

 1 FAILED TEST

from rosbag2.

pbaughman avatar pbaughman commented on July 30, 2024

I've made some progress fixing test_record_all by doing:

-  ASSERT_THAT(recorded_messages, SizeIs(4));                                       
+  ASSERT_GE(recorded_messages.size(), 4u);                                         
   auto string_messages = filter_messages<test_msgs::msg::Primitives>(
     recorded_messages, string_topic);
   auto array_messages = filter_messages<test_msgs::msg::StaticArrayPrimitives>(
     recorded_messages, array_topic);
-  ASSERT_THAT(string_messages, SizeIs(2));                                         
-  ASSERT_THAT(array_messages, SizeIs(2));                                          
-  EXPECT_THAT(string_messages[0]->string_value, Eq("Hello World"));                
-  EXPECT_THAT(array_messages[0]->bool_values, ElementsAre(true, false, true));     
-  EXPECT_THAT(array_messages[0]->string_values,                                    
-    ElementsAre("Complex Hello1", "Complex Hello2", "Complex Hello3"));            
+  ASSERT_GE(string_messages.size(), 2u);                                           
+  ASSERT_GE(array_messages.size(), 2u);                                            
+                                                                                   
+                                                                                   
+  EXPECT_THAT(                                                                     
+    string_messages,                                                               
+    UnorderedElementsAre(                                                          
+      Pointee(Field(&test_msgs::msg::Primitives::string_value, Eq("Hello World"))),
+      Pointee(Field(&test_msgs::msg::Primitives::string_value, Eq("Hello World"))) 
+    )                                                                              
+  );                                                                               
+                                                                                   
+  EXPECT_THAT(                                                                     
+    array_messages,                                                                
+    UnorderedElementsAre(                                                          
+      Pointee(                                                                     
+        Field(                                                                     
+          &test_msgs::msg::StaticArrayPrimitives::bool_values,                     
+          ElementsAre(true, false, true)                                           
+        )                                                                          
+      ),                                                                           
+      Pointee(                                                                     
+        Field(                                                                     
+          &test_msgs::msg::StaticArrayPrimitives::bool_values,                     
+          ElementsAre(true, false, true)                                           
+        )                                                                          
+      )                                                                            
+    )                                                                              
+  );                                                                               
+                                                                                   
+  EXPECT_THAT(                                                                     
+    array_messages,                                                                
+    UnorderedElementsAre(                                                          
+      Pointee(                                                                     
+        Field(                                                                     
+          &test_msgs::msg::StaticArrayPrimitives::string_values,                   
+          ElementsAre("Complex Hello1", "Complex Hello2", "Complex Hello3")        
+        )                                                                          
+      ),                                                                           
+      Pointee(                                                                     
+        Field(                                                                     
+          &test_msgs::msg::StaticArrayPrimitives::string_values,                   
+          ElementsAre("Complex Hello1", "Complex Hello2", "Complex Hello3")        
+        )                                                                          
+      )                                                                            
+    )                                                                              
+  );                                                                               

But the interference goes both ways. The rcl test test_get_node_names for example does not like to run at the same time as the rosbag2 tests

from rosbag2.

Karsten1987 avatar Karsten1987 commented on July 30, 2024

I believe it makes sense to have the rosbag2 tests run more robustly, but tests in general will always be subject to parallel execution when testing discovery behavior.
In this case I assume it's fine to prefix topics, or namespace them, so avoid duplication and assert correctly to see that we have the right topics present.

I am happy to review a PR for this.

Could we make the test assert that size of the recorded messages is 4 in case some other topics got sampled?

I believe we can do this, but should further assert then that these are indeed the right messages. That is to verify that all received messages are coming from the test.

Is there a way to namespace the topics that the test is recording?

I believe that's the right approach to avoid duplication.

Related: can we tell rosbag2 to only record from /array_topic and /string_topic?

We can tell that and I actually believe there is already a test for this. One of them being here:

auto process_handle = start_execution("ros2 bag record --output " + bag_path_ + " /test_topic");

from rosbag2.

pbaughman avatar pbaughman commented on July 30, 2024

@Karsten1987 I got about halfway done with fixing this before I realized that the interference goes both ways. Initially I (naively) thought that other tests were making rosbag2 tests fail, but that rosbag2 testss didn't make other tests fail. This assumption is wrong.

I'm considering a more general approach now where we can use ROS_DOMAIN_ID or something else to isolate parallel tests from one-another. I'm going to close this issue for now because while I'm confident we can 'fix' the rosbag2 tests, it will be very time consuming and maintenance heavy.

from rosbag2.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.