diff --git a/launch_ros/launch_ros/parameter_descriptions.py b/launch_ros/launch_ros/parameter_descriptions.py index 46f226d3..dd071fac 100644 --- a/launch_ros/launch_ros/parameter_descriptions.py +++ b/launch_ros/launch_ros/parameter_descriptions.py @@ -171,7 +171,7 @@ def __init__( self, param_file: Union[FilePath, SomeSubstitutionsType], *, - allow_substs: [bool, SomeSubstitutionsType] = False + allow_substs: Union[bool, SomeSubstitutionsType] = False ) -> None: """ Construct a parameter file description. @@ -185,6 +185,8 @@ def __init__( # during cleanup, so make sure to initialize them here. self.__evaluated_param_file: Optional[Path] = None self.__created_tmp_file = False + self.__allow_substs: Union[bool, List[Substitution]] = False + self.__evaluated_allow_substs: Optional[bool] = None ensure_argument_type( param_file, @@ -194,7 +196,7 @@ def __init__( ) ensure_argument_type( allow_substs, - bool, + SomeSubstitutionsType_types_tuple + (bool,), 'allow_subst', 'ParameterFile()' ) @@ -202,7 +204,6 @@ def __init__( if isinstance(param_file, SomeSubstitutionsType_types_tuple): self.__param_file = normalize_to_list_of_substitutions(param_file) self.__allow_substs = normalize_typed_substitution(allow_substs, data_type=bool) - self.__evaluated_allow_substs: Optional[bool] = None @property def param_file(self) -> Union[FilePath, List[Substitution]]: @@ -249,6 +250,7 @@ def evaluate(self, context: LaunchContext) -> Path: h.write(parsed) param_file_path = Path(h.name) self.__created_tmp_file = True + self.__evaluated_allow_substs = allow_substs self.__evaluated_param_file = param_file_path return param_file_path @@ -259,7 +261,12 @@ def cleanup(self): os.unlink(self.__evaluated_param_file) except FileNotFoundError: pass + self.__created_tmp_file = False + self.__evaluated_param_file = None + self.__evaluated_allow_substs = None + elif isinstance(self.__allow_substs, list): self.__evaluated_param_file = None + self.__evaluated_allow_substs = None def __del__(self): self.cleanup() diff --git a/test_launch_ros/test/test_launch_ros/descriptions/test_parameter_file.py b/test_launch_ros/test/test_launch_ros/descriptions/test_parameter_file.py index 80adc449..ebef189f 100644 --- a/test_launch_ros/test/test_launch_ros/descriptions/test_parameter_file.py +++ b/test_launch_ros/test/test_launch_ros/descriptions/test_parameter_file.py @@ -15,7 +15,9 @@ """Tests for launch_ros.descriptions.ParameterFile.""" from contextlib import contextmanager +import gc import os +import sys from tempfile import NamedTemporaryFile from launch import Substitution @@ -52,6 +54,9 @@ def __init__(self, text): def perform(self, context): return self.__text + def set_text(self, text): + self.__text = text + @expose_substitution('test') def parse_test_substitution(data): @@ -136,3 +141,53 @@ def test_parameter_file_description(original_contents, expected_contents, allow_ else: assert param_file.exists() assert os.fspath(desc.param_file) == os.fspath(file_name) + + +def test_parameter_file_allow_substs_substitution(): + lc = MockContext() + with get_parameter_file('{}') as file_name: + desc = ParameterFile( + file_name, + allow_substs=CustomSubstitution('true'), + ) + assert isinstance(desc.allow_substs, list) + desc.evaluate(lc) + assert desc.allow_substs is True + desc.cleanup() + assert isinstance(desc.allow_substs, list) + + +def test_parameter_file_invalid_allow_substs_does_not_fail_cleanup(): + unraisable = [] + original_unraisablehook = sys.unraisablehook + sys.unraisablehook = unraisable.append + try: + with pytest.raises(TypeError, match='allow_subst'): + ParameterFile('params.yaml', allow_substs=object()) + gc.collect() + finally: + sys.unraisablehook = original_unraisablehook + + assert not unraisable + + +@pytest.mark.parametrize('first, second', [('false', 'true'), ('true', 'false')]) +def test_parameter_file_allow_substs_re_evaluates_after_cleanup(first, second): + lc = MockContext() + allow_substs = CustomSubstitution(first) + with get_parameter_file('{}') as file_name: + desc = ParameterFile(file_name, allow_substs=allow_substs) + + first_path = desc.evaluate(lc) + assert desc.allow_substs is (first == 'true') + assert (os.fspath(first_path) != file_name) is (first == 'true') + desc.cleanup() + assert os.path.exists(file_name) + + allow_substs.set_text(second) + second_path = desc.evaluate(lc) + assert desc.allow_substs is (second == 'true') + assert (os.fspath(second_path) != file_name) is (second == 'true') + desc.cleanup() + assert os.path.exists(file_name) + assert isinstance(desc.allow_substs, list) diff --git a/test_launch_ros/test/test_launch_ros/frontend/test_node_frontend.py b/test_launch_ros/test/test_launch_ros/frontend/test_node_frontend.py index 0bebb6d5..547de598 100644 --- a/test_launch_ros/test/test_launch_ros/frontend/test_node_frontend.py +++ b/test_launch_ros/test/test_launch_ros/frontend/test_node_frontend.py @@ -57,7 +57,7 @@ def test_launch_frontend_xml(): - + @@ -131,6 +131,7 @@ def test_launch_frontend_yaml(): value: ['2', '5', '8'] type: list_of_str - from: {} + allow_substs: $(eval False) env: - name: var value: '1'