From 5587b6dc8418f1ee11d12315c1ceaf458c11965e Mon Sep 17 00:00:00 2001 From: Christopher Graham Date: Thu, 8 Oct 2026 12:52:14 -0400 Subject: [PATCH 1/2] Make --help explain how with-connect works. Give a clearer error when someone types a subcommand like start. --- README.md | 3 ++- main.py | 34 +++++++++++++++++++++--- test_main.py | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 106 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 9243311..3e4bbc7 100644 --- a/README.md +++ b/README.md @@ -102,7 +102,8 @@ with-connect --license ./rstudio-connect.lic You can eval the output to set the variables in your shell: ```bash -eval $(with-connect --license ./rstudio-connect.lic) +eval "$(with-connect --license ./rstudio-connect.lic)" +export CONNECT_API_KEY CONNECT_SERVER CONTAINER_ID curl -H "Authorization: Key $CONNECT_API_KEY" $CONNECT_SERVER/__api__/v1/content # Stop Connect when done (--stop without argument uses $CONTAINER_ID) diff --git a/main.py b/main.py index 8be4ad5..872b69c 100644 --- a/main.py +++ b/main.py @@ -22,6 +22,22 @@ REGISTRY_CUTOVER_YEAR = 2026 REGISTRY_CUTOVER_MONTH = 4 +DESCRIPTION = """\ +Starts Posit Connect in Docker and waits until it is ready. + +With no command, prints CONNECT_API_KEY, CONNECT_SERVER and CONTAINER_ID +as KEY=value lines on stdout and leaves the container running. +Progress and errors go to stderr. + +With "-- COMMAND", runs COMMAND with CONNECT_API_KEY and CONNECT_SERVER +set, then stops the container and exits with the command's exit code.""" + +EPILOG = """\ +examples: + with-connect --license ./rstudio-connect.lic + with-connect --license ./rstudio-connect.lic -- bash -c 'curl -H "Authorization: Key $CONNECT_API_KEY" $CONNECT_SERVER/__api__/v1/content' +""" + def parse_args(): """ @@ -31,7 +47,9 @@ def parse_args(): and the command to run against Connect. """ parser = argparse.ArgumentParser( - description="Run Posit Connect with optional command execution" + description=DESCRIPTION, + epilog=EPILOG, + formatter_class=argparse.RawDescriptionHelpFormatter, ) parser.add_argument( "--version", @@ -45,7 +63,7 @@ def parse_args(): ) parser.add_argument( "--image", - help="Container image to use, including tag (overrides --version)", + help="Container image to use, including tag (use instead of --version)", ) parser.add_argument( "--config", help="Path to rstudio-connect.gcfg configuration file" @@ -66,7 +84,7 @@ def parse_args(): "--port", type=int, default=3939, - help="Port to map the Connect container to (default: 3939)", + help="Port to map the Connect container to (default: 3939, 0 is not supported)", ) parser.add_argument( "--stop", @@ -86,7 +104,15 @@ def parse_args(): main_args = sys.argv[1:] command_args = [] - args = parser.parse_args(main_args) + args, unknown = parser.parse_known_args(main_args) + if unknown and unknown[0].startswith("-"): + parser.error(f"unrecognized arguments: {' '.join(unknown)}") + if unknown: + parser.error( + f"unexpected argument '{unknown[0]}', with-connect has no subcommands. " + "Run with-connect with no command to start Connect, " + "or put a command after '--'." + ) args.command = command_args return args diff --git a/test_main.py b/test_main.py index 37b2c26..3e8e05e 100644 --- a/test_main.py +++ b/test_main.py @@ -2,7 +2,7 @@ import subprocess import sys import tempfile -from unittest.mock import MagicMock, Mock +from unittest.mock import MagicMock, Mock, patch import docker import main @@ -265,6 +265,61 @@ def test_stop_argument_in_help(): assert "CONTAINER_ID" in result.stdout +def run_cli(*args): + return subprocess.run( + [sys.executable, "main.py", *args], + capture_output=True, + text=True, + ) + + +def parse_with_argv(*args): + with patch.object(sys, "argv", ["with-connect", *args]): + return main.parse_args() + + +def test_subcommand_gives_clear_error(): + result = run_cli("start") + + assert result.returncode == 2 + assert "no subcommands" in result.stderr + + +def test_unknown_flag_uses_default_error(): + result = run_cli("--json") + + assert result.returncode == 2 + assert "unrecognized arguments: --json" in result.stderr + assert "no subcommands" not in result.stderr + + +def test_subcommand_followed_by_flag_gives_clear_error(): + result = run_cli("start", "--json") + + assert result.returncode == 2 + assert "unexpected argument 'start'" in result.stderr + assert "no subcommands" in result.stderr + + +def test_unknown_flag_with_value_blames_the_flag(): + result = run_cli("--name", "foo") + + assert result.returncode == 2 + assert "unrecognized arguments: --name foo" in result.stderr + assert "no subcommands" not in result.stderr + + +def test_command_after_separator_is_not_a_subcommand(): + args = parse_with_argv("--port", "4000", "--", "start", "--json") + + assert args.port == 4000 + assert args.command == ["start", "--json"] + + +def test_no_command_without_separator(): + assert parse_with_argv("--quiet").command == [] + + def test_stop_nonexistent_container(): """Test that --stop with nonexistent container returns error.""" result = subprocess.run( @@ -350,6 +405,24 @@ def test_stop_nonexistent_container(): test_stop_argument_in_help() print("✓ test_stop_argument_in_help passed") + test_subcommand_gives_clear_error() + print("✓ test_subcommand_gives_clear_error passed") + + test_unknown_flag_uses_default_error() + print("✓ test_unknown_flag_uses_default_error passed") + + test_subcommand_followed_by_flag_gives_clear_error() + print("✓ test_subcommand_followed_by_flag_gives_clear_error passed") + + test_unknown_flag_with_value_blames_the_flag() + print("✓ test_unknown_flag_with_value_blames_the_flag passed") + + test_command_after_separator_is_not_a_subcommand() + print("✓ test_command_after_separator_is_not_a_subcommand passed") + + test_no_command_without_separator() + print("✓ test_no_command_without_separator passed") + test_stop_nonexistent_container() print("✓ test_stop_nonexistent_container passed") From 1711fd3cc0e64344a594e0869e5fc59199492a59 Mon Sep 17 00:00:00 2001 From: Christopher Graham Date: Tue, 13 Oct 2026 14:19:11 -0400 Subject: [PATCH 2/2] Use argparse's normal error for unknown arguments and reject a bad --port right away. Add how to stop the container to --help. --- main.py | 28 ++++++++++------- test_main.py | 88 +++++++++++++++++++++++++++++----------------------- 2 files changed, 66 insertions(+), 50 deletions(-) diff --git a/main.py b/main.py index 872b69c..7911ad6 100644 --- a/main.py +++ b/main.py @@ -27,6 +27,7 @@ With no command, prints CONNECT_API_KEY, CONNECT_SERVER and CONTAINER_ID as KEY=value lines on stdout and leaves the container running. +Stop it later with: with-connect --stop CONTAINER_ID Progress and errors go to stderr. With "-- COMMAND", runs COMMAND with CONNECT_API_KEY and CONNECT_SERVER @@ -39,6 +40,17 @@ """ +def port_number(value: str) -> int: + """Parse --port. Docker can only map a fixed host port from 1 to 65535.""" + try: + port = int(value) + except ValueError: + raise argparse.ArgumentTypeError(f"invalid port value: '{value}'") + if not 1 <= port <= 65535: + raise argparse.ArgumentTypeError(f"{port} is not a valid port. Use a free port from 1 to 65535.") + return port + + def parse_args(): """ Parse command line arguments. @@ -82,9 +94,9 @@ def parse_args(): ) parser.add_argument( "--port", - type=int, + type=port_number, default=3939, - help="Port to map the Connect container to (default: 3939, 0 is not supported)", + help="Port to map the Connect container to (default: 3939)", ) parser.add_argument( "--stop", @@ -92,7 +104,7 @@ def parse_args(): default=None, const="", # sentinel for --stop without argument metavar="CONTAINER_ID", - help="Stop a running Connect container by ID (uses CONTAINER_ID env var if not specified)", + help="Stop a running Connect container by ID (uses the exported CONTAINER_ID env var if not specified)", ) # Handle -- separator and capture remaining args @@ -104,15 +116,7 @@ def parse_args(): main_args = sys.argv[1:] command_args = [] - args, unknown = parser.parse_known_args(main_args) - if unknown and unknown[0].startswith("-"): - parser.error(f"unrecognized arguments: {' '.join(unknown)}") - if unknown: - parser.error( - f"unexpected argument '{unknown[0]}', with-connect has no subcommands. " - "Run with-connect with no command to start Connect, " - "or put a command after '--'." - ) + args = parser.parse_args(main_args) args.command = command_args return args diff --git a/test_main.py b/test_main.py index 3e8e05e..fa0071d 100644 --- a/test_main.py +++ b/test_main.py @@ -1,7 +1,9 @@ +import io import os import subprocess import sys import tempfile +from contextlib import redirect_stderr, redirect_stdout from unittest.mock import MagicMock, Mock, patch import docker @@ -265,51 +267,58 @@ def test_stop_argument_in_help(): assert "CONTAINER_ID" in result.stdout -def run_cli(*args): - return subprocess.run( - [sys.executable, "main.py", *args], - capture_output=True, - text=True, - ) - - def parse_with_argv(*args): with patch.object(sys, "argv", ["with-connect", *args]): return main.parse_args() -def test_subcommand_gives_clear_error(): - result = run_cli("start") +def parse_exit(*args): + """Run parse_args expecting it to exit. Returns the exit code and all output.""" + output = io.StringIO() + with patch.object(sys, "argv", ["with-connect", *args]), redirect_stdout(output), redirect_stderr(output): + try: + main.parse_args() + except SystemExit as e: + return e.code, output.getvalue() + raise AssertionError("parse_args did not exit") - assert result.returncode == 2 - assert "no subcommands" in result.stderr +def test_port_zero_is_rejected(): + code, output = parse_exit("--port", "0") -def test_unknown_flag_uses_default_error(): - result = run_cli("--json") + assert code == 2 + assert "0 is not a valid port" in output - assert result.returncode == 2 - assert "unrecognized arguments: --json" in result.stderr - assert "no subcommands" not in result.stderr +def test_port_above_range_is_rejected(): + code, output = parse_exit("--port", "70000") -def test_subcommand_followed_by_flag_gives_clear_error(): - result = run_cli("start", "--json") + assert code == 2 + assert "70000 is not a valid port" in output - assert result.returncode == 2 - assert "unexpected argument 'start'" in result.stderr - assert "no subcommands" in result.stderr +def test_port_not_a_number_is_rejected(): + code, output = parse_exit("--port", "abc") -def test_unknown_flag_with_value_blames_the_flag(): - result = run_cli("--name", "foo") + assert code == 2 + assert "invalid port value: 'abc'" in output - assert result.returncode == 2 - assert "unrecognized arguments: --name foo" in result.stderr - assert "no subcommands" not in result.stderr +def test_port_range_limits_are_accepted(): + assert parse_with_argv("--port", "1").port == 1 + assert parse_with_argv("--port", "65535").port == 65535 -def test_command_after_separator_is_not_a_subcommand(): + +def test_help_explains_how_to_stop(): + code, output = parse_exit("--help") + help_text = " ".join(output.split()) + + assert code == 0 + assert "Stop it later with: with-connect --stop CONTAINER_ID" in help_text + assert "exported CONTAINER_ID" in help_text + + +def test_command_after_separator(): args = parse_with_argv("--port", "4000", "--", "start", "--json") assert args.port == 4000 @@ -405,20 +414,23 @@ def test_stop_nonexistent_container(): test_stop_argument_in_help() print("✓ test_stop_argument_in_help passed") - test_subcommand_gives_clear_error() - print("✓ test_subcommand_gives_clear_error passed") + test_port_zero_is_rejected() + print("✓ test_port_zero_is_rejected passed") + + test_port_above_range_is_rejected() + print("✓ test_port_above_range_is_rejected passed") - test_unknown_flag_uses_default_error() - print("✓ test_unknown_flag_uses_default_error passed") + test_port_not_a_number_is_rejected() + print("✓ test_port_not_a_number_is_rejected passed") - test_subcommand_followed_by_flag_gives_clear_error() - print("✓ test_subcommand_followed_by_flag_gives_clear_error passed") + test_port_range_limits_are_accepted() + print("✓ test_port_range_limits_are_accepted passed") - test_unknown_flag_with_value_blames_the_flag() - print("✓ test_unknown_flag_with_value_blames_the_flag passed") + test_help_explains_how_to_stop() + print("✓ test_help_explains_how_to_stop passed") - test_command_after_separator_is_not_a_subcommand() - print("✓ test_command_after_separator_is_not_a_subcommand passed") + test_command_after_separator() + print("✓ test_command_after_separator passed") test_no_command_without_separator() print("✓ test_no_command_without_separator passed")