Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unit tests for composition actions #15

Open
wants to merge 3 commits into
base: rolling
Choose a base branch
from

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Apr 16, 2019

This is the start of unit tests for composition. It depends on #8, so if you'd like to run locally make a branch that merges this branch and #8 .

Issues to resolve:

  1. How to make launch_test test_launch_ros/test/rostest/composition.test.py part of the test suite in a pure python package?
  2. Add log_level to ComposableNodeDescription
  3. Edit: resolved by Move OnProcessStart from LoadComposableNodes to ComposableNodeContainer #16 LaunchComposableNodes returns actions to be executed when the container emits the event OnProcessStart, but that means they won't trigger if the container process is already started. At the moment composable nodes launching only works via the composable_node_descriptions on the container action
  4. How to handle failures to launch nodes? Should emit an event so other launch actions can do something if a node fails to launch
  5. How to unload nodes? Should emit an event with the node unique_id so it's possible to make an unload action

@hidmic
Copy link
Contributor

hidmic commented Apr 30, 2019

@sloretz #8 just got merged, mind to check if this is still working and put it on review (or not)?

@sloretz sloretz force-pushed the sloretz/unit_test_composition branch from 60c4d37 to 1c009e2 Compare April 30, 2019 23:56
@sloretz
Copy link
Contributor Author

sloretz commented May 1, 2019

@hidmic The test passes running with:

launch_test src/ros2/launch_ros/test_launch_ros/test/rostest/composition.test.py

This isn't ready to be merged as the test is not run with colcon test --packages-select test_launch_ros. I'm not sure how to make a launch test part of the test suite for a pure python package. Is there a pytest plugin for launch-test? If not, a quick alternative would be a test file that called launch_test using subprocess.

@hidmic
Copy link
Contributor

hidmic commented May 1, 2019

That's a good point, we don't have a tool for that atm. I've filed an issue ros2/launch#237.

@hidmic
Copy link
Contributor

hidmic commented May 15, 2019

@sloretz with ros2/launch#236 now we can call launch_test from Python code to some extent.

@hidmic
Copy link
Contributor

hidmic commented Aug 20, 2019

@sloretz once ros2/launch#312 goes in, we can have launch tests in pure Python packages.

@jacobperron
Copy link
Member

@wjwwood wjwwood force-pushed the sloretz/unit_test_composition branch from 1c009e2 to 5e9592a Compare January 18, 2020 00:47
@wjwwood
Copy link
Member

wjwwood commented Jan 18, 2020

@sloretz I rebased this and I'm going to try and resurrect it in general, hope you don't mind.

@hidmic
Copy link
Contributor

hidmic commented Apr 4, 2022

@wjwwood friendly ping

@wjwwood
Copy link
Member

wjwwood commented Apr 4, 2022

I do not remember what I was going to do to "resurrect" these tests...

I'll re-review it, but I'm afraid after dropping the ball on this I've lost context.

@wjwwood
Copy link
Member

wjwwood commented Apr 4, 2022

I'm guessing it needs to be updated to use the "launch tests from python packages" updates and/or changed to not use ready_fn in favor of pytest.

@audrow audrow changed the base branch from master to rolling June 28, 2022 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants