Add support for not optional environment variable substitution (ros2#288

)

* Add support for non optional environment variable substitution

Signed-off-by: ivanpauno <[email protected]>

* Address per review comments

Signed-off-by: ivanpauno <[email protected]>

* Correct test in launch_xml

Signed-off-by: ivanpauno <[email protected]>

launch/launch/substitutions/environment_variable.py

@@ -17,8 +17,10 @@
 import os
 from typing import Iterable
 from typing import List
+from typing import Optional
 from typing import Text
 
+from .substitution_failure import SubstitutionFailure
 from ..frontend.expose import expose_substitution
 from ..launch_context import LaunchContext
 from ..some_substitutions_type import SomeSubstitutionsType
@@ -37,14 +39,24 @@ def __init__(
         self,
         name: SomeSubstitutionsType,
         *,
-        default_value: SomeSubstitutionsType = ''
+        default_value: Optional[SomeSubstitutionsType] = None
     ) -> None:
-        """Constructor."""
+        """
+        Construct an enviroment variable substitution.
+
+        :param name: name of the environment variable.
+        :param default_value: used when the environment variable doesn't exist.
+            If `None`, the substitution is not optional.
+        :raise `SubstitutionFailure`:
+            If the environment variable doesn't exist and `default_value` is `None`.
+        """
         super().__init__()
 
         from ..utilities import normalize_to_list_of_substitutions  # import here to avoid loop
         self.__name = normalize_to_list_of_substitutions(name)
-        self.__default_value = normalize_to_list_of_substitutions(default_value)
+        if default_value is not None:
+            default_value = normalize_to_list_of_substitutions(default_value)
+        self.__default_value = default_value
 
     @classmethod
     def parse(cls, data: Iterable[SomeSubstitutionsType]):
@@ -73,7 +85,16 @@ def describe(self) -> Text:
     def perform(self, context: LaunchContext) -> Text:
         """Perform the substitution by looking up the environment variable."""
         from ..utilities import perform_substitutions  # import here to avoid loop
-        return os.environ.get(
-            perform_substitutions(context, self.name),
-            perform_substitutions(context, self.default_value)
+        default_value = self.default_value
+        if default_value is not None:
+            default_value = perform_substitutions(context, self.default_value)
+        name = perform_substitutions(context, self.name)
+        value = os.environ.get(
+            name,
+            default_value
         )
+        if value is None:
+            raise SubstitutionFailure(
+                "environment variable '{}' does not exist".format(name)
+            )
+        return value

launch/test/launch/actions/test_set_and_unset_environment_variable.py

@@ -69,5 +69,5 @@ def test_unset_nonexistent_key():
     lc1 = LaunchContext()
 
     assert os.environ.get('ANOTHER_NONEXISTENT_KEY') is None
-    UnsetEnvironmentVariable(EnvironmentVariable('NONEXISTENT_KEY')).visit(lc1)
+    UnsetEnvironmentVariable('ANOTHER_NONEXISTENT_KEY').visit(lc1)
     assert os.environ.get('ANOTHER_NONEXISTENT_KEY') is None

launch/test/launch/frontend/test_substitutions.py

@@ -100,7 +100,7 @@ def test_quoted_nested_substitution():
     assert subst[2].perform(None) == '_of_'
     assert subst[3].name[0].perform(None) == 'something '
     assert subst[3].name[1].perform(None) == '10'
-    assert subst[3].default_value[0].perform(None) == ''
+    assert subst[3].default_value is None
 
 
 def test_double_quoted_nested_substitution():
@@ -114,7 +114,7 @@ def test_double_quoted_nested_substitution():
     assert subst[0].name[1].perform(context) == '"asd_bds"'
     assert len(subst[0].default_value) == 2
     assert subst[0].default_value[0].name[0].perform(context) == 'DEFAULT'
-    assert subst[0].default_value[0].default_value[0].perform(context) == ''
+    assert subst[0].default_value[0].default_value is None
     assert subst[0].default_value[1].perform(context) == '_qsd'
 
 
@@ -129,7 +129,7 @@ def test_combining_quotes_nested_substitution():
     assert subst[0].name[1].perform(context) == "'asd_bds'"
     assert len(subst[0].default_value) == 2
     assert subst[0].default_value[0].name[0].perform(context) == 'DEFAULT'
-    assert subst[0].default_value[0].default_value[0].perform(context) == ''
+    assert subst[0].default_value[0].default_value is None
     assert subst[0].default_value[1].perform(context) == '_qsd'
 
 
@@ -146,12 +146,18 @@ def test_env_subst():
     assert isinstance(env, EnvironmentVariable)
     assert 'asd' == ''.join([x.perform(None) for x in env.name])
     assert 'bsd' == ''.join([x.perform(None) for x in env.default_value])
-    subst = parse_substitution('$(env asd)')
+    subst = parse_substitution("$(env asd '')")
     assert len(subst) == 1
     env = subst[0]
     assert isinstance(env, EnvironmentVariable)
     assert 'asd' == ''.join([x.perform(None) for x in env.name])
     assert '' == ''.join([x.perform(None) for x in env.default_value])
+    subst = parse_substitution('$(env asd)')
+    assert len(subst) == 1
+    env = subst[0]
+    assert isinstance(env, EnvironmentVariable)
+    assert 'asd' == ''.join([x.perform(None) for x in env.name])
+    assert env.default_value is None
 
 
 def test_eval_subst():

launch/test/launch/substitutions/test_environment_variable.py

@@ -0,0 +1,46 @@
+# Copyright 2019 Open Source Robotics Foundation, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Tests for the EnvironmentVariable substitution class."""
+
+import os
+
+from launch import LaunchContext
+from launch.actions import SetEnvironmentVariable
+from launch.substitutions import EnvironmentVariable
+from launch.substitutions import SubstitutionFailure
+
+import pytest
+
+
+def test_this_launch_file_path():
+    if 'MY_ENVIRONMENT_VARIABLE' in os.environ:
+        del os.environ['MY_ENVIRONMENT_VARIABLE']
+    lc = LaunchContext()
+    sub1 = EnvironmentVariable('MY_ENVIRONMENT_VARIABLE')
+    with pytest.raises(SubstitutionFailure) as ex:
+        sub1.perform(lc)
+    ex.match("environment variable 'MY_ENVIRONMENT_VARIABLE' does not exist")
+
+    sub2 = EnvironmentVariable('MY_ENVIRONMENT_VARIABLE', default_value='')
+    assert '' == sub2.perform(lc)
+
+    sub3 = EnvironmentVariable('MY_ENVIRONMENT_VARIABLE', default_value='MY_DEFAULT_VALUE')
+    assert 'MY_DEFAULT_VALUE' == sub3.perform(lc)
+
+    SetEnvironmentVariable('MY_ENVIRONMENT_VARIABLE', 'SOME_VALUE').visit(lc)
+    assert 'SOME_VALUE' == sub1.perform(lc)
+    assert 'SOME_VALUE' == sub2.perform(lc)
+    assert 'SOME_VALUE' == sub3.perform(lc)
+    del os.environ['MY_ENVIRONMENT_VARIABLE']

launch_xml/test/launch_xml/executable.xml

@@ -1,5 +1,5 @@
 <launch>
-    <executable cmd="ls -l -a -s" cwd="/" name="my_ls" shell="true" output='log' launch-prefix='$(env LAUNCH_PREFIX)'>
+    <executable cmd="ls -l -a -s" cwd="/" name="my_ls" shell="true" output="log" launch-prefix="$(env LAUNCH_PREFIX '')">
         <env name="var" value="1"/>
     </executable>
 </launch>