Use --isolated if no ROS_DOMAIN_ID is set to help parallel testing (r…

…os2#251)

* Make auto selected domain ID show up in test log

Signed-off-by: Pete Baughman <pete.baughman@apex.ai>

* Use deterministic selection for domain isolation

  - On my linux machine running the ROS nightly docker image, it makes domain
    selection take about 1ms on average when coordinating 50 isolated domains
  - If there's another platform where opening sockets is significantly slower, the
    old random seed code is still here, but commented out

Signed-off-by: Pete Baughman <pete.baughman@apex.ai>

* Put logic to disable isolation in launch_test

  - This respects the value of ROS_DOMAIN_ID that's set when the test is run.
  - Setting this in launch_testing_ament_cmake used the value at build time, which was unintuitive

Signed-off-by: Pete Baughman <pete.baughman@apex.ai>

* update description

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

* combine conditions

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

launch_testing/README.md

@@ -159,6 +159,19 @@ launch_test examples/args.test.py dut_arg:=value
 
 See the [launch_testing example with arguments](examples/args.test.py) for further reference.
 
+## ROS_DOMAIN_ID Isolation
+
+If the ROS_DOMAIN_ID environment variable isn't set, `launch_test` will automatically coordinate with other `launch_test` processes running on the same host
+to use a unique ROS_DOMAIN_ID for the launched processes.
+This allows multiple instances to run in parallel (the default with `colcon test`).
+Note that `launch_test` cannot coordinate unique domains across multiple hosts.  
+If the ROS_DOMAIN_ID environment variable is already set, `launch_test` respects the environment variable and won't attempt to select a different ID.
+In this case it's the responsibility of the user to design tests that can be safely run in parallel, or not use parallel test workers.
+
+When working on a system without a ROS_DOMAIN_ID set, the automatic domain isolation behavior can be disabled with the --disable-isolation flag.
+This can be useful for debugging tests by running without isolation and running a command like `ros2 topic echo` in another terminal window to see what's
+happening in the test as it runs.
+
 ## Using CMake
 
 To run launch tests from a CMakeLists.txt file, you'll need to declare a dependency on

launch_testing/launch_testing/domain_coordinator.py

@@ -12,7 +12,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import random
 import socket
 
 # To coordinate ROS_DOMAIN_IDs between multiple instances of launch_test, we
@@ -36,7 +35,17 @@ class _default_selector:
     # value, then increment from there until we find one that isn't taken
 
     def __init__(self):
-        self._value = random.randint(1, 100)
+        # When we need to coordinate 10 or 20 domains, it's about 10x faster
+        # to start with a random seed value here.  It's also the difference between
+        # 1ms and 100us so it's totally insignificant
+        # Leaving this here in case it's ever useful in the future to speed up
+        # domain selection
+        # self._value = random.randint(1, 100)
+
+        # Slower, but deterministic:
+        # Always start at '1' so if there's weirdness where domains are colliding when
+        # they shouldn't, it's easier to debug
+        self._value = 1
 
     def __call__(self):
         retval = ((self._value - 1) % 100) + 1

launch_testing/launch_testing/launch_test.py

@@ -50,8 +50,8 @@ def add_arguments(parser):
     )
     # TODO(hidmic): Provide this option for rostests only.
     parser.add_argument(
-        '-i', '--isolated', action='store_true', default=False,
-        help='Isolate tests using a custom ROS_DOMAIN_ID. Useful for test parallelization.'
+        '--disable-isolation', action='store_true', default=False,
+        help='Disable automatic ROS_DOMAIN_ID isolation.'
     )
     parser.add_argument(
         'launch_arguments', nargs='*',
@@ -65,16 +65,24 @@ def add_arguments(parser):
 
 def parse_arguments():
     parser = argparse.ArgumentParser(
-        description='Launch integration testing tool.'
+        description='Launch integration testing tool. Uses a unique domain id '
+                    "if the environment variable ROS_DOMAIN_ID isn't set."
     )
     add_arguments(parser)
     return parser, parser.parse_args()
 
 
 def run(parser, args, test_runner_cls=LaunchTestRunner):
-    if args.isolated:
-        domain_id = get_coordinated_domain_id()  # Must copy this to a local to keep it alive
-        _logger_.debug('Running with ROS_DOMAIN_ID {}'.format(domain_id))
+
+    # If ROS_DOMAIN_ID is already set, launch_test will respect that domain ID and use it.
+    # If ROS_DOMAIN_ID is not set, launch_test will pick a ROS_DOMAIN_ID that's not being used
+    # by another launch_test process.
+    # This is to allow launch_test to run in parallel and not have ROS cross-talk.
+    # If the user needs to debug a test and they don't have ROS_DOMAIN_ID set in their environment
+    # they can disable isolation by passing the --disable-isolation flag.
+    if 'ROS_DOMAIN_ID' not in os.environ and not args.disable_isolation:
+        domain_id = get_coordinated_domain_id()  # Must keep this as a local to keep it alive
+        _logger_.info('Running with ROS_DOMAIN_ID {}'.format(domain_id))
         os.environ['ROS_DOMAIN_ID'] = str(domain_id)
 
     # Load the test file as a module and make sure it has the required