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.

Sign up for GitHub

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

Migrate launch tests to new launch_testing features & API #340

Merged
merged 11 commits into from
Apr 30, 2019
Merged

Migrate launch tests to new launch_testing features & API #340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
da8d032
Update after launch_testing features becoming legacy.
hidmic Mar 25, 2019
f23ba1d
Migrate test_rclcpp tests to new launch_testing API.
hidmic Mar 25, 2019
c378528
Migrate test_communication tests to new launch_testing API.
hidmic Apr 12, 2019
cf7849f
Migrate test_security tests to new launch_testing API.
hidmic Apr 12, 2019
2cf422f
Migrate test_cli_remapping tests to new launch_testing API.
hidmic Apr 12, 2019
5ecfa26
Stop using injected attributes in launch tests.
hidmic Apr 12, 2019
8847818
Bump test_rclcpp tests timeout to please CI.
hidmic Apr 15, 2019
48c02ed
Fix PATH in test_security tests.
hidmic Apr 15, 2019
6f1e9d4
Bump test_security tests timeout to please CI.
hidmic Apr 15, 2019
fb39c31
Address peer review comments.
hidmic Apr 26, 2019
797ae3b
Please flake8 on test_cli_remapping.
hidmic Apr 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions test_cli_remapping/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ if(BUILD_TESTING)

find_package(ament_cmake REQUIRED)
find_package(ament_lint_auto REQUIRED)
find_package(ament_cmake_pytest REQUIRED)
find_package(launch_testing_ament_cmake REQUIRED)
find_package(rclcpp REQUIRED)
find_package(test_msgs REQUIRED)

Expand All @@ -34,8 +34,9 @@ if(BUILD_TESTING)
"rclcpp"
"test_msgs")

ament_add_pytest_test(test_cli_remapping
add_launch_test(
test/test_cli_remapping.py
TARGET test_cli_remapping
PYTHON_EXECUTABLE "${_PYTHON_EXECUTABLE}"
ENV
NAME_MAKER_RCLCPP=$<TARGET_FILE:name_maker_rclcpp>
Expand Down
2 changes: 1 addition & 1 deletion test_cli_remapping/package.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@

<build_depend>ament_cmake</build_depend>

<test_depend>ament_cmake_pytest</test_depend>
<test_depend>ament_lint_auto</test_depend>
<test_depend>ament_lint_common</test_depend>
<test_depend>launch</test_depend>
<test_depend>launch_testing</test_depend>
<test_depend>launch_testing_ament_cmake</test_depend>
<test_depend>rclcpp</test_depend>
<test_depend>rclpy</test_depend>
<test_depend>test_msgs</test_depend>
Expand Down
264 changes: 136 additions & 128 deletions test_cli_remapping/test/test_cli_remapping.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,18 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import asyncio
import functools
import os
import random
import sys
import time

import unittest

from launch import LaunchDescription
from launch import LaunchService
from launch.actions import ExecuteProcess
from launch.actions import OpaqueCoroutine
from launch_testing import LaunchTestService
from launch.actions import OpaqueFunction
import launch_testing

import pytest
import rclpy


Expand All @@ -42,127 +40,137 @@ def get_environment_variable(name):
get_environment_variable('NAME_MAKER_RCLPY')
)


@pytest.fixture(params=CLIENT_LIBRARY_EXECUTABLES)
def node_fixture(request):
"""Create a fixture with a node, name_maker executable, and random string."""
rclpy.init()
node = rclpy.create_node('test_cli_remapping')
try:
yield {
'node': node,
'executable': request.param,
'random_string': '%d_%s' % (
random.randint(0, 9999), time.strftime('%H_%M_%S', time.gmtime()))
}
finally:
node.destroy_node()
TEST_CASES = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably out of the scope of this PR, but IMHO this migrated test looks a little difficult to read. I mean, I needed to read the comments in launch/launch_testing/example_test_context.test.py to understand what was happening here (maybe, there is better documentation somewhere).
Passing test argument in the returned test context it's not quite intuitive. Also, it would be nice to have better parameterization support.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's hard to read, I did my best to not split it up while keeping some of the unittest like structure. Alternatively, we could partially recreate what pytest does with decorators to inject the test functions like it used to be but it seemed a bit too much for a migration.

'namespace_replacement': (
'/ns/s{random_string}/relative/name',
'__ns:=/ns/s{random_string}'
),
'node_name_replacement': (
'node_{random_string}',
'__node:=node_{random_string}'
),
'topic_and_service_replacement': (
'/remapped/s{random_string}',
'/fully/qualified/name:=/remapped/s{random_string}'
),
'topic_replacement': (
'/remapped/s{random_string}',
'rostopic://~/private/name:=/remapped/s{random_string}'
),
'service_replacement': (
'/remapped/s{random_string}',
'rosservice://~/private/name:=/remapped/s{random_string}'
)
}


@launch_testing.parametrize('executable', CLIENT_LIBRARY_EXECUTABLES)
def generate_test_description(executable, ready_fn):
command = [executable]
# Execute python files using same python used to start this test
env = dict(os.environ)
if command[0][-3:] == '.py':
command.insert(0, sys.executable)
env['PYTHONUNBUFFERED'] = '1'

launch_description = LaunchDescription()

test_context = {}
for replacement_name, (replacement_value, cli_argument) in TEST_CASES.items():
random_string = '%d_%s' % (
random.randint(0, 9999), time.strftime('%H_%M_%S', time.gmtime()))
launch_description.add_action(
ExecuteProcess(
cmd=command + [cli_argument.format(**locals())],
name='name_maker_' + replacement_name, env=env
)
)
test_context[replacement_name] = replacement_value.format(**locals())

launch_description.add_action(
OpaqueFunction(function=lambda context: ready_fn())
)

return launch_description, test_context


class TestCLIRemapping(unittest.TestCase):

ATTEMPTS = 10
TIME_BETWEEN_ATTEMPTS = 1

@classmethod
def setUpClass(cls):
rclpy.init()
cls.node = rclpy.create_node('test_cli_remapping')

@classmethod
def tearDownClass(cls):
cls.node.destroy_node()
rclpy.shutdown()


def remapping_test(*, cli_args):
"""Return a decorator that returns a test function."""
def real_decorator(coroutine_test):
"""Return a test function that runs a coroutine test in a loop with a launched process."""
@functools.wraps(coroutine_test)
def test_func(node_fixture):
"""Run an executable with cli_args and coroutine test in the same asyncio loop."""
# Create a command launching a name_maker executable specified by the pytest fixture
command = [node_fixture['executable']]
# format command line arguments with random string from test fixture
for arg in cli_args:
command.append(arg.format(random_string=node_fixture['random_string']))

# Execute python files using same python used to start this test
env = dict(os.environ)
if command[0][-3:] == '.py':
command.insert(0, sys.executable)
env['PYTHONUNBUFFERED'] = '1'
ld = LaunchDescription()
launch_test = LaunchTestService()
launch_test.add_fixture_action(ld, ExecuteProcess(
cmd=command, name='name_maker_' + coroutine_test.__name__, env=env
))
launch_test.add_test_action(ld, OpaqueCoroutine(
coroutine=coroutine_test, args=[node_fixture], ignore_context=True
))
launch_service = LaunchService()
launch_service.include_launch_description(ld)
return_code = launch_test.run(launch_service)
assert return_code == 0, 'Launch failed with exit code %r' % (return_code,)
return test_func
return real_decorator


def get_topics(node_fixture):
topic_names_and_types = node_fixture['node'].get_topic_names_and_types()
return [name for name, _ in topic_names_and_types]


def get_services(node_fixture):
service_names_and_types = node_fixture['node'].get_service_names_and_types()
return [name for name, _ in service_names_and_types]


ATTEMPTS = 10
TIME_BETWEEN_ATTEMPTS = 1


@remapping_test(cli_args=('__node:=node_{random_string}',))
async def test_node_name_replacement_new(node_fixture):
node_name = 'node_{random_string}'.format(**node_fixture)

for attempt in range(ATTEMPTS):
if node_name in node_fixture['node'].get_node_names():
break
await asyncio.sleep(TIME_BETWEEN_ATTEMPTS)
rclpy.spin_once(node_fixture['node'], timeout_sec=0)
assert node_name in node_fixture['node'].get_node_names()


@remapping_test(cli_args=('__ns:=/ns/s{random_string}',))
async def test_namespace_replacement(node_fixture):
name = '/ns/s{random_string}/relative/name'.format(**node_fixture)

for attempt in range(ATTEMPTS):
if name in get_topics(node_fixture) and name in get_services(node_fixture):
break
await asyncio.sleep(TIME_BETWEEN_ATTEMPTS)
rclpy.spin_once(node_fixture['node'], timeout_sec=0)
assert name in get_topics(node_fixture) and name in get_services(node_fixture)


@remapping_test(cli_args=('/fully/qualified/name:=/remapped/s{random_string}',))
async def test_topic_and_service_replacement(node_fixture):
name = '/remapped/s{random_string}'.format(**node_fixture)

for attempt in range(ATTEMPTS):
if name in get_topics(node_fixture) and name in get_services(node_fixture):
break
await asyncio.sleep(TIME_BETWEEN_ATTEMPTS)
rclpy.spin_once(node_fixture['node'], timeout_sec=0)
assert name in get_topics(node_fixture) and name in get_services(node_fixture)


@remapping_test(cli_args=('rostopic://~/private/name:=/remapped/s{random_string}',))
async def test_topic_replacement(node_fixture):
name = '/remapped/s{random_string}'.format(**node_fixture)

for attempt in range(ATTEMPTS):
if name in get_topics(node_fixture):
break
await asyncio.sleep(TIME_BETWEEN_ATTEMPTS)
rclpy.spin_once(node_fixture['node'], timeout_sec=0)
assert name in get_topics(node_fixture) and name not in get_services(node_fixture)


@remapping_test(cli_args=('rosservice://~/private/name:=/remapped/s{random_string}',))
async def test_service_replacement(node_fixture):
name = '/remapped/s{random_string}'.format(**node_fixture)

for attempt in range(ATTEMPTS):
if name in get_services(node_fixture):
break
await asyncio.sleep(TIME_BETWEEN_ATTEMPTS)
rclpy.spin_once(node_fixture['node'], timeout_sec=0)
assert name not in get_topics(node_fixture) and name in get_services(node_fixture)
def get_topics(self):
topic_names_and_types = self.node.get_topic_names_and_types()
return [name for name, _ in topic_names_and_types]

def get_services(self):
service_names_and_types = self.node.get_service_names_and_types()
return [name for name, _ in service_names_and_types]

def test_namespace_replacement(self, namespace_replacement):
for attempt in range(self.ATTEMPTS):
if (
namespace_replacement in self.get_topics() and
namespace_replacement in self.get_services()
):
break
time.sleep(self.TIME_BETWEEN_ATTEMPTS)
rclpy.spin_once(self.node, timeout_sec=0)
self.assertIn(namespace_replacement, self.get_topics())
self.assertIn(namespace_replacement, self.get_services())

def test_node_name_replacement(self, node_name_replacement):
for attempt in range(self.ATTEMPTS):
if node_name_replacement in self.node.get_node_names():
break
time.sleep(self.TIME_BETWEEN_ATTEMPTS)
rclpy.spin_once(self.node, timeout_sec=0)
self.assertIn(node_name_replacement, self.node.get_node_names())

def test_topic_and_service_replacement(self, topic_and_service_replacement):
for attempt in range(self.ATTEMPTS):
if (
topic_and_service_replacement in self.get_topics() and
topic_and_service_replacement in self.get_services()
):
break
time.sleep(self.TIME_BETWEEN_ATTEMPTS)
rclpy.spin_once(self.node, timeout_sec=0)
self.assertIn(topic_and_service_replacement, self.get_topics())
self.assertIn(topic_and_service_replacement, self.get_services())

def test_topic_replacement(self, topic_replacement):
for attempt in range(self.ATTEMPTS):
if topic_replacement in self.get_topics():
break
time.sleep(self.TIME_BETWEEN_ATTEMPTS)
rclpy.spin_once(self.node, timeout_sec=0)
self.assertIn(topic_replacement, self.get_topics())
self.assertNotIn(topic_replacement, self.get_services())

def test_service_replacement(self, service_replacement):
for attempt in range(self.ATTEMPTS):
if service_replacement in self.get_services():
break
time.sleep(self.TIME_BETWEEN_ATTEMPTS)
rclpy.spin_once(self.node, timeout_sec=0)
self.assertNotIn(service_replacement, self.get_topics())
self.assertIn(service_replacement, self.get_services())


@launch_testing.post_shutdown_test()
class TestCLIRemappingAfterShutdown(unittest.TestCase):

def test_processes_finished_gracefully(self, proc_info):
"""Test that both executables finished gracefully."""
launch_testing.asserts.assertExitCodes(proc_info)
11 changes: 7 additions & 4 deletions test_communication/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ if(BUILD_TESTING)
find_package(ament_lint_auto REQUIRED)
ament_lint_auto_find_test_dependencies()

find_package(ament_cmake_pytest REQUIRED)
find_package(launch_testing_ament_cmake REQUIRED)

# Provides PYTHON_EXECUTABLE_DEBUG
find_package(python_cmake_module REQUIRED)
Expand Down Expand Up @@ -187,8 +187,9 @@ if(BUILD_TESTING)

list(LENGTH TEST_MESSAGE_TYPES length)
math(EXPR timeout "${length} * 15")
ament_add_pytest_test(test_publisher_subscriber${suffix}
add_launch_test(
"${CMAKE_CURRENT_BINARY_DIR}/test_publisher_subscriber${suffix}_$<CONFIG>.py"
TARGET test_publisher_subscriber${suffix}
PYTHON_EXECUTABLE "${_PYTHON_EXECUTABLE}"
APPEND_LIBRARY_DIRS "${append_library_dirs}"
TIMEOUT ${timeout}
Expand Down Expand Up @@ -237,8 +238,9 @@ if(BUILD_TESTING)

list(LENGTH TEST_SERVICE_TYPES length)
math(EXPR timeout "${length} * 30")
ament_add_pytest_test(test_requester_replier${suffix}
add_launch_test(
"${CMAKE_CURRENT_BINARY_DIR}/test_requester_replier${suffix}_$<CONFIG>.py"
TARGET test_requester_replier${suffix}
PYTHON_EXECUTABLE "${_PYTHON_EXECUTABLE}"
APPEND_LIBRARY_DIRS "${append_library_dirs}"
TIMEOUT ${timeout}
Expand Down Expand Up @@ -274,8 +276,9 @@ if(BUILD_TESTING)

list(LENGTH TEST_ACTION_TYPES length)
math(EXPR timeout "${length} * 30")
ament_add_pytest_test(test_action_client_server${suffix}
add_launch_test(
"${CMAKE_CURRENT_BINARY_DIR}/test_action_client_server${suffix}_$<CONFIG>.py"
TARGET test_action_client_server${suffix}
PYTHON_EXECUTABLE "${_PYTHON_EXECUTABLE}"
APPEND_LIBRARY_DIRS "${append_library_dirs}"
TIMEOUT ${timeout}
Expand Down
2 changes: 1 addition & 1 deletion test_communication/package.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@

<test_depend>ament_cmake</test_depend>
<test_depend>ament_cmake_gtest</test_depend>
<test_depend>ament_cmake_pytest</test_depend>
<test_depend>ament_lint_auto</test_depend>
<test_depend>ament_lint_common</test_depend>
<test_depend>launch</test_depend>
<test_depend>launch_testing</test_depend>
<test_depend>launch_testing_ament_cmake</test_depend>
<test_depend>osrf_testing_tools_cpp</test_depend>
<test_depend>rcl</test_depend>
<test_depend>rclcpp</test_depend>
Expand Down
Loading