Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add '--write' option to avoid an unintentional data loss. #638

Merged
merged 3 commits into from
Jun 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions ros2param/ros2param/verb/dump.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# limitations under the License.

import os
import sys

from rcl_interfaces.srv import ListParameters

Expand Down Expand Up @@ -50,10 +51,10 @@ def add_arguments(self, parser, cli_name): # noqa: D102
parser.add_argument(
'--output-dir',
default='.',
help='The absolute path were to save the generated file')
help='DEPRECATED: The absolute path where to save the generated file')
parser.add_argument(
'--print', action='store_true',
help='Print generated file in terminal rather than saving a file.')
help='DEPRECATED: Does nothing.')

@staticmethod
def get_parameter_value(node, node_name, param):
Expand Down Expand Up @@ -124,6 +125,15 @@ def main(self, *, args): # noqa: D102
f"'{node_name.full_name}': {e}")

if args.print:
print(
"WARNING: '--print' is deprecated; this utility prints to stdout by default",
file=sys.stderr)

if args.output_dir != '.':
print(
"WARNING: '--output-dir' is deprecated; use redirection to save to a file",
file=sys.stderr)
else:
print(yaml.dump(yaml_output, default_flow_style=False))
return

Expand Down
48 changes: 26 additions & 22 deletions ros2param/test/test_verb_dump.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,45 +210,49 @@ def test_verb_dump_invalid_path(self):
)

def test_verb_dump(self):
with tempfile.TemporaryDirectory() as tmpdir:
with self.launch_param_dump_command(
arguments=[f'{TEST_NAMESPACE}/{TEST_NODE}', '--output-dir', tmpdir]
) as param_dump_command:
assert param_dump_command.wait_for_shutdown(timeout=TEST_TIMEOUT)
assert param_dump_command.exit_code == launch_testing.asserts.EXIT_OK
assert launch_testing.tools.expect_output(
expected_lines=[''],
text=param_dump_command.output,
strict=True
)
# Compare generated parameter file against expected
generated_param_file = os.path.join(tmpdir, self._output_file())
assert (open(generated_param_file, 'r').read() == EXPECTED_PARAMETER_FILE)
with self.launch_param_dump_command(
arguments=[f'{TEST_NAMESPACE}/{TEST_NODE}']
) as param_dump_command:
assert param_dump_command.wait_for_shutdown(timeout=TEST_TIMEOUT)
assert param_dump_command.exit_code == launch_testing.asserts.EXIT_OK
assert launch_testing.tools.expect_output(
expected_text=EXPECTED_PARAMETER_FILE + '\n',
text=param_dump_command.output,
strict=True
)

# TODO(fujitatomoya): remove this test when '--print' option is removed
def test_verb_dump_print(self):
# If '--print' is provided, ensure it does nothing but print parameters to stdout
with self.launch_param_dump_command(
arguments=[f'{TEST_NAMESPACE}/{TEST_NODE}', '--print']
) as param_dump_command:
assert param_dump_command.wait_for_shutdown(timeout=TEST_TIMEOUT)
assert param_dump_command.exit_code == launch_testing.asserts.EXIT_OK
assert launch_testing.tools.expect_output(
expected_text=EXPECTED_PARAMETER_FILE + '\n',
expected_lines=[
"WARNING: '--print' is deprecated; this utility prints to stdout by default"
],
text=param_dump_command.output,
strict=True
)
# If both '--output-dir' and '--print' options are provided, ensure it only prints
# and no file is written

# TODO(fujitatomoya): remove this test when '--output-dir' option is removed
def test_verb_dump_output(self):
# If '--output-dir' is provided, ensure it only saves parameters into file
with tempfile.TemporaryDirectory() as tmpdir:
with self.launch_param_dump_command(
arguments=[f'{TEST_NAMESPACE}/{TEST_NODE}', '--output-dir', tmpdir, '--print']
arguments=[f'{TEST_NAMESPACE}/{TEST_NODE}', '--output-dir', tmpdir]
) as param_dump_command:
assert param_dump_command.wait_for_shutdown(timeout=TEST_TIMEOUT)
assert param_dump_command.exit_code == launch_testing.asserts.EXIT_OK
assert launch_testing.tools.expect_output(
expected_text=EXPECTED_PARAMETER_FILE + '\n',
expected_lines=[
"WARNING: '--output-dir' is deprecated; use redirection to save to a file"
],
text=param_dump_command.output,
strict=True
)
# Make sure the file was not create, thus '--print' did preempt
not_generated_param_file = os.path.join(tmpdir, self._output_file())
assert not os.path.exists(not_generated_param_file)
# Compare generated parameter file against expected
generated_param_file = os.path.join(tmpdir, self._output_file())
assert (open(generated_param_file, 'r').read() == EXPECTED_PARAMETER_FILE)
6 changes: 3 additions & 3 deletions ros2param/test/test_verb_load.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ def test_verb_load(self):
)
# Dump with ros2 param dump and compare that output matches input file
with self.launch_param_dump_command(
arguments=[f'{TEST_NAMESPACE}/{TEST_NODE}', '--print']
arguments=[f'{TEST_NAMESPACE}/{TEST_NODE}']
) as param_dump_command:
assert param_dump_command.wait_for_shutdown(timeout=TEST_TIMEOUT)
assert param_dump_command.exit_code == launch_testing.asserts.EXIT_OK
Expand Down Expand Up @@ -323,7 +323,7 @@ def test_verb_load_wildcard(self):
)
# Dump with ros2 param and check that wildcard parameters are loaded
with self.launch_param_dump_command(
arguments=[f'{TEST_NAMESPACE}/{TEST_NODE}', '--print']
arguments=[f'{TEST_NAMESPACE}/{TEST_NODE}']
) as param_dump_command:
assert param_dump_command.wait_for_shutdown(timeout=TEST_TIMEOUT)
assert param_dump_command.exit_code == launch_testing.asserts.EXIT_OK
Expand All @@ -344,7 +344,7 @@ def test_verb_load_wildcard(self):

# Dump and check that wildcard parameters were overriden if in node namespace
with self.launch_param_dump_command(
arguments=[f'{TEST_NAMESPACE}/{TEST_NODE}', '--print']
arguments=[f'{TEST_NAMESPACE}/{TEST_NODE}']
) as param_dump_command:
assert param_dump_command.wait_for_shutdown(timeout=TEST_TIMEOUT)
assert param_dump_command.exit_code == launch_testing.asserts.EXIT_OK
Expand Down