UBCSailbot / sailbot_workspace

UBC Sailbot's monorepo
https://ubcsailbot.github.io/sailbot_workspace/main/
Apache License 2.0
4 stars 1 forks source link

Integration Test Framework #224

Closed hhenry01 closed 8 months ago

hhenry01 commented 9 months ago

Description

Config Files

This is where we define our tests.

required_packages is basically a list of stuff we want to have running for the test, and any extra config options/files we want to give it. Non-ROS executables like virtual_iridium and the website are to be put here too, they'll just be handled explicitly.

inputs is a list of inputs. I'm thinking the type can be something like ROS or HTTP, the name is the topic name or URL in the case of HTTP, and body is the actual input data. The body will need to be processed differently depending on the type, but we should be able to generically set key value pairs based on the type.

expected_outputs is formatted the exact same way as the inputs, just on the other end.

Test Flow

  1. Run the python script with a config file.
  2. Start all specified packages.
  3. Start monitoring output topics.
  4. Send all inputs.
  5. Over the span of 3(?) seconds, check that the expected outputs appear.

Verification

Resources

hhenry01 commented 8 months ago

I think it would be nice if our integrations tests just ran our global launch file. We can have test-only nodes and parameters if needed

Right now I have it so the integration test file runs the ros2 launch ... command to start up our nodes. I'll see how launching nodes from a node will look and hopefully it won't be super janky.

hhenry01 commented 8 months ago

I've got everything I consider as essential features implemented. One nice-to-have would be to allow ranges for expected values, seeing as we can't be exact with some of our expectations. This would be a fair bit of extra work though so I have a DONT_CARE key for such scenarios instead.

I still need to work on the documentation. My naming conventions are also a mess because I was loosely following the style used in boat_simulator and controller but didn't read any style guidelines :P

For getting it to work with ros2 launch I'll need to look into how Python ROS packages work.

patrick-5546 commented 8 months ago

@DFriend01 as the ros launch expert can you help out with this?

DFriend01 commented 8 months ago

It's not a requirement for a launch file to be registered with a package. However, I am fairly sure that it is required to have a node to be registered with a package (i.e. the integration testing node in this case).

We probably need to create a ROS package for integration tests if we want to use the ROS CLI when using this integration test framework (ros2 run, ros2 launch, etc.)

hhenry01 commented 8 months ago

It's not a requirement for a launch file to be registered with a package. However, I am fairly sure that it is required to have a node to be registered with a package (i.e. the integration testing node in this case).

We probably need to create a ROS package for integration tests if we want to use the ROS CLI when using this integration test framework (ros2 run, ros2 launch, etc.)

Because of this, I'll move the integration test files back into a separate folder from global_launch. Having it in the same folder makes global_launch too messy imo, and makes it harder to understand global_launch itself.

hhenry01 commented 8 months ago

I think it would be best to merge the PR in now as is and implement HTTP and website support in a follow up PR. It'd probably be worthwhile to present this in a team meeting as well to get opinions from website and global pathfinding. I'm also not sure how to best integrate this with the CI, but the PR is big enough as it is.

Have fun reviewing the 1100 line (excluding generated code) diff. 🙃

patrick-5546 commented 8 months ago

Yea can you present this in the first software-wide meeting back on jan 13? Do you want me to review before then?

hhenry01 commented 8 months ago

Yea can you present this in the first software-wide meeting back on jan 13? Do you want me to review before then?

If you have the time that would be beneficial so that it's more accessible for people to view.

hhenry01 commented 8 months ago

Added a diagram outlining the structure. It renders as: image