Add runner option to ament_add_test #174

pbaughman · 2019-06-04T23:08:32Z

Allow the user of ament_add_gtest and ament_add_pytest_test to specify a different test runner. This will be used by ROS2 tests so they can optionally specify a runner that provides isolation between tests running in parallel

(resolves #172)

A note about changing dependencies:

Previously, the dependencies for ament_cmake_test looked like this:

$ colcon list --topological-graph --packages-up-to ament_cmake_test
ament_package     +*.                                                                                          
ament_cmake_core   +*                                                                                          
ament_cmake_test    +

In this PR, I changed run_test.py from a python script to a python script that imports most of its functionality from a new python library called ament_add_test. This required me to change the dependencies of ament_cmake_test to depend on ament_cmake_python. Now the deps look like this:

$ colcon list --topological-graph --packages-up-to ament_cmake_test
ament_package       +*..                                                                                          
ament_cmake_core     +**                                                                                          
ament_cmake_python    +*                                                                                          
ament_cmake_test       +

As you can see, this did not cause many other indirect dependencies. Since ament_cmake_test relied on python anyway (to execute run_test.py) I think this is OK.

pbaughman · 2019-06-04T23:10:51Z

@tfoote This is the first of two PRs required to implement isolated testing for ROS2. This change is ROS agnostic, but will allow us to have wrappers around ament_add_pytest_test and ament_add_gtest that set up the test isolation

@wjwwood This is the approach we discussed last week that keeps ROS stuff out of ament. I will create a follow-up PR in ament_cmake_ros with the aforementioned wrappers.

wjwwood · 2019-06-04T23:25:41Z

@dirk-thomas can you comment on the change in the dependency structure due to this change? Is it ok or is there a better way to do this to avoid that dependency?

dirk-thomas

can you comment on the change in the dependency structure due to this change?

The added dependency looks reasonable to me.

ament_cmake_gtest/cmake/ament_add_gtest.cmake

ament_cmake_gtest/cmake/ament_add_gtest_test.cmake

dirk-thomas · 2019-06-04T23:58:11Z

ament_cmake_gtest/cmake/ament_add_gtest_test.cmake

@@ -83,6 +83,7 @@ function(ament_add_gtest_test target)
    COMMAND ${cmd}
    OUTPUT_FILE "${CMAKE_BINARY_DIR}/ament_cmake_gtest/${target}.txt"
    RESULT_FILE "${result_file}"
+    RUNNER "${ARG_RUNNER}"


Should this not be added conditionally only - same as in ament_add_gtest?

I'll make it match the style of the other arguments. Note that ament_add_gtest does things differently than ament_add_gtest_test and ament_add_pytest_test even before my change

ament_cmake_pytest/cmake/ament_add_pytest_test.cmake

ament_cmake_test/ament_cmake_test/__init__.py

ament_cmake_test/cmake/ament_add_test.cmake

pbaughman · 2019-06-05T17:07:27Z

@dirk-thomas Ok, I think the history is fixed now. Git blame on github, and on my machine now shows you and William wrote most of the code of ament_cmake_test/__init__.py 4 years ago.

pbaughman · 2019-06-05T18:41:35Z

@dirk-thomas Ok, I think this is ready to go. I believe I've addressed all of your comments and I tested it along with ros2/ament_cmake_ros#3 and verified that rcl and rclpy can use the new 'isolated' version of the test functions successfully

ament_cmake_gtest/cmake/ament_add_gtest.cmake

ament_cmake_test/cmake/ament_add_test.cmake

pbaughman · 2019-06-06T16:14:53Z

@dirk-thomas 👍

ament_cmake_gtest/cmake/ament_add_gtest_test.cmake

dirk-thomas · 2019-06-06T16:17:41Z

Since we don't want to squash on merge can you please squash the commit 4 (and soon 5) into the first commit. Thanks.

- By default, still uses run_test.py - Example use case: ament_cmake_ros can use a test runner that sets a ROS_DOMAIN_ID Signed-off-by: Pete Baughman <[email protected]>

- This should let us see the history Signed-off-by: Pete Baughman <[email protected]>

- Adds an ament_cmake_test python package Signed-off-by: Pete Baughman <[email protected]>

pbaughman · 2019-06-06T16:38:28Z

@dirk-thomas Done

dirk-thomas · 2019-06-06T16:46:47Z

Sanity check to rule out regressions:

dirk-thomas · 2019-06-06T18:08:02Z

Thanks for the enhancement.

* ament_cmake allow speficiation of a different test runner - By default, still uses run_test.py - Example use case: ament_cmake_ros can use a test runner that sets a ROS_DOMAIN_ID Signed-off-by: Pete Baughman <[email protected]> * ament_cmake move run_test.py to a python module - This should let us see the history Signed-off-by: Pete Baughman <[email protected]> * ament_cmake refactor run_test.py into an importable python module - Adds an ament_cmake_test python package Signed-off-by: Pete Baughman <[email protected]>

* ament_cmake allow speficiation of a different test runner - By default, still uses run_test.py - Example use case: ament_cmake_ros can use a test runner that sets a ROS_DOMAIN_ID Signed-off-by: Pete Baughman <[email protected]> * ament_cmake move run_test.py to a python module - This should let us see the history Signed-off-by: Pete Baughman <[email protected]> * ament_cmake refactor run_test.py into an importable python module - Adds an ament_cmake_test python package Signed-off-by: Pete Baughman <[email protected]> Signed-off-by: Steven! Ragnarök <[email protected]>

* Add runner option to ament_add_test (#174) * ament_cmake allow speficiation of a different test runner - By default, still uses run_test.py - Example use case: ament_cmake_ros can use a test runner that sets a ROS_DOMAIN_ID Signed-off-by: Pete Baughman <[email protected]> * ament_cmake move run_test.py to a python module - This should let us see the history Signed-off-by: Pete Baughman <[email protected]> * ament_cmake refactor run_test.py into an importable python module - Adds an ament_cmake_test python package Signed-off-by: Pete Baughman <[email protected]> Signed-off-by: Steven! Ragnarök <[email protected]> * improve handling of encoding (#181) Signed-off-by: Dirk Thomas <[email protected]> Signed-off-by: Steven! Ragnarök <[email protected]> * use deterministic order for updated env vars (#196) Signed-off-by: Dirk Thomas <[email protected]> Signed-off-by: Steven! Ragnarök <[email protected]> * Run tests in current binary directory, not global source directory (#206) Switch to CMAKE_CURRENT_BINARY_DIR for consistency with CTest Signed-off-by: Dan Rose <[email protected]> Signed-off-by: Steven! Ragnarök <[email protected]> * remove status attribute from result XML, add skipped tag instead (#218) Signed-off-by: Dirk Thomas <[email protected]> Signed-off-by: Steven! Ragnarök <[email protected]> * Declare AMENT_TEST_RESULTS_DIR as a PATH (#221) Signed-off-by: Dan Rose <[email protected]> Signed-off-by: Steven! Ragnarök <[email protected]> * Generate xunit files valid for the junit10.xsd Signed-off-by: Jose Luis Rivero <[email protected]> Signed-off-by: Steven! Ragnarök <[email protected]> * Revert "Run tests in current binary directory, not global source directory (#206)" This reverts commit 4354d62. Signed-off-by: Steven! Ragnarök <[email protected]> Co-authored-by: Peter Baughman <[email protected]> Co-authored-by: Dirk Thomas <[email protected]> Co-authored-by: Dan Rose <[email protected]> Co-authored-by: Jose Luis Rivero <[email protected]>

wjwwood requested a review from dirk-thomas June 4, 2019 23:24

dirk-thomas reviewed Jun 5, 2019

View reviewed changes

dirk-thomas added enhancement New feature or request in progress Actively being worked on (Kanban column) requires-changes labels Jun 5, 2019

pbaughman force-pushed the add_runner_option branch 2 times, most recently from f0dc669 to 5115416 Compare June 5, 2019 17:04

This was referenced Jun 5, 2019

Move --disable-isolation argument from launch_testing to ros2test ros2/ros_testing#2

Merged

Add ament_add_pytest_isolated and ament_add_gtest_isolated ros2/ament_cmake_ros#3

Merged

pbaughman mentioned this pull request Jun 5, 2019

I would like to extend run_test.py to provide ROS_DOMAIN_ID isolation #172

Closed

dirk-thomas reviewed Jun 6, 2019

View reviewed changes

ament_cmake_gtest/cmake/ament_add_gtest.cmake Outdated Show resolved Hide resolved

ament_cmake_test/cmake/ament_add_test.cmake Outdated Show resolved Hide resolved

dirk-thomas reviewed Jun 6, 2019

View reviewed changes

ament_cmake_gtest/cmake/ament_add_gtest_test.cmake Outdated Show resolved Hide resolved

Pete Baughman added 3 commits June 6, 2019 09:35

ament_cmake allow speficiation of a different test runner

bf4e156

- By default, still uses run_test.py - Example use case: ament_cmake_ros can use a test runner that sets a ROS_DOMAIN_ID Signed-off-by: Pete Baughman <[email protected]>

ament_cmake move run_test.py to a python module

9796f5f

- This should let us see the history Signed-off-by: Pete Baughman <[email protected]>

ament_cmake refactor run_test.py into an importable python module

e46066c

- Adds an ament_cmake_test python package Signed-off-by: Pete Baughman <[email protected]>

pbaughman force-pushed the add_runner_option branch from 5e8b15b to e46066c Compare June 6, 2019 16:35

dirk-thomas removed the requires-changes label Jun 6, 2019

dirk-thomas merged commit 5918ede into ament:master Jun 6, 2019

dirk-thomas removed the in progress Actively being worked on (Kanban column) label Jun 6, 2019

pbaughman deleted the add_runner_option branch June 6, 2019 18:08

pbaughman mentioned this pull request Jun 10, 2019

Add 'runner' option to ament_add_gmock / nose #177

Merged

nuclearsandwich mentioned this pull request Mar 23, 2020

[Dashing] Backport junit xml fixes for updated xunit plugin on ci.ros2.org #230

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add runner option to ament_add_test #174

Add runner option to ament_add_test #174

pbaughman commented Jun 4, 2019 •

edited

Loading

pbaughman commented Jun 4, 2019

wjwwood commented Jun 4, 2019

dirk-thomas left a comment

dirk-thomas Jun 4, 2019

pbaughman Jun 5, 2019

pbaughman commented Jun 5, 2019

pbaughman commented Jun 5, 2019 •

edited

Loading

pbaughman commented Jun 6, 2019

dirk-thomas commented Jun 6, 2019

pbaughman commented Jun 6, 2019

dirk-thomas commented Jun 6, 2019

dirk-thomas commented Jun 6, 2019

Add runner option to ament_add_test #174

Add runner option to ament_add_test #174

Conversation

pbaughman commented Jun 4, 2019 • edited Loading

A note about changing dependencies:

pbaughman commented Jun 4, 2019

wjwwood commented Jun 4, 2019

dirk-thomas left a comment

Choose a reason for hiding this comment

dirk-thomas Jun 4, 2019

Choose a reason for hiding this comment

pbaughman Jun 5, 2019

Choose a reason for hiding this comment

pbaughman commented Jun 5, 2019

pbaughman commented Jun 5, 2019 • edited Loading

pbaughman commented Jun 6, 2019

dirk-thomas commented Jun 6, 2019

pbaughman commented Jun 6, 2019

dirk-thomas commented Jun 6, 2019

dirk-thomas commented Jun 6, 2019

pbaughman commented Jun 4, 2019 •

edited

Loading

pbaughman commented Jun 5, 2019 •

edited

Loading