From 5e62d3c0a7017f78c695b10d31dcfeab2bfa37de Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Thu, 16 Nov 2023 14:47:25 +0800 Subject: [PATCH 1/9] Add the ability to parse configuration overrides when finalizing apps. --- src/briefcase/commands/base.py | 43 ++++++- tests/commands/base/test_finalize.py | 113 +++++++++++++++++- .../base/test_parse_config_overrides.py | 76 ++++++++++++ tests/commands/base/test_parse_options.py | 17 ++- 4 files changed, 242 insertions(+), 7 deletions(-) create mode 100644 tests/commands/base/test_parse_config_overrides.py diff --git a/src/briefcase/commands/base.py b/src/briefcase/commands/base.py index 9c511c18a..bb18bbcda 100644 --- a/src/briefcase/commands/base.py +++ b/src/briefcase/commands/base.py @@ -11,6 +11,7 @@ from abc import ABC, abstractmethod from argparse import RawDescriptionHelpFormatter from pathlib import Path +from typing import Any from cookiecutter import exceptions as cookiecutter_exceptions from cookiecutter.repository import is_repo_url @@ -103,6 +104,26 @@ def split_passthrough(args): return args[:pos], args[pos + 1 :] +def parse_config_overrides(config_overrides: list[str] | None) -> dict[str, Any]: + """Parse command line -C/--config option overrides. + + :param config_overrides: The values passed in as configuration overrides. Each value + *should* be a "key=" string. + :returns: A dictionary of app configuration keys to override and their new values. + :raises BriefcaseCommandError: if any of the values can't be parsed as valid TOML. + """ + overrides = {} + if config_overrides: + for value in config_overrides: + try: + overrides.update(tomllib.loads(value)) + except ValueError: + raise BriefcaseCommandError( + f"Unable to parse configuration override {value}" + ) + return overrides + + class BaseCommand(ABC): cmd_line = "briefcase {command} {platform} {output_format}" supported_host_os = {"Darwin", "Linux", "Windows"} @@ -530,7 +551,11 @@ def finalize_app_config(self, app: AppConfig): :param app: The app configuration to finalize. """ - def finalize(self, app: AppConfig | None = None): + def finalize( + self, + app: AppConfig | None = None, + config_overrides: list[str] | None = None, + ): """Finalize Briefcase configuration. This will: @@ -544,6 +569,10 @@ def finalize(self, app: AppConfig | None = None): :param app: If provided, the specific app configuration to finalize. By default, all apps will be finalized. """ + # Convert any configuration overrides provided at the command line + # into values that can be applied to the app's config + overrides = parse_config_overrides(config_overrides) + self.verify_host() self.verify_tools() @@ -551,10 +580,14 @@ def finalize(self, app: AppConfig | None = None): for app in self.apps.values(): if hasattr(app, "__draft__"): self.finalize_app_config(app) + for key, value in overrides.items(): + setattr(app, key, value) delattr(app, "__draft__") else: if hasattr(app, "__draft__"): self.finalize_app_config(app) + for key, value in overrides.items(): + setattr(app, key, value) delattr(app, "__draft__") def verify_app(self, app: AppConfig): @@ -687,6 +720,14 @@ def add_default_options(self, parser): :param parser: a stub argparse parser for the command. """ + parser.add_argument( + "-C", + "--config", + dest="config_overrides", + action="append", + metavar="KEY=VALUE", + help="Override the value of the app configuration item KEY with VALUE.", + ) parser.add_argument( "-v", "--verbosity", diff --git a/tests/commands/base/test_finalize.py b/tests/commands/base/test_finalize.py index e0740702b..982674f91 100644 --- a/tests/commands/base/test_finalize.py +++ b/tests/commands/base/test_finalize.py @@ -38,9 +38,56 @@ def base_command(tmp_path, first_app, second_app): ) -def test_finalize_all(base_command, first_app, second_app): +@pytest.mark.parametrize( + "overrides,first_app_values,second_app_values", + [ + # No overrides + ( + None, + { + "version": "0.0.1", + "description": "The first simple app", + }, + { + "version": "0.0.2", + "description": "The second simple app", + }, + ), + # Multiple overrides, different types + ( + [ + "version='42.37'", + "key1='val1'", + "key2=2", + "key3=[1, 'two']", + ], + { + "version": "42.37", + "description": "The first simple app", + "key1": "val1", + "key2": 2, + "key3": [1, "two"], + }, + { + "version": "42.37", + "description": "The second simple app", + "key1": "val1", + "key2": 2, + "key3": [1, "two"], + }, + ), + ], +) +def test_finalize_all( + base_command, + first_app, + second_app, + overrides, + first_app_values, + second_app_values, +): "A call to finalize verifies host, tools, and finalized all app configs" - base_command.finalize() + base_command.finalize(config_overrides=overrides) # The right sequence of things will be done assert base_command.actions == [ @@ -58,10 +105,60 @@ def test_finalize_all(base_command, first_app, second_app): assert not hasattr(first_app, "__draft__") assert not hasattr(second_app, "__draft__") - -def test_finalize_single(base_command, first_app, second_app): + # Override values have been set + for key, value in first_app_values.items(): + assert getattr(first_app, key) == value + for key, value in second_app_values.items(): + assert getattr(second_app, key) == value + + +@pytest.mark.parametrize( + "overrides,first_app_values,second_app_values", + [ + # No overrides + ( + None, + { + "version": "0.0.1", + "description": "The first simple app", + }, + { + "version": "0.0.2", + "description": "The second simple app", + }, + ), + # Multiple overrides, different types + ( + [ + "version='42.37'", + "key1='val1'", + "key2=2", + "key3=[1, 'two']", + ], + { + "version": "42.37", + "description": "The first simple app", + "key1": "val1", + "key2": 2, + "key3": [1, "two"], + }, + { + "version": "0.0.2", + "description": "The second simple app", + }, + ), + ], +) +def test_finalize_single( + base_command, + first_app, + second_app, + overrides, + first_app_values, + second_app_values, +): "A call to finalize verifies host, tools, and finalized all app configs" - base_command.finalize(first_app) + base_command.finalize(first_app, overrides) # The right sequence of things will be done assert base_command.actions == [ @@ -77,6 +174,12 @@ def test_finalize_single(base_command, first_app, second_app): assert not hasattr(first_app, "__draft__") assert hasattr(second_app, "__draft__") + # Override values have been set + for key, value in first_app_values.items(): + assert getattr(first_app, key) == value + for key, value in second_app_values.items(): + assert getattr(second_app, key) == value + def test_finalize_all_repeat(base_command, first_app, second_app): "Multiple calls to finalize verifies host & tools multiple times, but only once on config" diff --git a/tests/commands/base/test_parse_config_overrides.py b/tests/commands/base/test_parse_config_overrides.py new file mode 100644 index 000000000..b1b2d208e --- /dev/null +++ b/tests/commands/base/test_parse_config_overrides.py @@ -0,0 +1,76 @@ +import pytest + +from briefcase.commands.base import parse_config_overrides +from briefcase.exceptions import BriefcaseCommandError + + +@pytest.mark.parametrize( + "overrides, values", + [ + # No content + (None, {}), + ([], {}), + # Boolean + (["key=true"], {"key": True}), + # Integers + (["key=42"], {"key": 42}), + (["key=-42"], {"key": -42}), + # Integers + (["key=42.37"], {"key": 42.37}), + (["key=-42.37"], {"key": -42.37}), + # Strings + (["key='hello'"], {"key": "hello"}), + (["key=''"], {"key": ""}), + (["key='42'"], {"key": "42"}), + (['key="hello"'], {"key": "hello"}), + (['key=""'], {"key": ""}), + (['key="42"'], {"key": "42"}), + # List + (['key=[1, "two", true]'], {"key": [1, "two", True]}), + # Dictionary + (['key={a=1, b="two", c=true}'], {"key": {"a": 1, "b": "two", "c": True}}), + # Multiple values + ( + [ + "key1=42", + 'key2="hello"', + 'key3=[1, "two", true]', + 'key4={a=1, b="two", c=true}', + ], + { + "key1": 42, + "key2": "hello", + "key3": [1, "two", True], + "key4": {"a": 1, "b": "two", "c": True}, + }, + ), + ], +) +def test_valid_overrides(overrides, values): + """Valid values can be parsed as config overrides.""" + assert parse_config_overrides(overrides) == values + + +@pytest.mark.parametrize( + "overrides", + [ + # Bare string + ["foobar"], + # Unquoted string + ["key=foobar"], + # Unbalanced quote + ["key='foobar"], + # Unmatched brackets + ['key=[1, "two",'], + # Unmatches parentheses + ['key={a=1, b="two"'], + # Valid value followed by invalid + ["good=42", "key=foobar"], + ], +) +def test_invalid_overrides(overrides): + with pytest.raises( + BriefcaseCommandError, + match=r"Unable to parse configuration override ", + ): + parse_config_overrides(overrides) diff --git a/tests/commands/base/test_parse_options.py b/tests/commands/base/test_parse_options.py index 684e59dd1..ba0109d48 100644 --- a/tests/commands/base/test_parse_options.py +++ b/tests/commands/base/test_parse_options.py @@ -5,12 +5,27 @@ def test_parse_options(base_command): """Command line options are parsed if provided.""" - options = base_command.parse_options(extra=("-x", "wibble", "-r", "important")) + options = base_command.parse_options( + extra=( + "-x", + "wibble", + "-r", + "important", + "-C", + "width=10", + "-C", + "height=20", + ) + ) assert options == { "extra": "wibble", "mystery": None, "required": "important", + "config_overrides": [ + "width=10", + "height=20", + ], } assert base_command.input.enabled assert base_command.logger.verbosity == LogLevel.INFO From 806886e4c36746cf3a679a2721603edc31a44b12 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Thu, 16 Nov 2023 14:48:31 +0800 Subject: [PATCH 2/9] Pass config overrides through all app-facing commands. --- src/briefcase/commands/build.py | 3 +- src/briefcase/commands/create.py | 9 ++++- src/briefcase/commands/dev.py | 3 +- src/briefcase/commands/open.py | 9 ++++- src/briefcase/commands/package.py | 3 +- src/briefcase/commands/run.py | 1 + src/briefcase/commands/update.py | 1 + tests/commands/create/test_call.py | 40 +++++++++++++++++++ tests/commands/new/test_call.py | 9 ++++- tests/commands/publish/test_call.py | 24 +++++++++-- tests/platforms/android/gradle/test_run.py | 3 ++ tests/platforms/iOS/xcode/test_run.py | 1 + tests/platforms/linux/appimage/test_create.py | 8 +++- tests/platforms/linux/system/test_create.py | 8 +++- tests/platforms/macOS/app/test_package.py | 1 + tests/platforms/web/static/test_run.py | 2 + tests/platforms/windows/app/test_package.py | 1 + tests/test_cmdline.py | 40 ++++++++++++------- 18 files changed, 135 insertions(+), 31 deletions(-) diff --git a/src/briefcase/commands/build.py b/src/briefcase/commands/build.py index 9c3c7ddc4..71c09ad2d 100644 --- a/src/briefcase/commands/build.py +++ b/src/briefcase/commands/build.py @@ -88,6 +88,7 @@ def __call__( update_support: bool = False, no_update: bool = False, test_mode: bool = False, + config_overrides: list[str] | None = None, **options, ) -> dict | None: # Has the user requested an invalid set of options? @@ -112,7 +113,7 @@ def __call__( # Confirm host compatibility, that all required tools are available, # and that the app configuration is finalized. - self.finalize(app) + self.finalize(app, config_overrides) if app: state = self._build_app( diff --git a/src/briefcase/commands/create.py b/src/briefcase/commands/create.py index 44b938afc..1297b9580 100644 --- a/src/briefcase/commands/create.py +++ b/src/briefcase/commands/create.py @@ -831,10 +831,15 @@ def verify_app_tools(self, app: AppConfig): super().verify_app_tools(app) NativeAppContext.verify(tools=self.tools, app=app) - def __call__(self, app: AppConfig | None = None, **options) -> dict | None: + def __call__( + self, + app: AppConfig | None = None, + config_overrides: list[str] | None = None, + **options, + ) -> dict | None: # Confirm host compatibility, that all required tools are available, # and that the app configuration is finalized. - self.finalize(app) + self.finalize(app, config_overrides) if app: state = self.create_app(app, **options) diff --git a/src/briefcase/commands/dev.py b/src/briefcase/commands/dev.py index 5380c2b16..2957afb5c 100644 --- a/src/briefcase/commands/dev.py +++ b/src/briefcase/commands/dev.py @@ -179,6 +179,7 @@ def __call__( run_app: Optional[bool] = True, test_mode: Optional[bool] = False, passthrough: Optional[List[str]] = None, + config_overrides: list[str] | None = None, **options, ): # Which app should we run? If there's only one defined @@ -200,7 +201,7 @@ def __call__( ) # Confirm host compatibility, that all required tools are available, # and that the app configuration is finalized. - self.finalize(app) + self.finalize(app, config_overrides) self.verify_app(app) diff --git a/src/briefcase/commands/open.py b/src/briefcase/commands/open.py index ec8f38906..ef572c508 100644 --- a/src/briefcase/commands/open.py +++ b/src/briefcase/commands/open.py @@ -44,10 +44,15 @@ def open_app(self, app: AppConfig, **options): return state - def __call__(self, app: AppConfig | None = None, **options): + def __call__( + self, + app: AppConfig | None = None, + config_overrides: list[str] | None = None, + **options, + ): # Confirm host compatibility, that all required tools are available, # and that the app configuration is finalized. - self.finalize(app) + self.finalize(app, config_overrides) if app: state = self.open_app(app, **options) diff --git a/src/briefcase/commands/package.py b/src/briefcase/commands/package.py index 465fc4774..dc1d36188 100644 --- a/src/briefcase/commands/package.py +++ b/src/briefcase/commands/package.py @@ -137,11 +137,12 @@ def __call__( self, app: AppConfig | None = None, update: bool = False, + config_overrides: list[str] | None = None, **options, ) -> dict | None: # Confirm host compatibility, that all required tools are available, # and that the app configuration is finalized. - self.finalize(app) + self.finalize(app, config_overrides) if app: state = self._package_app(app, update=update, **options) diff --git a/src/briefcase/commands/run.py b/src/briefcase/commands/run.py index 6a54b499d..7e491f9ec 100644 --- a/src/briefcase/commands/run.py +++ b/src/briefcase/commands/run.py @@ -248,6 +248,7 @@ def __call__( no_update: bool = False, test_mode: bool = False, passthrough: list[str] | None = None, + config_overrides: list[str] | None = None, **options, ) -> dict | None: # Which app should we run? If there's only one defined diff --git a/src/briefcase/commands/update.py b/src/briefcase/commands/update.py index 4b3809fcc..4d929784d 100644 --- a/src/briefcase/commands/update.py +++ b/src/briefcase/commands/update.py @@ -68,6 +68,7 @@ def __call__( update_resources: bool = False, update_support: bool = False, test_mode: bool = False, + config_overrides: list[str] | None = None, **options, ) -> dict | None: # Confirm host compatibility, that all required tools are available, diff --git a/tests/commands/create/test_call.py b/tests/commands/create/test_call.py index 156013990..e14d8d1e1 100644 --- a/tests/commands/create/test_call.py +++ b/tests/commands/create/test_call.py @@ -92,3 +92,43 @@ def test_create_single(tracking_create_command, tmp_path): assert not ( tmp_path / "base_path" / "build" / "second" / "tester" / "dummy" / "new" ).exists() + + +def test_create_single_with_overrides(tracking_create_command, tmp_path): + """Command line overrides can be used during app creation.""" + tracking_create_command( + app=tracking_create_command.apps["first"], + config_overrides=["template='https://example.com/override'"], + ) + + # The right sequence of things will be done + assert tracking_create_command.actions == [ + # Host OS is verified + ("verify-host",), + # Tools are verified + ("verify-tools",), + # App config has been finalized + ("finalize-app-config", "first"), + # Create the first app + ("generate", "first"), + ("support", "first"), + ("verify-app-template", "first"), + ("verify-app-tools", "first"), + ("code", "first", False), + ("requirements", "first", False), + ("resources", "first"), + ("cleanup", "first"), + ] + + # New app content has been created + assert ( + tmp_path / "base_path" / "build" / "first" / "tester" / "dummy" / "new" + ).exists() + assert not ( + tmp_path / "base_path" / "build" / "second" / "tester" / "dummy" / "new" + ).exists() + + # The command line override has been applied. + assert ( + tracking_create_command.apps["first"].template == "https://example.com/override" + ) diff --git a/tests/commands/new/test_call.py b/tests/commands/new/test_call.py index 8e3e1890c..ef3bb1423 100644 --- a/tests/commands/new/test_call.py +++ b/tests/commands/new/test_call.py @@ -40,5 +40,12 @@ def test_new_app(new_command): # Tools are verified ("verify-tools",), # Run the first app - ("new", {"template": None, "template_branch": None}), + ( + "new", + { + "config_overrides": None, + "template": None, + "template_branch": None, + }, + ), ] diff --git a/tests/commands/publish/test_call.py b/tests/commands/publish/test_call.py index 59b271833..d2767bcf4 100644 --- a/tests/commands/publish/test_call.py +++ b/tests/commands/publish/test_call.py @@ -31,13 +31,21 @@ def test_publish(publish_command, first_app, second_app): # App tools are verified for first app ("verify-app-tools", "first"), # Publish the first app to s3 - ("publish", "first", "s3", {}), + ("publish", "first", "s3", {"config_overrides": None}), # App template is verified for second app ("verify-app-template", "second"), # App tools are verified for second app ("verify-app-tools", "second"), # Publish the second app to s3 - ("publish", "second", "s3", {"publish_state": "first"}), + ( + "publish", + "second", + "s3", + { + "config_overrides": None, + "publish_state": "first", + }, + ), ] @@ -69,13 +77,21 @@ def test_publish_alternative_channel(publish_command, first_app, second_app): # App tools are verified for first app ("verify-app-tools", "first"), # Publish the first app to the alternative channel - ("publish", "first", "alternative", {}), + ("publish", "first", "alternative", {"config_overrides": None}), # App template is verified for second app ("verify-app-template", "second"), # App tools are verified for second app ("verify-app-tools", "second"), # Publish the second app to the alternative channel - ("publish", "second", "alternative", {"publish_state": "first"}), + ( + "publish", + "second", + "alternative", + { + "config_overrides": None, + "publish_state": "first", + }, + ), ] diff --git a/tests/platforms/android/gradle/test_run.py b/tests/platforms/android/gradle/test_run.py index 3549bbcfd..7d6e78517 100644 --- a/tests/platforms/android/gradle/test_run.py +++ b/tests/platforms/android/gradle/test_run.py @@ -94,6 +94,7 @@ def test_device_option(run_command): "passthrough": [], "extra_emulator_args": None, "shutdown_on_exit": False, + "config_overrides": None, } @@ -115,6 +116,7 @@ def test_extra_emulator_args_option(run_command): "passthrough": [], "extra_emulator_args": ["-no-window", "-no-audio"], "shutdown_on_exit": False, + "config_overrides": None, } @@ -134,6 +136,7 @@ def test_shutdown_on_exit_option(run_command): "passthrough": [], "extra_emulator_args": None, "shutdown_on_exit": True, + "config_overrides": None, } diff --git a/tests/platforms/iOS/xcode/test_run.py b/tests/platforms/iOS/xcode/test_run.py index 6f858af10..316ef3473 100644 --- a/tests/platforms/iOS/xcode/test_run.py +++ b/tests/platforms/iOS/xcode/test_run.py @@ -51,6 +51,7 @@ def test_device_option(run_command): "test_mode": False, "passthrough": [], "appname": None, + "config_overrides": None, } diff --git a/tests/platforms/linux/appimage/test_create.py b/tests/platforms/linux/appimage/test_create.py index 62674398c..a544a4633 100644 --- a/tests/platforms/linux/appimage/test_create.py +++ b/tests/platforms/linux/appimage/test_create.py @@ -23,7 +23,9 @@ def test_default_options(create_command): """The default options are as expected.""" options = create_command.parse_options([]) - assert options == {} + assert options == { + "config_overrides": None, + } assert create_command.use_docker @@ -32,7 +34,9 @@ def test_options(create_command): """The extra options can be parsed.""" options = create_command.parse_options(["--no-docker"]) - assert options == {} + assert options == { + "config_overrides": None, + } assert not create_command.use_docker diff --git a/tests/platforms/linux/system/test_create.py b/tests/platforms/linux/system/test_create.py index 2039dcaed..87cf6a8e1 100644 --- a/tests/platforms/linux/system/test_create.py +++ b/tests/platforms/linux/system/test_create.py @@ -9,7 +9,9 @@ def test_default_options(create_command): """The default options are as expected.""" options = create_command.parse_options([]) - assert options == {} + assert options == { + "config_overrides": None, + } assert create_command.target_image is None @@ -18,7 +20,9 @@ def test_options(create_command): """The extra options can be parsed.""" options = create_command.parse_options(["--target", "somevendor:surprising"]) - assert options == {} + assert options == { + "config_overrides": None, + } assert create_command.target_image == "somevendor:surprising" diff --git a/tests/platforms/macOS/app/test_package.py b/tests/platforms/macOS/app/test_package.py index 93dda82fa..31db0fa1e 100644 --- a/tests/platforms/macOS/app/test_package.py +++ b/tests/platforms/macOS/app/test_package.py @@ -46,6 +46,7 @@ def test_device_option(package_command): "notarize_app": False, "packaging_format": "dmg", "update": False, + "config_overrides": None, } diff --git a/tests/platforms/web/static/test_run.py b/tests/platforms/web/static/test_run.py index 04e54f1db..cbf695bef 100644 --- a/tests/platforms/web/static/test_run.py +++ b/tests/platforms/web/static/test_run.py @@ -42,6 +42,7 @@ def test_default_options(run_command): "host": "localhost", "port": 8080, "open_browser": True, + "config_overrides": None, } @@ -63,6 +64,7 @@ def test_options(run_command): "host": "myhost", "port": 1234, "open_browser": False, + "config_overrides": None, } diff --git a/tests/platforms/windows/app/test_package.py b/tests/platforms/windows/app/test_package.py index eb9d99d62..a8f126ef7 100644 --- a/tests/platforms/windows/app/test_package.py +++ b/tests/platforms/windows/app/test_package.py @@ -187,6 +187,7 @@ def test_parse_options(package_command, cli_args, signing_options, is_sdk_needed adhoc_sign=False, packaging_format="msi", update=False, + config_overrides=None, ) expected_options = {**default_options, **signing_options} diff --git a/tests/test_cmdline.py b/tests/test_cmdline.py index 1ed6df5ef..7f3986cec 100644 --- a/tests/test_cmdline.py +++ b/tests/test_cmdline.py @@ -104,7 +104,11 @@ def test_new_command(logger, console): assert cmd.logger.verbosity == LogLevel.INFO assert cmd.logger is logger assert cmd.input is console - assert options == {"template": None, "template_branch": None} + assert options == { + "config_overrides": None, + "template": None, + "template_branch": None, + } # Common tests for dev and run commands. @@ -159,6 +163,7 @@ def test_dev_command(monkeypatch, logger, console, cmdline, expected_options): "run_app": True, "test_mode": False, "passthrough": [], + "config_overrides": None, **expected_options, } @@ -197,6 +202,7 @@ def test_run_command(monkeypatch, logger, console, cmdline, expected_options): "no_update": False, "test_mode": False, "passthrough": [], + "config_overrides": None, **expected_options, } @@ -218,6 +224,7 @@ def test_upgrade_command(monkeypatch, logger, console): assert options == { "list_tools": False, "tool_list": [], + "config_overrides": None, } @@ -235,7 +242,7 @@ def test_bare_command(monkeypatch, logger, console): assert cmd.logger.verbosity == LogLevel.INFO assert cmd.logger is logger assert cmd.input is console - assert options == {} + assert options == {"config_overrides": None} @pytest.mark.skipif(sys.platform != "linux", reason="requires Linux") @@ -251,7 +258,7 @@ def test_linux_default(logger, console): assert cmd.logger.verbosity == LogLevel.INFO assert cmd.logger is logger assert cmd.input is console - assert options == {} + assert options == {"config_overrides": None} @pytest.mark.skipif(sys.platform != "darwin", reason="requires macOS") @@ -267,7 +274,7 @@ def test_macOS_default(logger, console): assert cmd.logger.verbosity == LogLevel.INFO assert cmd.logger is logger assert cmd.input is console - assert options == {} + assert options == {"config_overrides": None} @pytest.mark.skipif(sys.platform != "win32", reason="requires Windows") @@ -283,7 +290,7 @@ def test_windows_default(logger, console): assert cmd.logger.verbosity == LogLevel.INFO assert cmd.logger is logger assert cmd.input is console - assert options == {} + assert options == {"config_overrides": None} def test_bare_command_help(monkeypatch, capsys, logger, console): @@ -299,7 +306,8 @@ def test_bare_command_help(monkeypatch, capsys, logger, console): # Help message is for default platform and format output = capsys.readouterr().out assert output.startswith( - "usage: briefcase create macOS app [-h] [-v] [-V] [--no-input] [--log]\n" + "usage: briefcase create macOS app [-h] [-C KEY=VALUE] [-v] [-V] [--no-input]\n" + " [--log]\n" "\n" "Create and populate a macOS app.\n" ) @@ -346,7 +354,7 @@ def test_command_explicit_platform(monkeypatch, logger, console): assert cmd.logger.verbosity == LogLevel.INFO assert cmd.logger is logger assert cmd.input is console - assert options == {} + assert options == {"config_overrides": None} def test_command_explicit_platform_case_handling(monkeypatch, logger, console): @@ -364,7 +372,7 @@ def test_command_explicit_platform_case_handling(monkeypatch, logger, console): assert cmd.logger.verbosity == LogLevel.INFO assert cmd.logger is logger assert cmd.input is console - assert options == {} + assert options == {"config_overrides": None} def test_command_explicit_platform_help(monkeypatch, capsys, logger, console): @@ -380,7 +388,8 @@ def test_command_explicit_platform_help(monkeypatch, capsys, logger, console): # Help message is for default platform and format output = capsys.readouterr().out assert output.startswith( - "usage: briefcase create macOS app [-h] [-v] [-V] [--no-input] [--log]\n" + "usage: briefcase create macOS app [-h] [-C KEY=VALUE] [-v] [-V] [--no-input]\n" + " [--log]\n" "\n" "Create and populate a macOS app.\n" ) @@ -400,7 +409,7 @@ def test_command_explicit_format(monkeypatch, logger, console): assert cmd.logger.verbosity == LogLevel.INFO assert cmd.logger is logger assert cmd.input is console - assert options == {} + assert options == {"config_overrides": None} def test_command_unknown_format(monkeypatch, logger, console): @@ -446,7 +455,8 @@ def test_command_explicit_format_help(monkeypatch, capsys, logger, console): # Help message is for default platform, but app format output = capsys.readouterr().out assert output.startswith( - "usage: briefcase create macOS app [-h] [-v] [-V] [--no-input] [--log]\n" + "usage: briefcase create macOS app [-h] [-C KEY=VALUE] [-v] [-V] [--no-input]\n" + " [--log]\n" "\n" "Create and populate a macOS app.\n" ) @@ -466,7 +476,7 @@ def test_command_disable_input(monkeypatch, logger, console): assert cmd.logger.verbosity == LogLevel.INFO assert cmd.logger is logger assert cmd.input is console - assert options == {} + assert options == {"config_overrides": None} def test_command_options(monkeypatch, capsys, logger, console): @@ -483,7 +493,7 @@ def test_command_options(monkeypatch, capsys, logger, console): assert cmd.logger.verbosity == LogLevel.INFO assert cmd.logger is logger assert cmd.input is console - assert options == {"channel": "s3"} + assert options == {"config_overrides": None, "channel": "s3"} def test_unknown_command_options(monkeypatch, capsys, logger, console): @@ -501,7 +511,7 @@ def test_unknown_command_options(monkeypatch, capsys, logger, console): output = capsys.readouterr().err assert output.startswith( - "usage: briefcase publish macOS Xcode [-h] [-v] [-V] [--no-input] [--log]\n" - " [-c {s3}]\n" + "usage: briefcase publish macOS Xcode [-h] [-C KEY=VALUE] [-v] [-V]\n" + " [--no-input] [--log] [-c {s3}]\n" "briefcase publish macOS Xcode: error: unrecognized arguments: -x" ) From 115bae2dcfe33277698d52511929087d82201fe0 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Thu, 16 Nov 2023 14:50:13 +0800 Subject: [PATCH 3/9] Document the -C option. --- docs/reference/commands/index.rst | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/reference/commands/index.rst b/docs/reference/commands/index.rst index 9579131bf..e7629e78d 100644 --- a/docs/reference/commands/index.rst +++ b/docs/reference/commands/index.rst @@ -20,6 +20,19 @@ Common options The following options are available to all commands: +``-C `` / ``--config `` +--------------------------------------------- + +Override the value of an app's configuration in ``pyproject.toml`` with the provided +value. + +The value passed to the setting should be valid TOML. If the value being overridden is a +string, this includes quoting the value. This may require the use of escape sequences at +the command line. For example, to override the template used by the create command, you +can use ``-C template=...``, but the value must be quoted:: + + briefcase create -C template=\'https://example.com/template\' + ``-h`` / ``--help`` ------------------- From ee2e2c34b1af8215f561c18d213c31c6e2887c29 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Thu, 16 Nov 2023 14:50:24 +0800 Subject: [PATCH 4/9] Add a changenote. --- changes/1115.feature.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/1115.feature.rst diff --git a/changes/1115.feature.rst b/changes/1115.feature.rst new file mode 100644 index 000000000..76c59fe7f --- /dev/null +++ b/changes/1115.feature.rst @@ -0,0 +1 @@ +The ``-C``/``--config`` option can now be used to override app settings from the command line. From 8816ee0a05dd0d045a3ecc7eb2b0ea1ed1ffb0e0 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Thu, 16 Nov 2023 15:18:10 +0800 Subject: [PATCH 5/9] Fixes for Py3.8 compatibility. --- docs/reference/commands/index.rst | 11 +++++++---- src/briefcase/commands/dev.py | 15 ++++++++------- src/briefcase/commands/new.py | 11 ++++++----- src/briefcase/commands/upgrade.py | 9 +++++---- 4 files changed, 26 insertions(+), 20 deletions(-) diff --git a/docs/reference/commands/index.rst b/docs/reference/commands/index.rst index e7629e78d..6f46c75be 100644 --- a/docs/reference/commands/index.rst +++ b/docs/reference/commands/index.rst @@ -27,11 +27,14 @@ Override the value of an app's configuration in ``pyproject.toml`` with the prov value. The value passed to the setting should be valid TOML. If the value being overridden is a -string, this includes quoting the value. This may require the use of escape sequences at -the command line. For example, to override the template used by the create command, you -can use ``-C template=...``, but the value must be quoted:: +string, this means you must quote the value. This may require the use of escape +sequences at the command line to ensure the value provided to Briefcase by the shell +includes the quotes. - briefcase create -C template=\'https://example.com/template\' +For example, to override the template used by the create command, you can use ``-C +template=...``, but the value must be quoted:: + + briefcase create -C template=\"https://example.com/template\" ``-h`` / ``--help`` ------------------- diff --git a/src/briefcase/commands/dev.py b/src/briefcase/commands/dev.py index 2957afb5c..9082e4268 100644 --- a/src/briefcase/commands/dev.py +++ b/src/briefcase/commands/dev.py @@ -1,8 +1,9 @@ +from __future__ import annotations + import os import subprocess import sys from pathlib import Path -from typing import List, Optional from briefcase.commands.run import RunAppMixin from briefcase.config import AppConfig @@ -112,7 +113,7 @@ def run_dev_app( app: AppConfig, env: dict, test_mode: bool, - passthrough: List[str], + passthrough: list[str], **options, ): """Run the app in the dev environment. @@ -174,11 +175,11 @@ def get_environment(self, app, test_mode: bool): def __call__( self, - appname: Optional[str] = None, - update_requirements: Optional[bool] = False, - run_app: Optional[bool] = True, - test_mode: Optional[bool] = False, - passthrough: Optional[List[str]] = None, + appname: str | None = None, + update_requirements: bool | None = False, + run_app: bool | None = True, + test_mode: bool | None = False, + passthrough: list[str] | None = None, config_overrides: list[str] | None = None, **options, ): diff --git a/src/briefcase/commands/new.py b/src/briefcase/commands/new.py index aa24ce22b..b7ebc6f55 100644 --- a/src/briefcase/commands/new.py +++ b/src/briefcase/commands/new.py @@ -1,7 +1,8 @@ +from __future__ import annotations + import re import unicodedata from email.utils import parseaddr -from typing import Optional from urllib.parse import urlparse from packaging.version import Version @@ -432,8 +433,8 @@ def build_app_context(self): def new_app( self, - template: Optional[str] = None, - template_branch: Optional[str] = None, + template: str | None = None, + template_branch: str | None = None, **options, ): """Ask questions to generate a new application, and generate a stub project from @@ -520,8 +521,8 @@ def verify_tools(self): def __call__( self, - template: Optional[str] = None, - template_branch: Optional[str] = None, + template: str | None = None, + template_branch: str | None = None, **options, ): # Confirm host compatibility, and that all required tools are available. diff --git a/src/briefcase/commands/upgrade.py b/src/briefcase/commands/upgrade.py index c130d3847..c432a0b22 100644 --- a/src/briefcase/commands/upgrade.py +++ b/src/briefcase/commands/upgrade.py @@ -1,6 +1,7 @@ +from __future__ import annotations + import sys from operator import attrgetter -from typing import List, Set, Type from briefcase.exceptions import ( BriefcaseCommandError, @@ -50,12 +51,12 @@ def add_options(self, parser): help="The Briefcase-managed tool to upgrade. If no tool is named, all tools will be upgraded.", ) - def get_tools_to_upgrade(self, tool_list: Set[str]) -> List[ManagedTool]: + def get_tools_to_upgrade(self, tool_list: set[str]) -> list[ManagedTool]: """Returns set of managed Tools that can be upgraded. Raises ``BriefcaseCommandError`` if user list contains any invalid tool names. """ - upgrade_list: set[Type[Tool]] + upgrade_list: set[type[Tool]] tools_to_upgrade: set[ManagedTool] = set() # Validate user tool list against tool registry @@ -94,7 +95,7 @@ def get_tools_to_upgrade(self, tool_list: Set[str]) -> List[ManagedTool]: return sorted(list(tools_to_upgrade), key=attrgetter("name")) - def __call__(self, tool_list: List[str], list_tools: bool = False, **options): + def __call__(self, tool_list: list[str], list_tools: bool = False, **options): """Perform tool upgrades or list tools qualifying for upgrade. :param tool_list: List of tool names from user to upgrade. From d37db2b9306f8eccfe626cb170177d1a864eaada Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Fri, 17 Nov 2023 10:00:10 +0800 Subject: [PATCH 6/9] Remove periods from help text for consistency. --- src/briefcase/commands/base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/briefcase/commands/base.py b/src/briefcase/commands/base.py index bb18bbcda..cd5dd5413 100644 --- a/src/briefcase/commands/base.py +++ b/src/briefcase/commands/base.py @@ -726,14 +726,14 @@ def add_default_options(self, parser): dest="config_overrides", action="append", metavar="KEY=VALUE", - help="Override the value of the app configuration item KEY with VALUE.", + help="Override the value of the app configuration item KEY with VALUE", ) parser.add_argument( "-v", "--verbosity", action="count", default=0, - help="Enable verbose logging. Use -vv and -vvv to increase logging verbosity.", + help="Enable verbose logging. Use -vv and -vvv to increase logging verbosity", ) parser.add_argument("-V", "--version", action="version", version=__version__) parser.add_argument( From 5dfd9729247d82f46f184d33540217708e1c717f Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Fri, 17 Nov 2023 10:01:53 +0800 Subject: [PATCH 7/9] Explicitly pass exception context. --- src/briefcase/commands/base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/briefcase/commands/base.py b/src/briefcase/commands/base.py index cd5dd5413..9219ce706 100644 --- a/src/briefcase/commands/base.py +++ b/src/briefcase/commands/base.py @@ -117,10 +117,10 @@ def parse_config_overrides(config_overrides: list[str] | None) -> dict[str, Any] for value in config_overrides: try: overrides.update(tomllib.loads(value)) - except ValueError: + except ValueError as e: raise BriefcaseCommandError( f"Unable to parse configuration override {value}" - ) + ) from e return overrides From ead16a713be9c4c809080bad89d64322c434eb49 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Fri, 17 Nov 2023 12:34:44 +0800 Subject: [PATCH 8/9] Rework override process so overrides are validated. --- docs/reference/commands/index.rst | 6 + src/briefcase/__main__.py | 7 +- src/briefcase/commands/base.py | 42 +-- src/briefcase/commands/build.py | 3 +- src/briefcase/commands/create.py | 3 +- src/briefcase/commands/dev.py | 3 +- src/briefcase/commands/new.py | 2 +- src/briefcase/commands/open.py | 3 +- src/briefcase/commands/package.py | 3 +- src/briefcase/commands/publish.py | 6 +- src/briefcase/commands/run.py | 1 - src/briefcase/commands/update.py | 1 - src/briefcase/platforms/linux/appimage.py | 4 +- src/briefcase/platforms/linux/system.py | 4 +- src/briefcase/platforms/windows/__init__.py | 4 +- tests/commands/base/test_finalize.py | 113 +-------- tests/commands/base/test_parse_config.py | 113 ++++++++- .../base/test_parse_config_overrides.py | 27 +- tests/commands/base/test_parse_options.py | 35 ++- tests/commands/build/test_call.py | 38 +-- tests/commands/create/test_call.py | 40 --- tests/commands/dev/test_call.py | 22 +- tests/commands/new/test_call.py | 13 +- tests/commands/open/test_call.py | 6 +- tests/commands/package/test_call.py | 26 +- tests/commands/publish/test_call.py | 32 +-- tests/commands/run/test_call.py | 40 +-- tests/commands/update/test_call.py | 10 +- tests/platforms/android/gradle/test_run.py | 12 +- tests/platforms/iOS/xcode/test_run.py | 4 +- tests/platforms/linux/appimage/test_create.py | 14 +- tests/platforms/linux/system/test_create.py | 16 +- tests/platforms/macOS/app/test_package.py | 4 +- tests/platforms/web/static/test_run.py | 8 +- tests/platforms/windows/app/test_package.py | 4 +- tests/test_cmdline.py | 239 +++++++++++++----- 36 files changed, 493 insertions(+), 415 deletions(-) diff --git a/docs/reference/commands/index.rst b/docs/reference/commands/index.rst index 6f46c75be..174ea8203 100644 --- a/docs/reference/commands/index.rst +++ b/docs/reference/commands/index.rst @@ -26,6 +26,12 @@ The following options are available to all commands: Override the value of an app's configuration in ``pyproject.toml`` with the provided value. +The key will be applied *after* (and will therefore take precedence over) any platform +or backend-specific configuration has been merged into the app's configuration. The key +must be a "top level" TOML key. The use of `dotted keys +`__ to define nested configuration structures is not +permitted. + The value passed to the setting should be valid TOML. If the value being overridden is a string, this means you must quote the value. This may require the use of escape sequences at the command line to ensure the value provided to Briefcase by the shell diff --git a/src/briefcase/__main__.py b/src/briefcase/__main__.py index a868c422d..703052fde 100644 --- a/src/briefcase/__main__.py +++ b/src/briefcase/__main__.py @@ -20,8 +20,11 @@ def main(): try: Command, extra_cmdline = parse_cmdline(sys.argv[1:]) command = Command(logger=logger, console=console) - options = command.parse_options(extra=extra_cmdline) - command.parse_config(Path.cwd() / "pyproject.toml") + options, overrides = command.parse_options(extra=extra_cmdline) + command.parse_config( + Path.cwd() / "pyproject.toml", + overrides=overrides, + ) command(**options) except HelpText as e: logger.info() diff --git a/src/briefcase/commands/base.py b/src/briefcase/commands/base.py index 9219ce706..7cb9a42cb 100644 --- a/src/briefcase/commands/base.py +++ b/src/briefcase/commands/base.py @@ -114,12 +114,19 @@ def parse_config_overrides(config_overrides: list[str] | None) -> dict[str, Any] """ overrides = {} if config_overrides: - for value in config_overrides: + for override in config_overrides: try: - overrides.update(tomllib.loads(value)) + key, _ = override.split("=", 1) + if "." in key: + raise BriefcaseConfigError( + "Can't override multi-level configuration keys" + ) + + # Now actually parse the value + overrides.update(tomllib.loads(override)) except ValueError as e: - raise BriefcaseCommandError( - f"Unable to parse configuration override {value}" + raise BriefcaseConfigError( + f"Unable to parse configuration override {override}" ) from e return overrides @@ -551,11 +558,7 @@ def finalize_app_config(self, app: AppConfig): :param app: The app configuration to finalize. """ - def finalize( - self, - app: AppConfig | None = None, - config_overrides: list[str] | None = None, - ): + def finalize(self, app: AppConfig | None = None): """Finalize Briefcase configuration. This will: @@ -569,10 +572,6 @@ def finalize( :param app: If provided, the specific app configuration to finalize. By default, all apps will be finalized. """ - # Convert any configuration overrides provided at the command line - # into values that can be applied to the app's config - overrides = parse_config_overrides(config_overrides) - self.verify_host() self.verify_tools() @@ -580,14 +579,10 @@ def finalize( for app in self.apps.values(): if hasattr(app, "__draft__"): self.finalize_app_config(app) - for key, value in overrides.items(): - setattr(app, key, value) delattr(app, "__draft__") else: if hasattr(app, "__draft__"): self.finalize_app_config(app) - for key, value in overrides.items(): - setattr(app, key, value) delattr(app, "__draft__") def verify_app(self, app: AppConfig): @@ -647,7 +642,8 @@ def parse_options(self, extra): :param extra: the remaining command line arguments after the initial ArgumentParser runs over the command line. - :return: dictionary of parsed arguments for Command + :return: dictionary of parsed arguments for Command, and a dictionary of parsed + configuration overrides. """ default_format = getattr( get_platforms().get(self.platform), "DEFAULT_OUTPUT_FORMAT", None @@ -707,7 +703,10 @@ def parse_options(self, extra): self.logger.verbosity = options.pop("verbosity") self.logger.save_log = options.pop("save_log") - return options + # Parse the configuration overrides + overrides = parse_config_overrides(options.pop("config_overrides")) + + return options, overrides def clone_options(self, command): """Clone options from one command to this one. @@ -821,7 +820,7 @@ def add_options(self, parser): :param parser: a stub argparse parser for the command. """ - def parse_config(self, filename): + def parse_config(self, filename, overrides): try: with open(filename, "rb") as config_file: # Parse the content of the pyproject.toml file, extracting @@ -833,6 +832,8 @@ def parse_config(self, filename): output_format=self.output_format, ) + # Create the global config + global_config.update(overrides) self.global_config = create_config( klass=GlobalConfig, config=global_config, @@ -842,6 +843,7 @@ def parse_config(self, filename): for app_name, app_config in app_configs.items(): # Construct an AppConfig object with the final set of # configuration options for the app. + app_config.update(overrides) self.apps[app_name] = create_config( klass=AppConfig, config=app_config, diff --git a/src/briefcase/commands/build.py b/src/briefcase/commands/build.py index 71c09ad2d..9c3c7ddc4 100644 --- a/src/briefcase/commands/build.py +++ b/src/briefcase/commands/build.py @@ -88,7 +88,6 @@ def __call__( update_support: bool = False, no_update: bool = False, test_mode: bool = False, - config_overrides: list[str] | None = None, **options, ) -> dict | None: # Has the user requested an invalid set of options? @@ -113,7 +112,7 @@ def __call__( # Confirm host compatibility, that all required tools are available, # and that the app configuration is finalized. - self.finalize(app, config_overrides) + self.finalize(app) if app: state = self._build_app( diff --git a/src/briefcase/commands/create.py b/src/briefcase/commands/create.py index 1297b9580..4baa19caf 100644 --- a/src/briefcase/commands/create.py +++ b/src/briefcase/commands/create.py @@ -834,12 +834,11 @@ def verify_app_tools(self, app: AppConfig): def __call__( self, app: AppConfig | None = None, - config_overrides: list[str] | None = None, **options, ) -> dict | None: # Confirm host compatibility, that all required tools are available, # and that the app configuration is finalized. - self.finalize(app, config_overrides) + self.finalize(app) if app: state = self.create_app(app, **options) diff --git a/src/briefcase/commands/dev.py b/src/briefcase/commands/dev.py index 9082e4268..ee1a30ffb 100644 --- a/src/briefcase/commands/dev.py +++ b/src/briefcase/commands/dev.py @@ -180,7 +180,6 @@ def __call__( run_app: bool | None = True, test_mode: bool | None = False, passthrough: list[str] | None = None, - config_overrides: list[str] | None = None, **options, ): # Which app should we run? If there's only one defined @@ -202,7 +201,7 @@ def __call__( ) # Confirm host compatibility, that all required tools are available, # and that the app configuration is finalized. - self.finalize(app, config_overrides) + self.finalize(app) self.verify_app(app) diff --git a/src/briefcase/commands/new.py b/src/briefcase/commands/new.py index b7ebc6f55..7979aa5bc 100644 --- a/src/briefcase/commands/new.py +++ b/src/briefcase/commands/new.py @@ -76,7 +76,7 @@ def binary_path(self, app): """A placeholder; New command doesn't have a binary path.""" raise NotImplementedError() - def parse_config(self, filename): + def parse_config(self, filename, overrides): """There is no configuration when starting a new project; this implementation overrides the base so that no config is parsed.""" pass diff --git a/src/briefcase/commands/open.py b/src/briefcase/commands/open.py index ef572c508..5d7b34629 100644 --- a/src/briefcase/commands/open.py +++ b/src/briefcase/commands/open.py @@ -47,12 +47,11 @@ def open_app(self, app: AppConfig, **options): def __call__( self, app: AppConfig | None = None, - config_overrides: list[str] | None = None, **options, ): # Confirm host compatibility, that all required tools are available, # and that the app configuration is finalized. - self.finalize(app, config_overrides) + self.finalize(app) if app: state = self.open_app(app, **options) diff --git a/src/briefcase/commands/package.py b/src/briefcase/commands/package.py index dc1d36188..465fc4774 100644 --- a/src/briefcase/commands/package.py +++ b/src/briefcase/commands/package.py @@ -137,12 +137,11 @@ def __call__( self, app: AppConfig | None = None, update: bool = False, - config_overrides: list[str] | None = None, **options, ) -> dict | None: # Confirm host compatibility, that all required tools are available, # and that the app configuration is finalized. - self.finalize(app, config_overrides) + self.finalize(app) if app: state = self._package_app(app, update=update, **options) diff --git a/src/briefcase/commands/publish.py b/src/briefcase/commands/publish.py index 689089f8f..c710fdee0 100644 --- a/src/briefcase/commands/publish.py +++ b/src/briefcase/commands/publish.py @@ -55,7 +55,11 @@ def _publish_app(self, app: AppConfig, channel: str, **options) -> dict | None: return state - def __call__(self, channel=None, **options): + def __call__( + self, + channel: str | None = None, + **options, + ): # Confirm host compatibility, that all required tools are available, # and that all app configurations are finalized. self.finalize() diff --git a/src/briefcase/commands/run.py b/src/briefcase/commands/run.py index 7e491f9ec..6a54b499d 100644 --- a/src/briefcase/commands/run.py +++ b/src/briefcase/commands/run.py @@ -248,7 +248,6 @@ def __call__( no_update: bool = False, test_mode: bool = False, passthrough: list[str] | None = None, - config_overrides: list[str] | None = None, **options, ) -> dict | None: # Which app should we run? If there's only one defined diff --git a/src/briefcase/commands/update.py b/src/briefcase/commands/update.py index 4d929784d..4b3809fcc 100644 --- a/src/briefcase/commands/update.py +++ b/src/briefcase/commands/update.py @@ -68,7 +68,6 @@ def __call__( update_resources: bool = False, update_support: bool = False, test_mode: bool = False, - config_overrides: list[str] | None = None, **options, ) -> dict | None: # Confirm host compatibility, that all required tools are available, diff --git a/src/briefcase/platforms/linux/appimage.py b/src/briefcase/platforms/linux/appimage.py index ceaa4c85f..09447cebc 100644 --- a/src/briefcase/platforms/linux/appimage.py +++ b/src/briefcase/platforms/linux/appimage.py @@ -70,9 +70,9 @@ def add_options(self, parser): def parse_options(self, extra): """Extract the use_docker option.""" - options = super().parse_options(extra) + options, overrides = super().parse_options(extra) self.use_docker = options.pop("use_docker") - return options + return options, overrides def clone_options(self, command): """Clone the use_docker option.""" diff --git a/src/briefcase/platforms/linux/system.py b/src/briefcase/platforms/linux/system.py index 12792e869..5b0615524 100644 --- a/src/briefcase/platforms/linux/system.py +++ b/src/briefcase/platforms/linux/system.py @@ -365,10 +365,10 @@ def add_options(self, parser): def parse_options(self, extra): """Extract the target_image option.""" - options = super().parse_options(extra) + options, overrides = super().parse_options(extra) self.target_image = options.pop("target") - return options + return options, overrides def clone_options(self, command): """Clone the target_image option.""" diff --git a/src/briefcase/platforms/windows/__init__.py b/src/briefcase/platforms/windows/__init__.py index dc2821702..deed7bb9e 100644 --- a/src/briefcase/platforms/windows/__init__.py +++ b/src/briefcase/platforms/windows/__init__.py @@ -229,9 +229,9 @@ def add_options(self, parser): def parse_options(self, extra): """Require the Windows SDK tool if an `identity` is specified for signing.""" - options = super().parse_options(extra=extra) + options, overrides = super().parse_options(extra=extra) self._windows_sdk_needed = options["identity"] is not None - return options + return options, overrides def sign_file( self, diff --git a/tests/commands/base/test_finalize.py b/tests/commands/base/test_finalize.py index 982674f91..e0740702b 100644 --- a/tests/commands/base/test_finalize.py +++ b/tests/commands/base/test_finalize.py @@ -38,56 +38,9 @@ def base_command(tmp_path, first_app, second_app): ) -@pytest.mark.parametrize( - "overrides,first_app_values,second_app_values", - [ - # No overrides - ( - None, - { - "version": "0.0.1", - "description": "The first simple app", - }, - { - "version": "0.0.2", - "description": "The second simple app", - }, - ), - # Multiple overrides, different types - ( - [ - "version='42.37'", - "key1='val1'", - "key2=2", - "key3=[1, 'two']", - ], - { - "version": "42.37", - "description": "The first simple app", - "key1": "val1", - "key2": 2, - "key3": [1, "two"], - }, - { - "version": "42.37", - "description": "The second simple app", - "key1": "val1", - "key2": 2, - "key3": [1, "two"], - }, - ), - ], -) -def test_finalize_all( - base_command, - first_app, - second_app, - overrides, - first_app_values, - second_app_values, -): +def test_finalize_all(base_command, first_app, second_app): "A call to finalize verifies host, tools, and finalized all app configs" - base_command.finalize(config_overrides=overrides) + base_command.finalize() # The right sequence of things will be done assert base_command.actions == [ @@ -105,60 +58,10 @@ def test_finalize_all( assert not hasattr(first_app, "__draft__") assert not hasattr(second_app, "__draft__") - # Override values have been set - for key, value in first_app_values.items(): - assert getattr(first_app, key) == value - for key, value in second_app_values.items(): - assert getattr(second_app, key) == value - - -@pytest.mark.parametrize( - "overrides,first_app_values,second_app_values", - [ - # No overrides - ( - None, - { - "version": "0.0.1", - "description": "The first simple app", - }, - { - "version": "0.0.2", - "description": "The second simple app", - }, - ), - # Multiple overrides, different types - ( - [ - "version='42.37'", - "key1='val1'", - "key2=2", - "key3=[1, 'two']", - ], - { - "version": "42.37", - "description": "The first simple app", - "key1": "val1", - "key2": 2, - "key3": [1, "two"], - }, - { - "version": "0.0.2", - "description": "The second simple app", - }, - ), - ], -) -def test_finalize_single( - base_command, - first_app, - second_app, - overrides, - first_app_values, - second_app_values, -): + +def test_finalize_single(base_command, first_app, second_app): "A call to finalize verifies host, tools, and finalized all app configs" - base_command.finalize(first_app, overrides) + base_command.finalize(first_app) # The right sequence of things will be done assert base_command.actions == [ @@ -174,12 +77,6 @@ def test_finalize_single( assert not hasattr(first_app, "__draft__") assert hasattr(second_app, "__draft__") - # Override values have been set - for key, value in first_app_values.items(): - assert getattr(first_app, key) == value - for key, value in second_app_values.items(): - assert getattr(second_app, key) == value - def test_finalize_all_repeat(base_command, first_app, second_app): "Multiple calls to finalize verifies host & tools multiple times, but only once on config" diff --git a/tests/commands/base/test_parse_config.py b/tests/commands/base/test_parse_config.py index 7f89c5a72..03fa549f6 100644 --- a/tests/commands/base/test_parse_config.py +++ b/tests/commands/base/test_parse_config.py @@ -9,7 +9,7 @@ def test_missing_config(base_command): """If the configuration file doesn't exit, raise an error.""" filename = base_command.base_path / "does_not_exist.toml" with pytest.raises(BriefcaseConfigError, match="Configuration file not found"): - base_command.parse_config(filename) + base_command.parse_config(filename, {}) def test_incomplete_global_config(base_command): @@ -32,7 +32,7 @@ def test_incomplete_global_config(base_command): BriefcaseConfigError, match=r"Global configuration is incomplete \(missing 'bundle', 'project_name'\)", ): - base_command.parse_config(filename) + base_command.parse_config(filename, {}) def test_incomplete_config(base_command): @@ -56,11 +56,11 @@ def test_incomplete_config(base_command): BriefcaseConfigError, match=r"Configuration for 'my-app' is incomplete \(missing 'sources'\)", ): - base_command.parse_config(filename) + base_command.parse_config(filename, {}) def test_parse_config(base_command): - """A well-formed configuration file can be augmented by the command line.""" + """A well-formed configuration file can turned into a set of configurations.""" filename = base_command.base_path / "pyproject.toml" create_file( filename, @@ -82,8 +82,8 @@ def test_parse_config(base_command): """, ) - # Parse the configuration - base_command.parse_config(filename) + # Parse the configuration, no overrides + base_command.parse_config(filename, {}) # There is a global configuration object assert repr(base_command.global_config) == "" @@ -113,3 +113,104 @@ def test_parse_config(base_command): assert base_command.apps["secondapp"].bundle == "org.beeware" assert base_command.apps["secondapp"].mystery == "sekrits" assert base_command.apps["secondapp"].extra == "something" + + +def test_parse_config_with_overrides(base_command): + """A well-formed configuration file can be augmented by the command line.""" + filename = base_command.base_path / "pyproject.toml" + create_file( + filename, + """ + [tool.briefcase] + project_name = "Sample project" + version = "1.2.3" + description = "A sample app" + bundle = "org.beeware" + mystery = 'default' + + [tool.briefcase.app.firstapp] + sources = ['src/firstapp'] + + [tool.briefcase.app.secondapp] + sources = ['src/secondapp'] + extra = 'something' + mystery = 'sekrits' + """, + ) + + # Parse the configuration, with overrides + base_command.parse_config( + filename, + { + "version": "2.3.4", + "custom": "something special", + "mystery": "overridden", + }, + ) + + # There is a global configuration object. It picks up the override values. + assert repr(base_command.global_config) == "" + assert base_command.global_config.project_name == "Sample project" + assert base_command.global_config.bundle == "org.beeware" + assert base_command.global_config.version == "2.3.4" + assert base_command.global_config.custom == "something special" + assert base_command.global_config.mystery == "overridden" + + # The first app will have all the base attributes required by an app, + # defined in the config file, with the values from the overrides taking precedence. + assert ( + repr(base_command.apps["firstapp"]) == "" + ) + assert base_command.apps["firstapp"].project_name == "Sample project" + assert base_command.apps["firstapp"].app_name == "firstapp" + assert base_command.apps["firstapp"].bundle == "org.beeware" + assert base_command.apps["firstapp"].version == "2.3.4" + assert base_command.apps["firstapp"].custom == "something special" + assert base_command.apps["firstapp"].mystery == "overridden" + assert not hasattr(base_command.apps["firstapp"], "extra") + + # The second app is much the same. The value for `mystery`` is overridden by the + # command line (superseding the app override); but the `extra` app-specific value + # is preserved. + assert ( + repr(base_command.apps["secondapp"]) + == "" + ) + assert base_command.apps["secondapp"].project_name == "Sample project" + assert base_command.apps["secondapp"].app_name == "secondapp" + assert base_command.apps["secondapp"].bundle == "org.beeware" + assert base_command.apps["secondapp"].version == "2.3.4" + assert base_command.apps["secondapp"].custom == "something special" + assert base_command.apps["secondapp"].mystery == "overridden" + assert base_command.apps["secondapp"].extra == "something" + + +def test_parse_config_with_invalid_override(base_command): + """If an override value doesn't pass validation, an exception is raised.""" + filename = base_command.base_path / "pyproject.toml" + create_file( + filename, + """ + [tool.briefcase] + project_name = "Sample project" + version = "1.2.3" + description = "A sample app" + bundle = "org.beeware" + mystery = 'default' + + [tool.briefcase.app.firstapp] + sources = ['src/firstapp'] + """, + ) + + # Parse the configuration, with overrides + with pytest.raises( + BriefcaseConfigError, + match=r"Version numbers must be PEP440 compliant", + ): + base_command.parse_config( + filename, + { + "version": "not-a-version-number", + }, + ) diff --git a/tests/commands/base/test_parse_config_overrides.py b/tests/commands/base/test_parse_config_overrides.py index b1b2d208e..04358357c 100644 --- a/tests/commands/base/test_parse_config_overrides.py +++ b/tests/commands/base/test_parse_config_overrides.py @@ -1,7 +1,7 @@ import pytest from briefcase.commands.base import parse_config_overrides -from briefcase.exceptions import BriefcaseCommandError +from briefcase.exceptions import BriefcaseConfigError @pytest.mark.parametrize( @@ -52,25 +52,26 @@ def test_valid_overrides(overrides, values): @pytest.mark.parametrize( - "overrides", + "overrides, message", [ # Bare string - ["foobar"], + (["foobar"], r"Unable to parse configuration override "), # Unquoted string - ["key=foobar"], + (["key=foobar"], r"Unable to parse configuration override "), # Unbalanced quote - ["key='foobar"], + (["key='foobar"], r"Unable to parse configuration override "), # Unmatched brackets - ['key=[1, "two",'], + (['key=[1, "two",'], r"Unable to parse configuration override "), # Unmatches parentheses - ['key={a=1, b="two"'], + (['key={a=1, b="two"'], r"Unable to parse configuration override "), # Valid value followed by invalid - ["good=42", "key=foobar"], + (["good=42", "key=foobar"], r"Unable to parse configuration override "), + # Space in the key. + (["spacy key=42"], r"Unable to parse configuration override "), + # Multi-level key. This is legal TOML, but difficult to merge. + (["multi.level.key=42"], r"Can't override multi-level configuration keys"), ], ) -def test_invalid_overrides(overrides): - with pytest.raises( - BriefcaseCommandError, - match=r"Unable to parse configuration override ", - ): +def test_invalid_overrides(overrides, message): + with pytest.raises(BriefcaseConfigError, match=message): parse_config_overrides(overrides) diff --git a/tests/commands/base/test_parse_options.py b/tests/commands/base/test_parse_options.py index ba0109d48..326d41049 100644 --- a/tests/commands/base/test_parse_options.py +++ b/tests/commands/base/test_parse_options.py @@ -3,9 +3,30 @@ from briefcase.console import LogLevel -def test_parse_options(base_command): - """Command line options are parsed if provided.""" - options = base_command.parse_options( +def test_parse_options_no_overrides(base_command): + """Command line options are parsed if provided without overrides.""" + options, overrides = base_command.parse_options( + extra=( + "-x", + "wibble", + "-r", + "important", + ) + ) + + assert options == { + "extra": "wibble", + "mystery": None, + "required": "important", + } + assert overrides == {} + assert base_command.input.enabled + assert base_command.logger.verbosity == LogLevel.INFO + + +def test_parse_options_with_overrides(base_command): + """Command line options and overrides are parsed if provided.""" + options, overrides = base_command.parse_options( extra=( "-x", "wibble", @@ -22,10 +43,10 @@ def test_parse_options(base_command): "extra": "wibble", "mystery": None, "required": "important", - "config_overrides": [ - "width=10", - "height=20", - ], + } + assert overrides == { + "width": 10, + "height": 20, } assert base_command.input.enabled assert base_command.logger.verbosity == LogLevel.INFO diff --git a/tests/commands/build/test_call.py b/tests/commands/build/test_call.py index 826afb250..3a5caa040 100644 --- a/tests/commands/build/test_call.py +++ b/tests/commands/build/test_call.py @@ -12,7 +12,7 @@ def test_specific_app(build_command, first_app, second_app): } # Configure no command line options - options = build_command.parse_options([]) + options, _ = build_command.parse_options([]) # Run the build command build_command(first_app, **options) @@ -43,7 +43,7 @@ def test_multiple_apps(build_command, first_app, second_app): } # Configure no command line options - options = build_command.parse_options([]) + options, _ = build_command.parse_options([]) # Run the build command build_command(**options) @@ -81,7 +81,7 @@ def test_non_existent(build_command, first_app_config, second_app): } # Configure no command line options - options = build_command.parse_options([]) + options, _ = build_command.parse_options([]) # Run the build command build_command(**options) @@ -125,7 +125,7 @@ def test_unbuilt(build_command, first_app_unbuilt, second_app): } # Configure no command line options - options = build_command.parse_options([]) + options, _ = build_command.parse_options([]) # Run the build command build_command(**options) @@ -163,7 +163,7 @@ def test_update_app(build_command, first_app, second_app): } # Configure a -a command line option - options = build_command.parse_options(["-u"]) + options, _ = build_command.parse_options(["-u"]) # Run the build command build_command(**options) @@ -227,7 +227,7 @@ def test_update_app_requirements(build_command, first_app, second_app): } # Configure update command line options - options = build_command.parse_options(["-r"]) + options, _ = build_command.parse_options(["-r"]) # Run the build command build_command(**options) @@ -291,7 +291,7 @@ def test_update_app_resources(build_command, first_app, second_app): } # Configure update command line options - options = build_command.parse_options(["--update-resources"]) + options, _ = build_command.parse_options(["--update-resources"]) # Run the build command build_command(**options) @@ -355,7 +355,7 @@ def test_update_non_existent(build_command, first_app_config, second_app): } # Configure no command line options - options = build_command.parse_options(["-u"]) + options, _ = build_command.parse_options(["-u"]) # Run the build command build_command(**options) @@ -415,7 +415,7 @@ def test_update_unbuilt(build_command, first_app_unbuilt, second_app): } # Configure no command line options - options = build_command.parse_options(["-u"]) + options, _ = build_command.parse_options(["-u"]) # Run the build command build_command(**options) @@ -479,7 +479,7 @@ def test_build_test(build_command, first_app, second_app): } # Configure command line options - options = build_command.parse_options(["--test"]) + options, _ = build_command.parse_options(["--test"]) # Run the build command build_command(**options) @@ -544,7 +544,7 @@ def test_build_test_no_update(build_command, first_app, second_app): } # Configure command line options - options = build_command.parse_options(["--test", "--no-update"]) + options, _ = build_command.parse_options(["--test", "--no-update"]) # Run the build command build_command(**options) @@ -587,7 +587,7 @@ def test_build_test_update_dependencies(build_command, first_app, second_app): } # Configure command line options - options = build_command.parse_options(["--test", "-r"]) + options, _ = build_command.parse_options(["--test", "-r"]) # Run the build command build_command(**options) @@ -652,7 +652,7 @@ def test_build_test_update_resources(build_command, first_app, second_app): } # Configure command line options - options = build_command.parse_options(["--test", "--update-resources"]) + options, _ = build_command.parse_options(["--test", "--update-resources"]) # Run the build command build_command(**options) @@ -716,7 +716,7 @@ def test_build_invalid_update(build_command, first_app, second_app): } # Configure command line options - options = build_command.parse_options(["-u", "--no-update"]) + options, _ = build_command.parse_options(["-u", "--no-update"]) # Run the build command with pytest.raises( @@ -736,7 +736,7 @@ def test_build_invalid_update_requirements(build_command, first_app, second_app) } # Configure command line options - options = build_command.parse_options(["-r", "--no-update"]) + options, _ = build_command.parse_options(["-r", "--no-update"]) # Run the build command with pytest.raises( @@ -756,7 +756,7 @@ def test_build_invalid_update_resources(build_command, first_app, second_app): } # Configure command line options - options = build_command.parse_options(["--update-resources", "--no-update"]) + options, _ = build_command.parse_options(["--update-resources", "--no-update"]) # Run the build command with pytest.raises( @@ -776,7 +776,7 @@ def test_build_invalid_update_support(build_command, first_app, second_app): } # Configure command line options - options = build_command.parse_options(["--update-support", "--no-update"]) + options, _ = build_command.parse_options(["--update-support", "--no-update"]) # Run the build command with pytest.raises( @@ -795,7 +795,7 @@ def test_test_app_non_existent(build_command, first_app_config, second_app): } # Configure command line options - options = build_command.parse_options(["-u", "--test"]) + options, _ = build_command.parse_options(["-u", "--test"]) # Run the build command build_command(**options) @@ -856,7 +856,7 @@ def test_test_app_unbuilt(build_command, first_app_unbuilt, second_app): } # Configure command line options - options = build_command.parse_options(["-u", "--test"]) + options, _ = build_command.parse_options(["-u", "--test"]) # Run the build command build_command(**options) diff --git a/tests/commands/create/test_call.py b/tests/commands/create/test_call.py index e14d8d1e1..156013990 100644 --- a/tests/commands/create/test_call.py +++ b/tests/commands/create/test_call.py @@ -92,43 +92,3 @@ def test_create_single(tracking_create_command, tmp_path): assert not ( tmp_path / "base_path" / "build" / "second" / "tester" / "dummy" / "new" ).exists() - - -def test_create_single_with_overrides(tracking_create_command, tmp_path): - """Command line overrides can be used during app creation.""" - tracking_create_command( - app=tracking_create_command.apps["first"], - config_overrides=["template='https://example.com/override'"], - ) - - # The right sequence of things will be done - assert tracking_create_command.actions == [ - # Host OS is verified - ("verify-host",), - # Tools are verified - ("verify-tools",), - # App config has been finalized - ("finalize-app-config", "first"), - # Create the first app - ("generate", "first"), - ("support", "first"), - ("verify-app-template", "first"), - ("verify-app-tools", "first"), - ("code", "first", False), - ("requirements", "first", False), - ("resources", "first"), - ("cleanup", "first"), - ] - - # New app content has been created - assert ( - tmp_path / "base_path" / "build" / "first" / "tester" / "dummy" / "new" - ).exists() - assert not ( - tmp_path / "base_path" / "build" / "second" / "tester" / "dummy" / "new" - ).exists() - - # The command line override has been applied. - assert ( - tracking_create_command.apps["first"].template == "https://example.com/override" - ) diff --git a/tests/commands/dev/test_call.py b/tests/commands/dev/test_call.py index 075122a0a..3b1293c09 100644 --- a/tests/commands/dev/test_call.py +++ b/tests/commands/dev/test_call.py @@ -65,7 +65,7 @@ def test_no_args_one_app(dev_command, first_app): } # Configure no command line options - options = dev_command.parse_options([]) + options, _ = dev_command.parse_options([]) # Run the run command dev_command(**options) @@ -94,7 +94,7 @@ def test_no_args_two_apps(dev_command, first_app, second_app): } # Configure no command line options - options = dev_command.parse_options([]) + options, _ = dev_command.parse_options([]) # Invoking the run command raises an error with pytest.raises(BriefcaseCommandError): @@ -112,7 +112,7 @@ def test_with_arg_one_app(dev_command, first_app): } # Configure a -a command line option - options = dev_command.parse_options(["-a", "first"]) + options, _ = dev_command.parse_options(["-a", "first"]) # Run the run command dev_command(**options) @@ -141,7 +141,7 @@ def test_with_arg_two_apps(dev_command, first_app, second_app): } # Configure a --app command line option - options = dev_command.parse_options(["--app", "second"]) + options, _ = dev_command.parse_options(["--app", "second"]) # Run the run command dev_command(**options) @@ -171,7 +171,7 @@ def test_bad_app_reference(dev_command, first_app, second_app): } # Configure a --app command line option - options = dev_command.parse_options(["--app", "does-not-exist"]) + options, _ = dev_command.parse_options(["--app", "does-not-exist"]) # Invoking the run command raises an error with pytest.raises(BriefcaseCommandError): @@ -189,7 +189,7 @@ def test_update_requirements(dev_command, first_app): } # Configure a requirements update - options = dev_command.parse_options(["-r"]) + options, _ = dev_command.parse_options(["-r"]) # Run the run command dev_command(**options) @@ -219,7 +219,7 @@ def test_run_uninstalled(dev_command, first_app_uninstalled): } # Configure no command line options - options = dev_command.parse_options([]) + options, _ = dev_command.parse_options([]) # Run the run command dev_command(**options) @@ -250,7 +250,7 @@ def test_update_uninstalled(dev_command, first_app_uninstalled): } # Configure a requirements update - options = dev_command.parse_options(["-r"]) + options, _ = dev_command.parse_options(["-r"]) # Run the run command dev_command(**options) @@ -280,7 +280,7 @@ def test_no_run(dev_command, first_app_uninstalled): } # Configure an update without run - options = dev_command.parse_options(["--no-run"]) + options, _ = dev_command.parse_options(["--no-run"]) # Run the run command dev_command(**options) @@ -308,7 +308,7 @@ def test_run_test(dev_command, first_app): } # Configure the test option - options = dev_command.parse_options(["--test"]) + options, _ = dev_command.parse_options(["--test"]) # Run the run command dev_command(**options) @@ -336,7 +336,7 @@ def test_run_test_uninstalled(dev_command, first_app_uninstalled): } # Configure the test option - options = dev_command.parse_options(["--test"]) + options, _ = dev_command.parse_options(["--test"]) # Run the run command dev_command(**options) diff --git a/tests/commands/new/test_call.py b/tests/commands/new/test_call.py index ef3bb1423..04648626e 100644 --- a/tests/commands/new/test_call.py +++ b/tests/commands/new/test_call.py @@ -21,14 +21,14 @@ def monkeypatch_verify_git(*a, **kw): def test_parse_config(new_command): """Attempting to parse the config is a no-op when invoking new.""" - assert new_command.parse_config("some_file.toml") is None + assert new_command.parse_config("some_file.toml", {}) is None def test_new_app(new_command): """A new application can be created.""" # Configure no command line options - options = new_command.parse_options([]) + options, _ = new_command.parse_options([]) # Run the run command new_command(**options) @@ -40,12 +40,5 @@ def test_new_app(new_command): # Tools are verified ("verify-tools",), # Run the first app - ( - "new", - { - "config_overrides": None, - "template": None, - "template_branch": None, - }, - ), + ("new", {"template": None, "template_branch": None}), ] diff --git a/tests/commands/open/test_call.py b/tests/commands/open/test_call.py index ce6f3c404..ed500e601 100644 --- a/tests/commands/open/test_call.py +++ b/tests/commands/open/test_call.py @@ -1,7 +1,7 @@ def test_open(open_command, first_app, second_app): """The open command can be called.""" # Configure no command line options - options = open_command.parse_options([]) + options, _ = open_command.parse_options([]) open_command(**options) @@ -32,7 +32,7 @@ def test_open(open_command, first_app, second_app): def test_open_single(open_command, first_app): """The open command can be called to open a single app from the config.""" # Configure no command line options - options = open_command.parse_options([]) + options, _ = open_command.parse_options([]) open_command(app=open_command.apps["first"], **options) @@ -56,7 +56,7 @@ def test_open_single(open_command, first_app): def test_create_before_open(open_command, tmp_path): """If the app doesn't exist, it will be created before opening.""" # Configure no command line options - options = open_command.parse_options([]) + options, _ = open_command.parse_options([]) open_command(app=open_command.apps["first"], **options) diff --git a/tests/commands/package/test_call.py b/tests/commands/package/test_call.py index 096236dce..a4a1d10a3 100644 --- a/tests/commands/package/test_call.py +++ b/tests/commands/package/test_call.py @@ -9,7 +9,7 @@ def test_no_args_package_one_app(package_command, first_app, tmp_path): } # Configure no command line options - options = package_command.parse_options([]) + options, _ = package_command.parse_options([]) # Run the run command package_command(**options) @@ -53,7 +53,7 @@ def test_package_one_explicit_app(package_command, first_app, second_app, tmp_pa } # Configure no command line arguments - options = package_command.parse_options([]) + options, _ = package_command.parse_options([]) # Run the build command on a specific app package_command(first_app, **options) @@ -98,7 +98,7 @@ def test_no_args_package_two_app(package_command, first_app, second_app, tmp_pat } # Configure no command line options - options = package_command.parse_options([]) + options, _ = package_command.parse_options([]) # Run the package command package_command(**options) @@ -159,7 +159,7 @@ def test_identity_arg_package_one_app(package_command, first_app, tmp_path): } # Configure an identity option - options = package_command.parse_options(["--identity", "test"]) + options, _ = package_command.parse_options(["--identity", "test"]) # Run the run command package_command(**options) @@ -203,7 +203,7 @@ def test_adhoc_sign_package_one_app(package_command, first_app, tmp_path): } # Configure an ad-hoc signing option - options = package_command.parse_options(["--adhoc-sign"]) + options, _ = package_command.parse_options(["--adhoc-sign"]) # Run the run command package_command(**options) @@ -252,7 +252,7 @@ def test_adhoc_sign_args_package_two_app( } # Configure adhoc command line options - options = package_command.parse_options(["--adhoc-sign"]) + options, _ = package_command.parse_options(["--adhoc-sign"]) # Run the package command package_command(**options) @@ -316,7 +316,7 @@ def test_identity_sign_args_package_two_app( } # Configure an identity option - options = package_command.parse_options(["--identity", "test"]) + options, _ = package_command.parse_options(["--identity", "test"]) # Run the run command package_command(**options) @@ -376,7 +376,7 @@ def test_package_alternate_format(package_command, first_app, tmp_path): } # Configure command line options with an alternate format - options = package_command.parse_options(["--packaging-format", "box"]) + options, _ = package_command.parse_options(["--packaging-format", "box"]) # Run the run command package_command(**options) @@ -419,7 +419,7 @@ def test_create_before_package(package_command, first_app_config, tmp_path): } # Configure no command line options - options = package_command.parse_options([]) + options, _ = package_command.parse_options([]) # Run the run command package_command(**options) @@ -482,7 +482,7 @@ def test_update_package_one_app(package_command, first_app, tmp_path): } # Configure an update option - options = package_command.parse_options(["-u"]) + options, _ = package_command.parse_options(["-u"]) # Run the run command package_command(**options) @@ -549,7 +549,7 @@ def test_update_package_two_app(package_command, first_app, second_app, tmp_path } # Configure an update option - options = package_command.parse_options(["--update"]) + options, _ = package_command.parse_options(["--update"]) # Run the package command package_command(**options) @@ -662,7 +662,7 @@ def test_build_before_package(package_command, first_app_unbuilt, tmp_path): } # Configure no command line options - options = package_command.parse_options([]) + options, _ = package_command.parse_options([]) # Run the run command package_command(**options) @@ -719,7 +719,7 @@ def test_already_packaged(package_command, first_app, tmp_path): create_file(artefact_path, "Packaged app") # Configure no command line options - options = package_command.parse_options([]) + options, _ = package_command.parse_options([]) # Run the run command package_command(**options) diff --git a/tests/commands/publish/test_call.py b/tests/commands/publish/test_call.py index d2767bcf4..4b5544388 100644 --- a/tests/commands/publish/test_call.py +++ b/tests/commands/publish/test_call.py @@ -12,7 +12,7 @@ def test_publish(publish_command, first_app, second_app): } # Configure no command line options - options = publish_command.parse_options([]) + options, _ = publish_command.parse_options([]) # Run the publish command publish_command(**options) @@ -31,21 +31,13 @@ def test_publish(publish_command, first_app, second_app): # App tools are verified for first app ("verify-app-tools", "first"), # Publish the first app to s3 - ("publish", "first", "s3", {"config_overrides": None}), + ("publish", "first", "s3", {}), # App template is verified for second app ("verify-app-template", "second"), # App tools are verified for second app ("verify-app-tools", "second"), # Publish the second app to s3 - ( - "publish", - "second", - "s3", - { - "config_overrides": None, - "publish_state": "first", - }, - ), + ("publish", "second", "s3", {"publish_state": "first"}), ] @@ -58,7 +50,7 @@ def test_publish_alternative_channel(publish_command, first_app, second_app): } # Configure no command line options - options = publish_command.parse_options(["-c", "alternative"]) + options, _ = publish_command.parse_options(["-c", "alternative"]) # Run the publish command publish_command(**options) @@ -77,21 +69,13 @@ def test_publish_alternative_channel(publish_command, first_app, second_app): # App tools are verified for first app ("verify-app-tools", "first"), # Publish the first app to the alternative channel - ("publish", "first", "alternative", {"config_overrides": None}), + ("publish", "first", "alternative", {}), # App template is verified for second app ("verify-app-template", "second"), # App tools are verified for second app ("verify-app-tools", "second"), # Publish the second app to the alternative channel - ( - "publish", - "second", - "alternative", - { - "config_overrides": None, - "publish_state": "first", - }, - ), + ("publish", "second", "alternative", {"publish_state": "first"}), ] @@ -104,7 +88,7 @@ def test_non_existent(publish_command, first_app_config, second_app): } # Configure no command line options - options = publish_command.parse_options([]) + options, _ = publish_command.parse_options([]) # Invoking the publish command raises an error with pytest.raises(BriefcaseCommandError): @@ -132,7 +116,7 @@ def test_unbuilt(publish_command, first_app_unbuilt, second_app): } # Configure no command line options - options = publish_command.parse_options([]) + options, _ = publish_command.parse_options([]) # Invoking the publish command raises an error with pytest.raises(BriefcaseCommandError): diff --git a/tests/commands/run/test_call.py b/tests/commands/run/test_call.py index 6145f9f78..899bdb611 100644 --- a/tests/commands/run/test_call.py +++ b/tests/commands/run/test_call.py @@ -11,7 +11,7 @@ def test_no_args_one_app(run_command, first_app): } # Configure no command line options - options = run_command.parse_options([]) + options, _ = run_command.parse_options([]) # Run the run command run_command(**options) @@ -42,7 +42,7 @@ def test_no_args_one_app_with_passthrough(run_command, first_app): } # Configure no command line options - options = run_command.parse_options(["--", "foo", "--bar"]) + options, _ = run_command.parse_options(["--", "foo", "--bar"]) # Run the run command run_command(**options) @@ -73,7 +73,7 @@ def test_no_args_two_apps(run_command, first_app, second_app): } # Configure no command line options - options = run_command.parse_options([]) + options, _ = run_command.parse_options([]) # Invoking the run command raises an error with pytest.raises(BriefcaseCommandError): @@ -91,7 +91,7 @@ def test_with_arg_one_app(run_command, first_app): } # Configure a -a command line option - options = run_command.parse_options(["-a", "first"]) + options, _ = run_command.parse_options(["-a", "first"]) # Run the run command run_command(**options) @@ -122,7 +122,7 @@ def test_with_arg_two_apps(run_command, first_app, second_app): } # Configure a --app command line option - options = run_command.parse_options(["--app", "second"]) + options, _ = run_command.parse_options(["--app", "second"]) # Run the run command run_command(**options) @@ -154,7 +154,7 @@ def test_bad_app_reference(run_command, first_app, second_app): } # Configure a --app command line option - options = run_command.parse_options(["--app", "does-not-exist"]) + options, _ = run_command.parse_options(["--app", "does-not-exist"]) # Invoking the run command raises an error with pytest.raises(BriefcaseCommandError): @@ -172,7 +172,7 @@ def test_create_app_before_start(run_command, first_app_config): } # Configure no command line options - options = run_command.parse_options([]) + options, _ = run_command.parse_options([]) # Run the run command run_command(**options) @@ -220,7 +220,7 @@ def test_build_app_before_start(run_command, first_app_unbuilt): } # Configure no command line options - options = run_command.parse_options([]) + options, _ = run_command.parse_options([]) # Run the run command run_command(**options) @@ -267,7 +267,7 @@ def test_update_app(run_command, first_app): } # Configure an update option - options = run_command.parse_options(["-u"]) + options, _ = run_command.parse_options(["-u"]) # Run the run command run_command(**options) @@ -314,7 +314,7 @@ def test_update_app_requirements(run_command, first_app): } # Configure an update option - options = run_command.parse_options(["-r"]) + options, _ = run_command.parse_options(["-r"]) # Run the run command run_command(**options) @@ -361,7 +361,7 @@ def test_update_app_resources(run_command, first_app): } # Configure an update option - options = run_command.parse_options(["--update-resources"]) + options, _ = run_command.parse_options(["--update-resources"]) # Run the run command run_command(**options) @@ -408,7 +408,7 @@ def test_update_unbuilt_app(run_command, first_app_unbuilt): } # Configure an update option - options = run_command.parse_options(["-u"]) + options, _ = run_command.parse_options(["-u"]) # Run the run command run_command(**options) @@ -456,7 +456,7 @@ def test_update_non_existent(run_command, first_app_config): } # Configure an update option - options = run_command.parse_options(["-u"]) + options, _ = run_command.parse_options(["-u"]) # Run the run command run_command(**options) @@ -504,7 +504,7 @@ def test_test_mode_existing_app(run_command, first_app): } # Configure the test option - options = run_command.parse_options(["--test"]) + options, _ = run_command.parse_options(["--test"]) # Run the run command run_command(**options) @@ -551,7 +551,7 @@ def test_test_mode_existing_app_with_passthrough(run_command, first_app): } # Configure the test option - options = run_command.parse_options(["--test", "--", "foo", "--bar"]) + options, _ = run_command.parse_options(["--test", "--", "foo", "--bar"]) # Run the run command run_command(**options) @@ -602,7 +602,7 @@ def test_test_mode_existing_app_no_update(run_command, first_app): } # Configure the test option - options = run_command.parse_options(["--test", "--no-update"]) + options, _ = run_command.parse_options(["--test", "--no-update"]) # Run the run command run_command(**options) @@ -637,7 +637,7 @@ def test_test_mode_existing_app_update_requirements(run_command, first_app): } # Configure the test option - options = run_command.parse_options(["--test", "--update-requirements"]) + options, _ = run_command.parse_options(["--test", "--update-requirements"]) # Run the run command run_command(**options) @@ -684,7 +684,7 @@ def test_test_mode_existing_app_update_resources(run_command, first_app): } # Configure the test option - options = run_command.parse_options(["--test", "--update-resources"]) + options, _ = run_command.parse_options(["--test", "--update-resources"]) # Run the run command run_command(**options) @@ -731,7 +731,7 @@ def test_test_mode_update_existing_app(run_command, first_app): } # Configure the test option - options = run_command.parse_options(["-u", "--test"]) + options, _ = run_command.parse_options(["-u", "--test"]) # Run the run command run_command(**options) @@ -778,7 +778,7 @@ def test_test_mode_non_existent(run_command, first_app_config): } # Configure a test option - options = run_command.parse_options(["--test"]) + options, _ = run_command.parse_options(["--test"]) # Run the run command run_command(**options) diff --git a/tests/commands/update/test_call.py b/tests/commands/update/test_call.py index cd48396e5..2ca085fc5 100644 --- a/tests/commands/update/test_call.py +++ b/tests/commands/update/test_call.py @@ -22,7 +22,7 @@ def monkeypatch_verify_git(*a, **kw): def test_update(update_command, first_app, second_app): """The update command can be called.""" # Configure no command line options - options = update_command.parse_options([]) + options, _ = update_command.parse_options([]) update_command(**options) @@ -51,7 +51,7 @@ def test_update(update_command, first_app, second_app): def test_update_single(update_command, first_app, second_app): """The update command can be called to update a single app from the config.""" # Configure no command line options - options = update_command.parse_options([]) + options, _ = update_command.parse_options([]) update_command(app=update_command.apps["first"], **options) @@ -74,7 +74,7 @@ def test_update_single(update_command, first_app, second_app): def test_update_with_requirements(update_command, first_app, second_app): """The update command can be called, requesting a requirements update.""" # Configure a requirements update - options = update_command.parse_options(["-r"]) + options, _ = update_command.parse_options(["-r"]) update_command(**options) @@ -105,7 +105,7 @@ def test_update_with_requirements(update_command, first_app, second_app): def test_update_with_resources(update_command, first_app, second_app): """The update command can be called, requesting a resources update.""" # Configure no command line options - options = update_command.parse_options(["--update-resources"]) + options, _ = update_command.parse_options(["--update-resources"]) update_command(**options) @@ -136,7 +136,7 @@ def test_update_with_resources(update_command, first_app, second_app): def test_update_with_support(update_command, first_app, second_app): """The update command can be called, requesting an app support update.""" # Configure no command line options - options = update_command.parse_options(["--update-support"]) + options, _ = update_command.parse_options(["--update-support"]) update_command(**options) diff --git a/tests/platforms/android/gradle/test_run.py b/tests/platforms/android/gradle/test_run.py index 7d6e78517..3db1db2ed 100644 --- a/tests/platforms/android/gradle/test_run.py +++ b/tests/platforms/android/gradle/test_run.py @@ -80,7 +80,7 @@ def test_binary_path(run_command, first_app_config, tmp_path): def test_device_option(run_command): """The -d option can be parsed.""" - options = run_command.parse_options(["-d", "myphone"]) + options, overrides = run_command.parse_options(["-d", "myphone"]) assert options == { "device_or_avd": "myphone", @@ -94,13 +94,13 @@ def test_device_option(run_command): "passthrough": [], "extra_emulator_args": None, "shutdown_on_exit": False, - "config_overrides": None, } + assert overrides == {} def test_extra_emulator_args_option(run_command): """The -d option can be parsed.""" - options = run_command.parse_options( + options, overrides = run_command.parse_options( ["--Xemulator=-no-window", "--Xemulator=-no-audio"] ) @@ -116,13 +116,13 @@ def test_extra_emulator_args_option(run_command): "passthrough": [], "extra_emulator_args": ["-no-window", "-no-audio"], "shutdown_on_exit": False, - "config_overrides": None, } + assert overrides == {} def test_shutdown_on_exit_option(run_command): """The -d option can be parsed.""" - options = run_command.parse_options(["--shutdown-on-exit"]) + options, overrides = run_command.parse_options(["--shutdown-on-exit"]) assert options == { "device_or_avd": None, @@ -136,8 +136,8 @@ def test_shutdown_on_exit_option(run_command): "passthrough": [], "extra_emulator_args": None, "shutdown_on_exit": True, - "config_overrides": None, } + assert overrides == {} def test_unsupported_template_version(run_command, first_app_generated): diff --git a/tests/platforms/iOS/xcode/test_run.py b/tests/platforms/iOS/xcode/test_run.py index 316ef3473..bb7896760 100644 --- a/tests/platforms/iOS/xcode/test_run.py +++ b/tests/platforms/iOS/xcode/test_run.py @@ -39,7 +39,7 @@ def mock_stream_app_logs(app, stop_func, **kwargs): def test_device_option(run_command): """The -d option can be parsed.""" - options = run_command.parse_options(["-d", "myphone"]) + options, overrides = run_command.parse_options(["-d", "myphone"]) assert options == { "udid": "myphone", @@ -51,8 +51,8 @@ def test_device_option(run_command): "test_mode": False, "passthrough": [], "appname": None, - "config_overrides": None, } + assert overrides == {} def test_run_multiple_devices_input_disabled(run_command, first_app_config): diff --git a/tests/platforms/linux/appimage/test_create.py b/tests/platforms/linux/appimage/test_create.py index a544a4633..0ab0bd169 100644 --- a/tests/platforms/linux/appimage/test_create.py +++ b/tests/platforms/linux/appimage/test_create.py @@ -21,22 +21,20 @@ def create_command(first_app_config, tmp_path): def test_default_options(create_command): """The default options are as expected.""" - options = create_command.parse_options([]) + options, overrides = create_command.parse_options([]) - assert options == { - "config_overrides": None, - } + assert options == {} + assert overrides == {} assert create_command.use_docker def test_options(create_command): """The extra options can be parsed.""" - options = create_command.parse_options(["--no-docker"]) + options, overrides = create_command.parse_options(["--no-docker"]) - assert options == { - "config_overrides": None, - } + assert options == {} + assert overrides == {} assert not create_command.use_docker diff --git a/tests/platforms/linux/system/test_create.py b/tests/platforms/linux/system/test_create.py index 87cf6a8e1..12a212a7d 100644 --- a/tests/platforms/linux/system/test_create.py +++ b/tests/platforms/linux/system/test_create.py @@ -7,22 +7,22 @@ def test_default_options(create_command): """The default options are as expected.""" - options = create_command.parse_options([]) + options, overrides = create_command.parse_options([]) - assert options == { - "config_overrides": None, - } + assert options == {} + assert overrides == {} assert create_command.target_image is None def test_options(create_command): """The extra options can be parsed.""" - options = create_command.parse_options(["--target", "somevendor:surprising"]) + options, overrides = create_command.parse_options( + ["--target", "somevendor:surprising"] + ) - assert options == { - "config_overrides": None, - } + assert options == {} + assert overrides == {} assert create_command.target_image == "somevendor:surprising" diff --git a/tests/platforms/macOS/app/test_package.py b/tests/platforms/macOS/app/test_package.py index 31db0fa1e..fbc158ed7 100644 --- a/tests/platforms/macOS/app/test_package.py +++ b/tests/platforms/macOS/app/test_package.py @@ -38,7 +38,7 @@ def test_package_formats(package_command): def test_device_option(package_command): """The -d option can be parsed.""" - options = package_command.parse_options(["--no-notarize"]) + options, overrides = package_command.parse_options(["--no-notarize"]) assert options == { "adhoc_sign": False, @@ -46,8 +46,8 @@ def test_device_option(package_command): "notarize_app": False, "packaging_format": "dmg", "update": False, - "config_overrides": None, } + assert overrides == {} def test_package_app(package_command, first_app_with_binaries, tmp_path, capsys): diff --git a/tests/platforms/web/static/test_run.py b/tests/platforms/web/static/test_run.py index cbf695bef..dea04baa5 100644 --- a/tests/platforms/web/static/test_run.py +++ b/tests/platforms/web/static/test_run.py @@ -28,7 +28,7 @@ def run_command(tmp_path): def test_default_options(run_command): """The default options are as expected.""" - options = run_command.parse_options([]) + options, overrides = run_command.parse_options([]) assert options == { "appname": None, @@ -42,13 +42,13 @@ def test_default_options(run_command): "host": "localhost", "port": 8080, "open_browser": True, - "config_overrides": None, } + assert overrides == {} def test_options(run_command): """The extra options can be parsed.""" - options = run_command.parse_options( + options, overrides = run_command.parse_options( ["--host", "myhost", "--port", "1234", "--no-browser"] ) @@ -64,8 +64,8 @@ def test_options(run_command): "host": "myhost", "port": 1234, "open_browser": False, - "config_overrides": None, } + assert overrides == {} def test_run(monkeypatch, run_command, first_app_built): diff --git a/tests/platforms/windows/app/test_package.py b/tests/platforms/windows/app/test_package.py index a8f126ef7..28d30d9a6 100644 --- a/tests/platforms/windows/app/test_package.py +++ b/tests/platforms/windows/app/test_package.py @@ -187,13 +187,13 @@ def test_parse_options(package_command, cli_args, signing_options, is_sdk_needed adhoc_sign=False, packaging_format="msi", update=False, - config_overrides=None, ) expected_options = {**default_options, **signing_options} - options = package_command.parse_options(extra=cli_args) + options, overrides = package_command.parse_options(extra=cli_args) assert options == expected_options + assert overrides == {} assert package_command._windows_sdk_needed is is_sdk_needed diff --git a/tests/test_cmdline.py b/tests/test_cmdline.py index 7f3986cec..db61cbcb9 100644 --- a/tests/test_cmdline.py +++ b/tests/test_cmdline.py @@ -35,8 +35,8 @@ def do_cmdline_parse(args: list, logger: Log, console: Console): """Simulate process to parse command line.""" Command, extra_cmdline = cmdline.parse_cmdline(args) cmd = Command(logger=logger, console=console) - options = cmd.parse_options(extra=extra_cmdline) - return cmd, options + options, overrides = cmd.parse_options(extra=extra_cmdline) + return cmd, options, overrides def test_empty(): @@ -93,9 +93,33 @@ def test_unknown_command(): ) -def test_new_command(logger, console): +@pytest.mark.parametrize( + "cmdline, expected_options, expected_overrides", + [ + ( + "new", + { + "template": None, + "template_branch": None, + }, + {}, + ), + ( + "new --template=path/to/template --template-branch=experiment -C version=\\'1.2.3\\' -C other=42", + { + "template": "path/to/template", + "template_branch": "experiment", + }, + { + "version": "1.2.3", + "other": 42, + }, + ), + ], +) +def test_new_command(logger, console, cmdline, expected_options, expected_overrides): """``briefcase new`` returns the New command.""" - cmd, options = do_cmdline_parse("new".split(), logger, console) + cmd, options, overrides = do_cmdline_parse(shlex.split(cmdline), logger, console) assert isinstance(cmd, NewCommand) assert cmd.platform == "all" @@ -104,51 +128,68 @@ def test_new_command(logger, console): assert cmd.logger.verbosity == LogLevel.INFO assert cmd.logger is logger assert cmd.input is console - assert options == { - "config_overrides": None, - "template": None, - "template_branch": None, - } + assert options == expected_options + assert overrides == expected_overrides # Common tests for dev and run commands. def dev_run_parameters(command): return [ - (f"{command} {args}", expected) - for args, expected in [ - ("", {}), - ("-r", {"update_requirements": True}), - ("--update-requirements", {"update_requirements": True}), - ("--test", {"test_mode": True}), - ("--test -r", {"test_mode": True, "update_requirements": True}), - ("--", {}), - ("-- ''", {"passthrough": [""]}), - ("-- --test", {"passthrough": ["--test"]}), - ("--test -- --test", {"test_mode": True, "passthrough": ["--test"]}), - ("--test -- -r", {"test_mode": True, "passthrough": ["-r"]}), - ("-r -- --test", {"update_requirements": True, "passthrough": ["--test"]}), - ("-- -y --no maybe", {"passthrough": ["-y", "--no", "maybe"]}), + (f"{command} {args}", expected, overrides) + for args, expected, overrides in [ + ("", {}, {}), + ("-r", {"update_requirements": True}, {}), + ( + "-r -C version=\\'1.2.3\\' -C other=42", + {"update_requirements": True}, + { + "version": "1.2.3", + "other": 42, + }, + ), + ("--update-requirements", {"update_requirements": True}, {}), + ("--test", {"test_mode": True}, {}), + ("--test -r", {"test_mode": True, "update_requirements": True}, {}), + ("--", {}, {}), + ("-- ''", {"passthrough": [""]}, {}), + ("-- --test", {"passthrough": ["--test"]}, {}), + ("--test -- --test", {"test_mode": True, "passthrough": ["--test"]}, {}), + ("--test -- -r", {"test_mode": True, "passthrough": ["-r"]}, {}), + ( + "-r -- --test", + {"update_requirements": True, "passthrough": ["--test"]}, + {}, + ), + ("-- -y --no maybe", {"passthrough": ["-y", "--no", "maybe"]}, {}), ( "--test -- -y --no maybe", {"test_mode": True, "passthrough": ["-y", "--no", "maybe"]}, + {}, ), ] ] @pytest.mark.parametrize( - "cmdline, expected_options", + "cmdline, expected_options, expected_overrides", dev_run_parameters("dev") + [ - ("dev --no-run", {"run_app": False}), + ("dev --no-run", {"run_app": False}, {}), ], ) -def test_dev_command(monkeypatch, logger, console, cmdline, expected_options): +def test_dev_command( + monkeypatch, + logger, + console, + cmdline, + expected_options, + expected_overrides, +): """``briefcase dev`` returns the Dev command.""" # Pretend we're on macOS, regardless of where the tests run. monkeypatch.setattr(sys, "platform", "darwin") - cmd, options = do_cmdline_parse(shlex.split(cmdline), logger, console) + cmd, options, overrides = do_cmdline_parse(shlex.split(cmdline), logger, console) assert isinstance(cmd, DevCommand) assert cmd.platform == "macOS" @@ -163,28 +204,35 @@ def test_dev_command(monkeypatch, logger, console, cmdline, expected_options): "run_app": True, "test_mode": False, "passthrough": [], - "config_overrides": None, **expected_options, } + assert overrides == expected_overrides @pytest.mark.parametrize( - "cmdline, expected_options", + "cmdline, expected_options, expected_overrides", dev_run_parameters("run") + [ - ("run -u", {"update": True}), - ("run --update", {"update": True}), - ("run --update-resources", {"update_resources": True}), - ("run --update-support", {"update_support": True}), - ("run --no-update", {"no_update": True}), + ("run -u", {"update": True}, {}), + ("run --update", {"update": True}, {}), + ("run --update-resources", {"update_resources": True}, {}), + ("run --update-support", {"update_support": True}, {}), + ("run --no-update", {"no_update": True}, {}), ], ) -def test_run_command(monkeypatch, logger, console, cmdline, expected_options): +def test_run_command( + monkeypatch, + logger, + console, + cmdline, + expected_options, + expected_overrides, +): """``briefcase run`` returns the Run command for the correct platform.""" # Pretend we're on macOS, regardless of where the tests run. monkeypatch.setattr(sys, "platform", "darwin") - cmd, options = do_cmdline_parse(shlex.split(cmdline), logger, console) + cmd, options, overrides = do_cmdline_parse(shlex.split(cmdline), logger, console) assert isinstance(cmd, macOSAppRunCommand) assert cmd.platform == "macOS" @@ -202,17 +250,48 @@ def test_run_command(monkeypatch, logger, console, cmdline, expected_options): "no_update": False, "test_mode": False, "passthrough": [], - "config_overrides": None, **expected_options, } + assert overrides == expected_overrides -def test_upgrade_command(monkeypatch, logger, console): +@pytest.mark.parametrize( + "cmdline,expected_options,expected_overrides", + [ + ( + "upgrade", + { + "list_tools": False, + "tool_list": [], + }, + {}, + ), + ( + "upgrade -C version='1.2.3' -C other=42", + { + "list_tools": False, + "tool_list": [], + }, + { + "version": "1.2.3", + "other": 42, + }, + ), + ], +) +def test_upgrade_command( + monkeypatch, + logger, + console, + cmdline, + expected_options, + expected_overrides, +): """``briefcase upgrade`` returns the upgrade command.""" # Pretend we're on macOS, regardless of where the tests run. monkeypatch.setattr(sys, "platform", "darwin") - cmd, options = do_cmdline_parse("upgrade".split(), logger, console) + cmd, options, overrides = do_cmdline_parse(cmdline.split(), logger, console) assert isinstance(cmd, UpgradeCommand) assert cmd.platform == "macOS" @@ -221,11 +300,8 @@ def test_upgrade_command(monkeypatch, logger, console): assert cmd.logger.verbosity == LogLevel.INFO assert cmd.logger is logger assert cmd.input is console - assert options == { - "list_tools": False, - "tool_list": [], - "config_overrides": None, - } + assert options == expected_options + assert overrides == expected_overrides def test_bare_command(monkeypatch, logger, console): @@ -233,7 +309,7 @@ def test_bare_command(monkeypatch, logger, console): # Pretend we're on macOS, regardless of where the tests run. monkeypatch.setattr(sys, "platform", "darwin") - cmd, options = do_cmdline_parse("create".split(), logger, console) + cmd, options, overrides = do_cmdline_parse("create".split(), logger, console) assert isinstance(cmd, macOSAppCreateCommand) assert cmd.platform == "macOS" @@ -242,14 +318,15 @@ def test_bare_command(monkeypatch, logger, console): assert cmd.logger.verbosity == LogLevel.INFO assert cmd.logger is logger assert cmd.input is console - assert options == {"config_overrides": None} + assert options == {} + assert overrides == {} @pytest.mark.skipif(sys.platform != "linux", reason="requires Linux") def test_linux_default(logger, console): """``briefcase create`` returns the linux create system command on Linux.""" - cmd, options = do_cmdline_parse("create".split(), logger, console) + cmd, options, overrides = do_cmdline_parse("create".split(), logger, console) assert isinstance(cmd, LinuxSystemCreateCommand) assert cmd.platform == "linux" @@ -258,14 +335,14 @@ def test_linux_default(logger, console): assert cmd.logger.verbosity == LogLevel.INFO assert cmd.logger is logger assert cmd.input is console - assert options == {"config_overrides": None} + assert options == {} @pytest.mark.skipif(sys.platform != "darwin", reason="requires macOS") def test_macOS_default(logger, console): """``briefcase create`` returns the macOS create command on Linux.""" - cmd, options = do_cmdline_parse("create".split(), logger, console) + cmd, options, overrides = do_cmdline_parse("create".split(), logger, console) assert isinstance(cmd, macOSAppCreateCommand) assert cmd.platform == "macOS" @@ -274,14 +351,15 @@ def test_macOS_default(logger, console): assert cmd.logger.verbosity == LogLevel.INFO assert cmd.logger is logger assert cmd.input is console - assert options == {"config_overrides": None} + assert options == {} + assert overrides == {} @pytest.mark.skipif(sys.platform != "win32", reason="requires Windows") def test_windows_default(logger, console): """``briefcase create`` returns the Windows create app command on Windows.""" - cmd, options = do_cmdline_parse("create".split(), logger, console) + cmd, options, overrides = do_cmdline_parse("create".split(), logger, console) assert isinstance(cmd, WindowsAppCreateCommand) assert cmd.platform == "windows" @@ -290,7 +368,8 @@ def test_windows_default(logger, console): assert cmd.logger.verbosity == LogLevel.INFO assert cmd.logger is logger assert cmd.input is console - assert options == {"config_overrides": None} + assert options == {} + assert overrides == {} def test_bare_command_help(monkeypatch, capsys, logger, console): @@ -345,7 +424,7 @@ def test_command_explicit_platform(monkeypatch, logger, console): # Pretend we're on macOS, regardless of where the tests run. monkeypatch.setattr(sys, "platform", "darwin") - cmd, options = do_cmdline_parse("create linux".split(), logger, console) + cmd, options, overrides = do_cmdline_parse("create linux".split(), logger, console) assert isinstance(cmd, LinuxSystemCreateCommand) assert cmd.platform == "linux" @@ -354,7 +433,8 @@ def test_command_explicit_platform(monkeypatch, logger, console): assert cmd.logger.verbosity == LogLevel.INFO assert cmd.logger is logger assert cmd.input is console - assert options == {"config_overrides": None} + assert options == {} + assert overrides == {} def test_command_explicit_platform_case_handling(monkeypatch, logger, console): @@ -363,7 +443,7 @@ def test_command_explicit_platform_case_handling(monkeypatch, logger, console): monkeypatch.setattr(sys, "platform", "darwin") # This is all lower case; the command normalizes to macOS - cmd, options = do_cmdline_parse("create macOS".split(), logger, console) + cmd, options, overrides = do_cmdline_parse("create macOS".split(), logger, console) assert isinstance(cmd, macOSAppCreateCommand) assert cmd.platform == "macOS" @@ -372,7 +452,8 @@ def test_command_explicit_platform_case_handling(monkeypatch, logger, console): assert cmd.logger.verbosity == LogLevel.INFO assert cmd.logger is logger assert cmd.input is console - assert options == {"config_overrides": None} + assert options == {} + assert overrides == {} def test_command_explicit_platform_help(monkeypatch, capsys, logger, console): @@ -400,7 +481,9 @@ def test_command_explicit_format(monkeypatch, logger, console): # Pretend we're on macOS, regardless of where the tests run. monkeypatch.setattr(sys, "platform", "darwin") - cmd, options = do_cmdline_parse("create macOS app".split(), logger, console) + cmd, options, overrides = do_cmdline_parse( + "create macOS app".split(), logger, console + ) assert isinstance(cmd, macOSAppCreateCommand) assert cmd.platform == "macOS" @@ -409,7 +492,8 @@ def test_command_explicit_format(monkeypatch, logger, console): assert cmd.logger.verbosity == LogLevel.INFO assert cmd.logger is logger assert cmd.input is console - assert options == {"config_overrides": None} + assert options == {} + assert overrides == {} def test_command_unknown_format(monkeypatch, logger, console): @@ -467,7 +551,9 @@ def test_command_disable_input(monkeypatch, logger, console): # Pretend we're on macOS, regardless of where the tests run. monkeypatch.setattr(sys, "platform", "darwin") - cmd, options = do_cmdline_parse("create --no-input".split(), logger, console) + cmd, options, overrides = do_cmdline_parse( + "create --no-input".split(), logger, console + ) assert isinstance(cmd, macOSAppCreateCommand) assert cmd.platform == "macOS" @@ -476,7 +562,8 @@ def test_command_disable_input(monkeypatch, logger, console): assert cmd.logger.verbosity == LogLevel.INFO assert cmd.logger is logger assert cmd.input is console - assert options == {"config_overrides": None} + assert options == {} + assert overrides == {} def test_command_options(monkeypatch, capsys, logger, console): @@ -486,14 +573,42 @@ def test_command_options(monkeypatch, capsys, logger, console): # Invoke a command that is known to have its own custom arguments # (In this case, the channel argument for publication) - cmd, options = do_cmdline_parse("publish macos app -c s3".split(), logger, console) + cmd, options, overrides = do_cmdline_parse( + "publish macos app -c s3".split(), logger, console + ) + + assert isinstance(cmd, macOSAppPublishCommand) + assert cmd.input.enabled + assert cmd.logger.verbosity == LogLevel.INFO + assert cmd.logger is logger + assert cmd.input is console + assert options == {"channel": "s3"} + assert overrides == {} + + +def test_command_overrides(monkeypatch, capsys, logger, console): + """Configuration overrides can be specified.""" + # Pretend we're on macOS, regardless of where the tests run. + monkeypatch.setattr(sys, "platform", "darwin") + + # Invoke a command that is known to have its own custom arguments + # (In this case, the channel argument for publication) + cmd, options, overrides = do_cmdline_parse( + "publish macos app -C version='1.2.3' -C extra=42".split(), + logger, + console, + ) assert isinstance(cmd, macOSAppPublishCommand) assert cmd.input.enabled assert cmd.logger.verbosity == LogLevel.INFO assert cmd.logger is logger assert cmd.input is console - assert options == {"config_overrides": None, "channel": "s3"} + assert options == {"channel": "s3"} + assert overrides == { + "version": "1.2.3", + "extra": 42, + } def test_unknown_command_options(monkeypatch, capsys, logger, console): From 7d102de196ec94ad2b0439304fee7b8ff867f51d Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Fri, 17 Nov 2023 15:23:38 +0800 Subject: [PATCH 9/9] Ensure that app_name isn't overridden, as it's an identifier. --- docs/reference/commands/index.rst | 3 +++ src/briefcase/commands/base.py | 7 ++++++- tests/commands/base/test_parse_config_overrides.py | 4 +++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/docs/reference/commands/index.rst b/docs/reference/commands/index.rst index 174ea8203..c3d8573a5 100644 --- a/docs/reference/commands/index.rst +++ b/docs/reference/commands/index.rst @@ -42,6 +42,9 @@ template=...``, but the value must be quoted:: briefcase create -C template=\"https://example.com/template\" +The only app key that *cannot* be overridden with ``-C`` is ``app_name``, as it is used +to identify apps. + ``-h`` / ``--help`` ------------------- diff --git a/src/briefcase/commands/base.py b/src/briefcase/commands/base.py index 7cb9a42cb..c776f957b 100644 --- a/src/briefcase/commands/base.py +++ b/src/briefcase/commands/base.py @@ -116,11 +116,16 @@ def parse_config_overrides(config_overrides: list[str] | None) -> dict[str, Any] if config_overrides: for override in config_overrides: try: + # Do initial checks of the key being overridden. + # These catch cases that would be valid TOML, but would result + # in invalid app configurations. key, _ = override.split("=", 1) if "." in key: raise BriefcaseConfigError( - "Can't override multi-level configuration keys" + "Can't override multi-level configuration keys." ) + elif key == "app_name": + raise BriefcaseConfigError("The app name cannot be overridden.") # Now actually parse the value overrides.update(tomllib.loads(override)) diff --git a/tests/commands/base/test_parse_config_overrides.py b/tests/commands/base/test_parse_config_overrides.py index 04358357c..6c7cb1f3b 100644 --- a/tests/commands/base/test_parse_config_overrides.py +++ b/tests/commands/base/test_parse_config_overrides.py @@ -69,7 +69,9 @@ def test_valid_overrides(overrides, values): # Space in the key. (["spacy key=42"], r"Unable to parse configuration override "), # Multi-level key. This is legal TOML, but difficult to merge. - (["multi.level.key=42"], r"Can't override multi-level configuration keys"), + (["multi.level.key=42"], r"Can't override multi-level configuration keys\."), + # Key that can't be overridden + (["app_name='foobar'"], r"The app name cannot be overridden\."), ], ) def test_invalid_overrides(overrides, message):