Enhance tests for 'sh' and 'bat' shells

The existing tests exercise much of the code for implementing colcon's
shell subsystem, but it doesn't validate the results. If the platform
supports the shell, we should check that the code for appending and
prepending paths to lists function as intended.

Author: cottsay

test/test_shell_bat.py

@@ -5,6 +5,7 @@
 from pathlib import Path
 import sys
 from tempfile import TemporaryDirectory
+from unittest.mock import patch
 
 from colcon_core import shell
 from colcon_core.plugin_system import SkipExtensionException
@@ -39,33 +40,36 @@ def _test_extension(prefix_path):
     extension.create_prefix_script(prefix_path, False)
     assert (prefix_path / 'local_setup.bat').exists()
 
+    # create_hook_append_value
+    append_hook_path = extension.create_hook_append_value(
+        'append_env_hook_name', prefix_path, 'pkg_name',
+        'APPEND_NAME', 'subdirectory')
+    assert append_hook_path.exists()
+    assert append_hook_path.name == 'append_env_hook_name.bat'
+    content = append_hook_path.read_text()
+    assert 'APPEND_NAME' in content
+
+    # create_hook_prepend_value
+    prepend_hook_path = extension.create_hook_prepend_value(
+        'prepend_env_hook_name', prefix_path, 'pkg_name',
+        'PREPEND_NAME', 'subdirectory')
+    assert prepend_hook_path.exists()
+    assert prepend_hook_path.name == 'prepend_env_hook_name.bat'
+    content = prepend_hook_path.read_text()
+    assert 'PREPEND_NAME' in content
+
     # create_package_script
     extension.create_package_script(
         prefix_path, 'pkg_name', [
-            ('hookA.bat', '/some/path/hookA.bat'),
+            (append_hook_path.relative_to(prefix_path), ()),
+            (prepend_hook_path.relative_to(prefix_path), ()),
             ('hookB.other', '/some/path/hookB.other')])
     assert (prefix_path / 'share' / 'pkg_name' / 'package.bat').exists()
     content = (prefix_path / 'share' / 'pkg_name' / 'package.bat').read_text()
-    assert 'hookA' in content
+    assert append_hook_path.name in content
+    assert prepend_hook_path.name in content
     assert 'hookB' not in content
 
-    # create_hook_append_value
-    hook_path = extension.create_hook_append_value(
-        'append_env_hook_name', prefix_path, 'pkg_name',
-        'APPEND_NAME', 'append_subdirectory')
-    assert hook_path.exists()
-    assert hook_path.name == 'append_env_hook_name.bat'
-    content = hook_path.read_text()
-    assert 'APPEND_NAME' in content
-
-    # create_hook_prepend_value
-    hook_path = extension.create_hook_prepend_value(
-        'env_hook_name', prefix_path, 'pkg_name', 'NAME', 'subdirectory')
-    assert hook_path.exists()
-    assert hook_path.name == 'env_hook_name.bat'
-    content = hook_path.read_text()
-    assert 'NAME' in content
-
     # generate_command_environment
     if sys.platform != 'win32':
         with pytest.raises(SkipExtensionException) as e:
@@ -93,3 +97,43 @@ def _test_extension(prefix_path):
             'task_name', prefix_path, {'dep': str(prefix_path)})
         env = run_until_complete(coroutine)
         assert isinstance(env, dict)
+
+        subdirectory_path = str(prefix_path / 'subdirectory')
+
+        # validate appending/prepending
+        with patch.dict(os.environ, {
+            'APPEND_NAME': os.pathsep.join(('', '', 'control', '', '')),
+            'PREPEND_NAME': os.pathsep.join(('', '', 'control', '', '')),
+        }):
+            coroutine = extension.generate_command_environment(
+                'task_name', prefix_path, {'pkg_name': str(prefix_path)})
+            env = run_until_complete(coroutine)
+            # Expect excess separators to be removed
+            assert env.get('APPEND_NAME') == os.pathsep.join((
+                'control',
+                subdirectory_path,
+            ))
+            # Expect excess separators to be removed
+            assert env.get('PREPEND_NAME') == os.pathsep.join((
+                subdirectory_path,
+                'control',
+            ))
+
+        # validate appending/prepending unique values
+        with patch.dict(os.environ, {
+            'APPEND_NAME': os.pathsep.join((subdirectory_path, 'control')),
+            'PREPEND_NAME': os.pathsep.join(('control', subdirectory_path)),
+        }):
+            coroutine = extension.generate_command_environment(
+                'task_name', prefix_path, {'pkg_name': str(prefix_path)})
+            env = run_until_complete(coroutine)
+            # Expect no change, value already appears earlier in the list
+            assert env.get('APPEND_NAME') == os.pathsep.join((
+                subdirectory_path,
+                'control',
+            ))
+            # Expect value to be *moved* to the front of the list
+            assert env.get('PREPEND_NAME') == os.pathsep.join((
+                subdirectory_path,
+                'control',
+            ))

test/test_shell_sh.py

@@ -5,6 +5,7 @@
 from pathlib import Path
 import sys
 from tempfile import TemporaryDirectory
+from unittest.mock import patch
 
 from colcon_core import shell
 from colcon_core.plugin_system import SkipExtensionException
@@ -40,33 +41,36 @@ def _test_extension(prefix_path):
     assert (prefix_path / 'local_setup.sh').exists()
     assert (prefix_path / '_local_setup_util_sh.py').exists()
 
+    # create_hook_append_value
+    append_hook_path = extension.create_hook_append_value(
+        'append_env_hook_name', prefix_path, 'pkg_name',
+        'APPEND_NAME', 'subdirectory')
+    assert append_hook_path.exists()
+    assert append_hook_path.name == 'append_env_hook_name.sh'
+    content = append_hook_path.read_text()
+    assert 'APPEND_NAME' in content
+
+    # create_hook_prepend_value
+    prepend_hook_path = extension.create_hook_prepend_value(
+        'prepend_env_hook_name', prefix_path, 'pkg_name',
+        'PREPEND_NAME', 'subdirectory')
+    assert prepend_hook_path.exists()
+    assert prepend_hook_path.name == 'prepend_env_hook_name.sh'
+    content = prepend_hook_path.read_text()
+    assert 'PREPEND_NAME' in content
+
     # create_package_script
     extension.create_package_script(
         prefix_path, 'pkg_name', [
-            ('hookA.sh', '/some/path/hookA.sh'),
+            (append_hook_path.relative_to(prefix_path), ()),
+            (prepend_hook_path.relative_to(prefix_path), ()),
             ('hookB.other', '/some/path/hookB.other')])
     assert (prefix_path / 'share' / 'pkg_name' / 'package.sh').exists()
     content = (prefix_path / 'share' / 'pkg_name' / 'package.sh').read_text()
-    assert 'hookA' in content
+    assert append_hook_path.name in content
+    assert prepend_hook_path.name in content
     assert 'hookB' not in content
 
-    # create_hook_append_value
-    hook_path = extension.create_hook_append_value(
-        'append_env_hook_name', prefix_path, 'pkg_name',
-        'APPEND_NAME', 'append_subdirectory')
-    assert hook_path.exists()
-    assert hook_path.name == 'append_env_hook_name.sh'
-    content = hook_path.read_text()
-    assert 'APPEND_NAME' in content
-
-    # create_hook_prepend_value
-    hook_path = extension.create_hook_prepend_value(
-        'env_hook_name', prefix_path, 'pkg_name', 'NAME', 'subdirectory')
-    assert hook_path.exists()
-    assert hook_path.name == 'env_hook_name.sh'
-    content = hook_path.read_text()
-    assert 'NAME' in content
-
     # generate_command_environment
     if sys.platform == 'win32':
         with pytest.raises(SkipExtensionException) as e:
@@ -94,3 +98,43 @@ def _test_extension(prefix_path):
             'task_name', prefix_path, {'dep': str(prefix_path)})
         env = run_until_complete(coroutine)
         assert isinstance(env, dict)
+
+        subdirectory_path = str(prefix_path / 'subdirectory')
+
+        # validate appending/prepending
+        with patch.dict(os.environ, {
+            'APPEND_NAME': os.pathsep.join(('', '', 'control', '', '')),
+            'PREPEND_NAME': os.pathsep.join(('', '', 'control', '', '')),
+        }):
+            coroutine = extension.generate_command_environment(
+                'task_name', prefix_path, {'pkg_name': str(prefix_path)})
+            env = run_until_complete(coroutine)
+            # Expect excess separators to be removed
+            assert env.get('APPEND_NAME') == os.pathsep.join((
+                'control',
+                subdirectory_path,
+            ))
+            # Expect excess separators to be removed
+            assert env.get('PREPEND_NAME') == os.pathsep.join((
+                subdirectory_path,
+                'control',
+            ))
+
+        # validate appending/prepending unique values
+        with patch.dict(os.environ, {
+            'APPEND_NAME': os.pathsep.join((subdirectory_path, 'control')),
+            'PREPEND_NAME': os.pathsep.join(('control', subdirectory_path)),
+        }):
+            coroutine = extension.generate_command_environment(
+                'task_name', prefix_path, {'pkg_name': str(prefix_path)})
+            env = run_until_complete(coroutine)
+            # Expect no change, value already appears earlier in the list
+            assert env.get('APPEND_NAME') == os.pathsep.join((
+                subdirectory_path,
+                'control',
+            ))
+            # Expect value to be *moved* to the front of the list
+            assert env.get('PREPEND_NAME') == os.pathsep.join((
+                subdirectory_path,
+                'control',
+            ))