diff --git a/src/briefcase/integrations/android_sdk.py b/src/briefcase/integrations/android_sdk.py index 500754cda..0972ef846 100644 --- a/src/briefcase/integrations/android_sdk.py +++ b/src/briefcase/integrations/android_sdk.py @@ -792,19 +792,20 @@ def select_target_device(self, device_or_avd): avd = choice[1:] else: # Either a running emulator, or a physical device. Regardless, - # we need to check if the device is developer enabled - try: - details = running_devices[choice] - if not details["authorized"]: - # An unauthorized physical device - raise AndroidDeviceNotAuthorized(choice) - - # Return the device ID and name. - device = choice - name = device_choices[choice] - avd = details.get("avd") - except KeyError as e: - raise InvalidDeviceError("device ID", choice) from e + # we need to check if the device is developer enabled. + # Functionally, we know the the device *must* be in the list of + # choices; which means it's also in the list of running devices + # and the list of device choices, so any KeyError on those lookups + # indicates a deeper problem. + details = running_devices[choice] + if not details["authorized"]: + # An unauthorized physical device + raise AndroidDeviceNotAuthorized(choice) + + # Return the device ID and name. + device = choice + name = device_choices[choice] + avd = details.get("avd") if avd: self.tools.logger.info( @@ -983,6 +984,7 @@ def start_emulator(self, avd): """ if avd not in set(self.emulators()): raise InvalidDeviceError("emulator AVD", avd) + emulator_popen = self.tools.subprocess.Popen( [ os.fsdecode(self.emulator_path), @@ -1041,8 +1043,9 @@ def start_emulator(self, avd): adb = None known_devices.add(device) - # Try again in 2 seconds... - self.sleep(2) + # If we haven't found a device, try again in 2 seconds... + if adb is None: + self.sleep(2) # Phase 2: Wait for the boot process to complete if not adb.has_booted(): diff --git a/src/briefcase/integrations/git.py b/src/briefcase/integrations/git.py index fbe2325d1..3403422e7 100644 --- a/src/briefcase/integrations/git.py +++ b/src/briefcase/integrations/git.py @@ -25,7 +25,7 @@ def verify_git_is_installed(tools): # Check whether the git executable could be imported. try: import git - except ImportError as e: + except ImportError as e: # pragma: no cover # macOS provides git as part of the Xcode command line tools, # and also hijacks /usr/bin/git with a trigger that prompts the # installation of those tools. Customize the message to account diff --git a/src/briefcase/integrations/xcode.py b/src/briefcase/integrations/xcode.py index b4a3b2cd8..e5955671b 100644 --- a/src/briefcase/integrations/xcode.py +++ b/src/briefcase/integrations/xcode.py @@ -183,26 +183,26 @@ def ensure_xcode_is_installed( line for line in output.split("\n") if line.startswith("Xcode ") ] if version_lines: - try: - # Split the content after the first space - # and split that content on the dots. - # Append 0's to fill any gaps caused by - # version numbers that don't have a minor version. - version = tuple( - int(v) for v in version_lines[0].split(" ")[1].split(".") - ) + (0, 0) - - if version < min_version: - min_version = ".".join(str(v) for v in min_version) - version = ".".join(str(v) for v in version) - raise BriefcaseCommandError( - f"Xcode {min_version} is required; {version} is installed. Please update Xcode." - ) - else: - # Version number is acceptable - return - except IndexError: - pass + # Split the content after the first space + # and split that content on the dots. + # Append 0's to fill any gaps caused by + # version numbers that don't have a minor version. + # At this point, version lines *must* have at least one element, + # and each line *must* have a string with at least one space, + # so if either array lookup fails, something weird is happening. + version = tuple( + int(v) for v in version_lines[0].split(" ")[1].split(".") + ) + (0, 0) + + if version < min_version: + min_version = ".".join(str(v) for v in min_version) + version = ".".join(str(v) for v in version) + raise BriefcaseCommandError( + f"Xcode {min_version} is required; {version} is installed. Please update Xcode." + ) + else: + # Version number is acceptable + return tools.logger.warning( """ diff --git a/tests/console/test_Log.py b/tests/console/test_Log.py index 61aa047d2..ac0d1dfd1 100644 --- a/tests/console/test_Log.py +++ b/tests/console/test_Log.py @@ -58,6 +58,8 @@ def test_save_log_to_file_no_exception(tmp_path, now): logger.save_log = True logger.debug("this is debug output") logger.info("this is info output") + logger.info("this is [bold]info output with markup[/bold]") + logger.info("this is [bold]info output with escaped markup[/bold]", markup=True) logger.warning("this is warning output") logger.error("this is error output") logger.print("this is print output") @@ -75,6 +77,8 @@ def test_save_log_to_file_no_exception(tmp_path, now): assert log_contents.startswith("Date/Time: 2022-06-25 16:12:29") assert f"{Log.DEBUG_PREFACE}this is debug output" in log_contents assert "this is info output" in log_contents + assert "this is [bold]info output with markup[/bold]" in log_contents + assert "this is info output with escaped markup" in log_contents assert "this is warning output" in log_contents assert "this is error output" in log_contents assert "this is print output" in log_contents diff --git a/tests/integrations/android_sdk/ADB/test_start_app.py b/tests/integrations/android_sdk/ADB/test_start_app.py index 119babee0..e59e3b289 100644 --- a/tests/integrations/android_sdk/ADB/test_start_app.py +++ b/tests/integrations/android_sdk/ADB/test_start_app.py @@ -1,3 +1,4 @@ +from subprocess import CalledProcessError from unittest.mock import MagicMock import pytest @@ -62,3 +63,15 @@ def test_invalid_device(mock_tools): with pytest.raises(InvalidDeviceError): adb.start_app("com.example.sample.package", "com.example.sample.activity") + + +def test_unable_to_start(mock_tools): + """If the adb calls for other reasons, the error is caught.""" + adb = ADB(mock_tools, "exampleDevice") + adb.run = MagicMock(side_effect=CalledProcessError(cmd=["adb"], returncode=1)) + + with pytest.raises( + BriefcaseCommandError, + match=r"Unable to start com.example.sample.package/com.example.sample.activity on exampleDevice", + ): + adb.start_app("com.example.sample.package", "com.example.sample.activity") diff --git a/tests/integrations/android_sdk/AndroidSDK/test_properties.py b/tests/integrations/android_sdk/AndroidSDK/test_properties.py index 7408aef8a..4b9b0c307 100644 --- a/tests/integrations/android_sdk/AndroidSDK/test_properties.py +++ b/tests/integrations/android_sdk/AndroidSDK/test_properties.py @@ -140,3 +140,11 @@ def test_bad_emulator_abi(mock_tools, android_sdk, host_os, host_arch): match=rf"The Android emulator does not currently support {host_os} {host_arch} hardware.", ): android_sdk.emulator_abi + + +def test_adb_for_device(mock_tools, android_sdk): + "An ADB instance can be bound to a device." + adb = android_sdk.adb("some-device") + + assert adb.tools == mock_tools + assert adb.device == "some-device" diff --git a/tests/integrations/android_sdk/AndroidSDK/test_start_emulator.py b/tests/integrations/android_sdk/AndroidSDK/test_start_emulator.py index 51d842776..bf508edfd 100644 --- a/tests/integrations/android_sdk/AndroidSDK/test_start_emulator.py +++ b/tests/integrations/android_sdk/AndroidSDK/test_start_emulator.py @@ -133,8 +133,86 @@ def test_start_emulator(mock_tools, android_sdk): ] ) - # Took a total of 5 naps. - assert android_sdk.sleep.call_count == 4 + # Took a total of 3 naps. + assert android_sdk.sleep.call_count == 3 + + +def test_start_emulator_fast_start(mock_tools, android_sdk): + """If the emulator starts quickly, that's OK.""" + # If the emulator starts *really* fast, there will only be 1 call to devices. + devices = { + "041234567892009a": { + "name": "Unknown device (not authorized for development)", + "authorized": False, + }, + "KABCDABCDA1513": { + "name": "Kogan Agora 9", + "authorized": True, + }, + "emulator-5554": { + "name": "generic_x86", + "authorized": True, + }, + } + + android_sdk.devices = MagicMock( + side_effect=[ + devices, + ] + ) + + # There will be 3 calls on adb.run (2 calls to avd_name, then + # 1 call to getprop) + android_sdk.mock_run.side_effect = [ + # emu avd_name + subprocess.CalledProcessError( + returncode=1, cmd="emu avd name" + ), # phyiscal device + "idleEmulator\nOK", # running emulator + # shell getprop sys.boot_completed + "1\n", + ] + + # poll() on the process continues to return None, indicating no problem. + emu_popen = MagicMock(spec_set=subprocess.Popen) + emu_popen.poll.return_value = None + mock_tools.subprocess.Popen.return_value = emu_popen + + # Start the emulator + device, name = android_sdk.start_emulator("idleEmulator") + + # The device details are as expected + assert device == "emulator-5554" + assert name == "@idleEmulator (running emulator)" + + # The process was started. + mock_tools.subprocess.Popen.assert_called_with( + [ + os.fsdecode(android_sdk.emulator_path), + "@idleEmulator", + "-dns-server", + "8.8.8.8", + ], + env=android_sdk.env, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + bufsize=1, + start_new_session=True, + ) + + # There were 3 calls to run + android_sdk.mock_run.assert_has_calls( + [ + # 2 calls to get avd name + call("emu", "avd", "name"), + call("emu", "avd", "name"), + # 1 calls to get boot property + call("shell", "getprop", "sys.boot_completed"), + ] + ) + + # Took no naps, as everything was ready when we found it. + assert android_sdk.sleep.call_count == 0 def test_emulator_fail_to_start(mock_tools, android_sdk): @@ -332,5 +410,5 @@ def test_emulator_fail_to_boot(mock_tools, android_sdk): ] ) - # Took a total of 5 naps. - assert android_sdk.sleep.call_count == 5 + # Took a total of 4 naps. + assert android_sdk.sleep.call_count == 4 diff --git a/tests/integrations/android_sdk/AndroidSDK/test_verify.py b/tests/integrations/android_sdk/AndroidSDK/test_verify.py index c73eb5f7d..5ef5d321d 100644 --- a/tests/integrations/android_sdk/AndroidSDK/test_verify.py +++ b/tests/integrations/android_sdk/AndroidSDK/test_verify.py @@ -37,6 +37,8 @@ def mock_unpack(filename, extract_dir): (extract_dir / "cmdline-tools" / "bin").mkdir(parents=True) (extract_dir / "cmdline-tools" / "bin" / "sdkmanager").touch(mode=0o644) (extract_dir / "cmdline-tools" / "bin" / "avdmanager").touch(mode=0o644) + # Include an extra tool that is already executable. + (extract_dir / "cmdline-tools" / "bin" / "other").touch(mode=0o755) def accept_license(android_sdk_root_path): diff --git a/tests/integrations/subprocess/test_Subprocess__check_output.py b/tests/integrations/subprocess/test_Subprocess__check_output.py index 9ac649fc3..98e5d9eaa 100644 --- a/tests/integrations/subprocess/test_Subprocess__check_output.py +++ b/tests/integrations/subprocess/test_Subprocess__check_output.py @@ -223,6 +223,37 @@ def test_calledprocesserror_exception_logging(mock_sub, capsys): assert capsys.readouterr().out == expected_output +def test_calledprocesserror_exception_logging_no_cmd_output(mock_sub, capsys): + """If command errors, and there is no command output, errors are still + printed.""" + mock_sub.tools.logger.verbosity = 2 + + called_process_error = CalledProcessError( + returncode=-1, + cmd="hello world", + output=None, + stderr="error line 1\nerror line 2", + ) + mock_sub._subprocess.check_output.side_effect = called_process_error + + with pytest.raises(CalledProcessError): + mock_sub.check_output(["hello", "world"]) + + expected_output = ( + "\n" + ">>> Running Command:\n" + ">>> hello world\n" + ">>> Working Directory:\n" + f">>> {Path.cwd()}\n" + ">>> Command Error Output (stderr):\n" + ">>> error line 1\n" + ">>> error line 2\n" + ">>> Return code: -1\n" + ) + + assert capsys.readouterr().out == expected_output + + @pytest.mark.parametrize( "in_kwargs, kwargs", [ diff --git a/tests/integrations/subprocess/test_Subprocess__final_kwargs.py b/tests/integrations/subprocess/test_Subprocess__final_kwargs.py index 76f702117..bf0ee568d 100644 --- a/tests/integrations/subprocess/test_Subprocess__final_kwargs.py +++ b/tests/integrations/subprocess/test_Subprocess__final_kwargs.py @@ -17,6 +17,19 @@ def test_no_overrides(mock_sub): } +def test_explicit_no_overrides(mock_sub): + """With explicitly no overrides, there are still kwargs.""" + assert mock_sub.final_kwargs(env=None) == { + "env": { + "VAR1": "Value 1", + "PS1": "\nLine 2\n\nLine 4", + "PWD": "/home/user/", + }, + "text": True, + "encoding": CONSOLE_ENCODING, + } + + def test_env_overrides(mock_sub): """If environmental overrides are provided, they supercede the default environment.""" diff --git a/tests/integrations/subprocess/test_get_process_id_by_command.py b/tests/integrations/subprocess/test_get_process_id_by_command.py index 4d05cb5cf..782cac2df 100644 --- a/tests/integrations/subprocess/test_get_process_id_by_command.py +++ b/tests/integrations/subprocess/test_get_process_id_by_command.py @@ -11,6 +11,7 @@ info=dict(cmdline=["/bin/cmd.sh", "--input", "data"], create_time=20, pid=100) ) ] + process_list_two_procs_diff_cmd = [ Process( info=dict( @@ -23,6 +24,7 @@ ) ), ] + process_list_two_procs_same_cmd = [ Process( info=dict(cmdline=["/bin/cmd.sh", "--input", "data"], create_time=20, pid=100) @@ -114,3 +116,17 @@ def test_get_process_id_by_command_w_command( found_pid = get_process_id_by_command(command=command, logger=Log()) assert found_pid == expected_pid assert capsys.readouterr().out == expected_stdout + + +def test_get_process_id_no_logging(monkeypatch, capsys): + """If no logger is provided, warnings about ambiguous matches aren't + printed.""" + monkeypatch.setattr( + "psutil.process_iter", + lambda attrs: process_list_two_procs_same_cmd, + ) + found_pid = get_process_id_by_command( + command_list=["/bin/cmd.sh", "--input", "data"] + ) + assert found_pid == 100 + assert capsys.readouterr().out == "" diff --git a/tests/integrations/xcode/simctl/single-device-booted.json b/tests/integrations/xcode/simctl/single-device-booted.json index 26918924b..0c73fa04e 100644 --- a/tests/integrations/xcode/simctl/single-device-booted.json +++ b/tests/integrations/xcode/simctl/single-device-booted.json @@ -7,6 +7,12 @@ ], "com.apple.CoreSimulator.SimRuntime.iOS-13-2" : [ + { + "state" : "Shutdown", + "udid" : "91314875-180C-41BB-AE5C-968C8B89897D", + "isAvailable" : true, + "name" : "iPhone 14" + }, { "state" : "Booted", "isAvailable" : true, diff --git a/tests/integrations/xcode/simctl/single-device-shutdown.json b/tests/integrations/xcode/simctl/single-device-shutdown.json index 294b04643..302f94121 100644 --- a/tests/integrations/xcode/simctl/single-device-shutdown.json +++ b/tests/integrations/xcode/simctl/single-device-shutdown.json @@ -7,6 +7,12 @@ ], "com.apple.CoreSimulator.SimRuntime.iOS-13-2" : [ + { + "state" : "Shutdown", + "udid" : "91314875-180C-41BB-AE5C-968C8B89897D", + "isAvailable" : true, + "name" : "iPhone 14" + }, { "state" : "Shutdown", "isAvailable" : true, diff --git a/tests/integrations/xcode/simctl/single-device-shutting-down.json b/tests/integrations/xcode/simctl/single-device-shutting-down.json index f1d7c5bcb..8129f75cf 100644 --- a/tests/integrations/xcode/simctl/single-device-shutting-down.json +++ b/tests/integrations/xcode/simctl/single-device-shutting-down.json @@ -7,6 +7,12 @@ ], "com.apple.CoreSimulator.SimRuntime.iOS-13-2" : [ + { + "state" : "Shutdown", + "udid" : "91314875-180C-41BB-AE5C-968C8B89897D", + "isAvailable" : true, + "name" : "iPhone 14" + }, { "state" : "Shutting Down", "isAvailable" : true, diff --git a/tests/integrations/xcode/simctl/single-device-unknown.json b/tests/integrations/xcode/simctl/single-device-unknown.json index a75ce601a..4e057d2d4 100644 --- a/tests/integrations/xcode/simctl/single-device-unknown.json +++ b/tests/integrations/xcode/simctl/single-device-unknown.json @@ -7,6 +7,12 @@ ], "com.apple.CoreSimulator.SimRuntime.iOS-13-2" : [ + { + "state" : "Shutdown", + "udid" : "91314875-180C-41BB-AE5C-968C8B89897D", + "isAvailable" : true, + "name" : "iPhone 14" + }, { "state" : "Booting", "isAvailable" : true, diff --git a/tests/integrations/xcode/simctl/single-device.json b/tests/integrations/xcode/simctl/single-device.json deleted file mode 100644 index 26918924b..000000000 --- a/tests/integrations/xcode/simctl/single-device.json +++ /dev/null @@ -1,18 +0,0 @@ -{ - "devices" : { - "com.apple.CoreSimulator.SimRuntime.tvOS-13-2" : [ - - ], - "com.apple.CoreSimulator.SimRuntime.watchOS-6-1" : [ - - ], - "com.apple.CoreSimulator.SimRuntime.iOS-13-2" : [ - { - "state" : "Booted", - "isAvailable" : true, - "name" : "iPhone 11", - "udid" : "2D3503A3-6EB9-4B37-9B17-C7EFEF2FA32D" - } - ] - } -} diff --git a/tests/integrations/xcode/test_verify_command_line_tools_install.py b/tests/integrations/xcode/test_verify_command_line_tools_install.py new file mode 100644 index 000000000..c8c56de35 --- /dev/null +++ b/tests/integrations/xcode/test_verify_command_line_tools_install.py @@ -0,0 +1,29 @@ +from subprocess import CalledProcessError +from unittest import mock + +from briefcase.integrations.xcode import verify_command_line_tools_install + + +def test_verify_command_line_tools_install(mock_tools): + "Xcode CLI tools can be verified" + mock_tools.subprocess.check_output.side_effect = [ + CalledProcessError(cmd=["xcode-select", "--install"], returncode=1), + "clang 37.42", # clang --version + ] + + verify_command_line_tools_install(mock_tools) + + # The command line tools are verified + assert mock_tools.xcode_cli is not None + + +def test_reverify_command_line_tools_install(mock_tools): + "A second call to verify is a no-op" + + xcode_cli = mock.MagicMock() + mock_tools.xcode_cli = xcode_cli + + verify_command_line_tools_install(mock_tools) + + # The command line tools are verified + assert mock_tools.xcode_cli is not None diff --git a/tests/integrations/xcode/test_verify_xcode_install.py b/tests/integrations/xcode/test_verify_xcode_install.py new file mode 100644 index 000000000..9cd3d9a2a --- /dev/null +++ b/tests/integrations/xcode/test_verify_xcode_install.py @@ -0,0 +1,36 @@ +from subprocess import CalledProcessError +from unittest import mock + +from briefcase.integrations.xcode import verify_xcode_install + + +def test_verify_xcode_install(mock_tools): + "Xcode can be verified" + mock_tools.subprocess.check_output.side_effect = [ + "/Applications/Xcode/app/Contents/Developer", # xcode-select -p + "Xcode 14.0.1", # xcodebuild -version + CalledProcessError(cmd=["xcode-select", "--install"], returncode=1), + "clang 37.42", # clang --version + ] + + verify_xcode_install(mock_tools) + + # Both Xcode and the command line tools are verified + assert mock_tools.xcode is not None + assert mock_tools.xcode_cli is not None + + +def test_reverify_xcode_install(mock_tools): + "A second call to verify is a no-op" + + xcode = mock.MagicMock() + mock_tools.xcode = xcode + + xcode_cli = mock.MagicMock() + mock_tools.xcode_cli = xcode_cli + + verify_xcode_install(mock_tools) + + # Both Xcode and the command line tools are verified + assert mock_tools.xcode == xcode + assert mock_tools.xcode_cli is not None diff --git a/tests/platforms/macOS/app/test_package.py b/tests/platforms/macOS/app/test_package.py index d13ed9a12..22c37e0ad 100644 --- a/tests/platforms/macOS/app/test_package.py +++ b/tests/platforms/macOS/app/test_package.py @@ -1,4 +1,5 @@ import os +import subprocess import sys from unittest import mock @@ -23,6 +24,7 @@ def package_command(tmp_path): command.sign_file = mock.MagicMock() command.notarize = mock.MagicMock() command.dmgbuild = mock.MagicMock() + command.tools.subprocess = mock.MagicMock(spec=subprocess) return command @@ -776,6 +778,20 @@ def test_dmg_with_missing_installer_background( ) +@pytest.mark.skipif(sys.platform != "darwin", reason="macOS specific test") +def test_verify(package_command): + "If you're on macOS, you can verify tools." + # Mock the existence of the command line tools + package_command.tools.subprocess.check_output.side_effect = [ + subprocess.CalledProcessError(cmd=["xcode-select", "--install"], returncode=1), + "clang 37.42", # clang --version + ] + + package_command.verify_tools() + + assert package_command.tools.xcode_cli is not None + + @pytest.mark.skipif(sys.platform == "darwin", reason="non-macOS specific test") def test_verify_non_macOS(package_command): "If you're not on macOS, you can't verify tools."