Enhance tests for 'sh' and 'bat' shells

colcon_core/shell/template/prefix_util.py.em

@@ -377,7 +377,7 @@ def _remove_ending_separators():
     commands = []
     for name in env_state:
         # skip variables that already had values before this script started prepending
-        if name in os.environ:
+        if os.environ.get(name):
             continue
         commands += [
             FORMAT_STR_REMOVE_LEADING_SEPARATOR.format_map({'name': name}),

test/test_shell_bat.py

@@ -5,10 +5,14 @@
 from pathlib import Path
 import sys
 from tempfile import TemporaryDirectory
+from unittest.mock import patch
 
 from colcon_core import shell
+from colcon_core.location import get_relative_package_index_path
 from colcon_core.plugin_system import SkipExtensionException
+from colcon_core.shell import get_environment_variables
 from colcon_core.shell.bat import BatShell
+from colcon_core.shell.dsv import DsvShell
 import pytest
 
 from .run_until_complete import run_until_complete
@@ -20,6 +24,7 @@ def test_extension():
     try:
         with TemporaryDirectory(prefix='test_colcon_') as prefix_path:
             _test_extension(Path(prefix_path))
+            _test_prefix_script(Path(prefix_path))
     finally:
         shell.use_all_shell_extensions = use_all_shell_extensions
 
@@ -39,33 +44,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 +101,145 @@ 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 without existing values
+        with patch.dict(os.environ) as env_patch:
+            env_patch.pop('APPEND_NAME', None)
+            env_patch.pop('PREPEND_NAME', None)
+            coroutine = extension.generate_command_environment(
+                'task_name', prefix_path, {'pkg_name': str(prefix_path)})
+            env = run_until_complete(coroutine)
+            assert env.get('APPEND_NAME') == subdirectory_path
+            assert env.get('PREPEND_NAME') == subdirectory_path
+
+        # validate appending/prepending with existing values
+        with patch.dict(os.environ, {
+            'APPEND_NAME': 'control',
+            'PREPEND_NAME': 'control',
+        }):
+            coroutine = extension.generate_command_environment(
+                'task_name', prefix_path, {'pkg_name': str(prefix_path)})
+            env = run_until_complete(coroutine)
+            assert env.get('APPEND_NAME') == os.pathsep.join((
+                'control',
+                subdirectory_path,
+            ))
+            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',
+            ))
+
+
+def _test_prefix_script(prefix_path):
+    extension = BatShell()
+
+    # BatShell requires the (non-primary) DsvShell extension as well
+    dsv_extension = DsvShell()
+
+    # create_hook_append_value
+    append_hook_path = dsv_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.dsv'
+    content = append_hook_path.read_text()
+    assert 'APPEND_NAME' in content
+
+    # create_hook_prepend_value
+    prepend_hook_path = dsv_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.dsv'
+    content = prepend_hook_path.read_text()
+    assert 'PREPEND_NAME' in content
+
+    # mark package as installed
+    package_index = prefix_path / get_relative_package_index_path()
+    package_index.mkdir(parents=True)
+    (package_index / 'pkg_name').write_text('')
+
+    # create_package_script
+    dsv_extension.create_package_script(
+        prefix_path, 'pkg_name', [
+            (append_hook_path.relative_to(prefix_path), ()),
+            (prepend_hook_path.relative_to(prefix_path), ())])
+    assert (prefix_path / 'share' / 'pkg_name' / 'package.dsv').exists()
+    content = (prefix_path / 'share' / 'pkg_name' / 'package.dsv').read_text()
+    assert append_hook_path.name in content
+    assert prepend_hook_path.name in content
+
+    # create_prefix_script
+    extension.create_prefix_script(prefix_path, True)
+    prefix_script = prefix_path / 'local_setup.bat'
+    prefix_script.exists()
+    assert (prefix_path / '_local_setup_util_bat.py').exists()
+
+    if sys.platform == 'win32':
+        subdirectory_path = str(prefix_path / 'subdirectory')
+
+        # validate appending/prepending without existing values
+        with patch.dict(os.environ, {
+            'APPEND_NAME': '',
+            'PREPEND_NAME': '',
+        }):
+            coroutine = get_environment_variables([prefix_script, '&&', 'set'])
+            env = run_until_complete(coroutine)
+            assert env.get('APPEND_NAME') == subdirectory_path
+            assert env.get('PREPEND_NAME') == subdirectory_path
+
+        # validate appending/prepending with existing values
+        with patch.dict(os.environ, {
+            'APPEND_NAME': 'control',
+            'PREPEND_NAME': 'control',
+        }):
+            coroutine = get_environment_variables([prefix_script, '&&', 'set'])
+            env = run_until_complete(coroutine)
+            assert env.get('APPEND_NAME') == os.pathsep.join((
+                'control',
+                subdirectory_path,
+            ))
+            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 = get_environment_variables([prefix_script, '&&', 'set'])
+            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',
+            ))
+            # TODO: The DsvShell behavior doesn't align with BatShell!
+            # ~~Expect value to be *moved* to the front of the list~~
+            assert env.get('PREPEND_NAME') == os.pathsep.join((
+                'control',
+                subdirectory_path,
+            ))