Add runner option to ament_add_test by pbaughman · Pull Request #174 · ament/ament_cmake

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

    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 <pete.baughman@apex.ai>

- This should let us see the history Signed-off-by: Pete Baughman <pete.baughman@apex.ai>

- Adds an ament_cmake_test python package Signed-off-by: Pete Baughman <pete.baughman@apex.ai>

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 <pete.baughman@apex.ai> * ament_cmake move run_test.py to a python module - This should let us see the history Signed-off-by: Pete Baughman <pete.baughman@apex.ai> * ament_cmake refactor run_test.py into an importable python module - Adds an ament_cmake_test python package Signed-off-by: Pete Baughman <pete.baughman@apex.ai>

* 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 <pete.baughman@apex.ai> * ament_cmake move run_test.py to a python module - This should let us see the history Signed-off-by: Pete Baughman <pete.baughman@apex.ai> * ament_cmake refactor run_test.py into an importable python module - Adds an ament_cmake_test python package Signed-off-by: Pete Baughman <pete.baughman@apex.ai> Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com>

* 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 <pete.baughman@apex.ai> * ament_cmake move run_test.py to a python module - This should let us see the history Signed-off-by: Pete Baughman <pete.baughman@apex.ai> * ament_cmake refactor run_test.py into an importable python module - Adds an ament_cmake_test python package Signed-off-by: Pete Baughman <pete.baughman@apex.ai> Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> * improve handling of encoding (#181) Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com> Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> * use deterministic order for updated env vars (#196) Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com> Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> * 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 <dan@digilabs.io> Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> * remove status attribute from result XML, add skipped tag instead (#218) Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com> Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> * Declare AMENT_TEST_RESULTS_DIR as a PATH (#221) Signed-off-by: Dan Rose <dan@digilabs.io> Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> * Generate xunit files valid for the junit10.xsd Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org> Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> * Revert "Run tests in current binary directory, not global source directory (#206)" This reverts commit 4354d62. Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com> Co-authored-by: Peter Baughman <dblue135@yahoo.com> Co-authored-by: Dirk Thomas <dirk-thomas@users.noreply.github.com> Co-authored-by: Dan Rose <rotu@users.noreply.github.com> Co-authored-by: Jose Luis Rivero <jrivero@osrfoundation.org>

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 <pete.baughman@apex.ai>

ament_cmake move run_test.py to a python module

9796f5f

- This should let us see the history Signed-off-by: Pete Baughman <pete.baughman@apex.ai>

ament_cmake refactor run_test.py into an importable python module

e46066c

- Adds an ament_cmake_test python package Signed-off-by: Pete Baughman <pete.baughman@apex.ai>

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

Conversation

pbaughman commented Jun 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

A note about changing dependencies:

Uh oh!

pbaughman commented Jun 4, 2019

Uh oh!

wjwwood commented Jun 4, 2019

Uh oh!

dirk-thomas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dirk-thomas Jun 4, 2019

Choose a reason for hiding this comment

Uh oh!

pbaughman Jun 5, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pbaughman commented Jun 5, 2019

Uh oh!

pbaughman commented Jun 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pbaughman commented Jun 6, 2019

Uh oh!

Uh oh!

dirk-thomas commented Jun 6, 2019

Uh oh!

pbaughman commented Jun 6, 2019

Uh oh!

dirk-thomas commented Jun 6, 2019

Uh oh!

dirk-thomas commented Jun 6, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pbaughman commented Jun 4, 2019 •

edited

Loading

pbaughman commented Jun 5, 2019 •

edited

Loading