Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Replace use of str(pathlib.Path(...)) in codebase #599

Merged
merged 9 commits into from
Sep 5, 2021
Prev Previous commit
Next Next commit
Merge remote-tracking branch 'upstream/master'
  • Loading branch information
iamzili committed Jul 12, 2021
commit 111a8c418f0fad1e6acdb9a3fd001a9260cd6368
2 changes: 1 addition & 1 deletion src/briefcase/integrations/android_sdk.py
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,7 @@ def logcat(self):
# the full output and can send input.
self.command.subprocess.run(
[
str(self.android_sdk.adb_path),
os.fsdecode(self.android_sdk.adb_path),
"-s",
self.device,
"logcat",
Expand Down
4 changes: 2 additions & 2 deletions src/briefcase/platforms/macOS/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def run_app(self, app: BaseConfig, **kwargs):
[
'open',
'-n', # Force a new app to be launched
str(self.binary_path(app)),
os.fsdecode(self.binary_path(app)),
],
check=True,
)
Expand Down Expand Up @@ -77,7 +77,7 @@ def run_app(self, app: BaseConfig, **kwargs):
'senderImagePath=="{sender}"'
' OR (processImagePath=="{sender}"'
' AND senderImagePath=="/usr/lib/libffi.dylib")'.format(
sender=str(self.binary_path(app) / "Contents" / "MacOS" / app.formal_name)
sender=os.fsdecode(self.binary_path(app) / "Contents" / "MacOS" / app.formal_name)
)
],
check=True,
Expand Down
27 changes: 0 additions & 27 deletions src/briefcase/platforms/macOS/app.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import os
import subprocess
import tempfile
from pathlib import Path

Expand Down Expand Up @@ -68,32 +67,6 @@ class macOSAppBuildCommand(macOSAppMixin, BuildCommand):
class macOSAppRunCommand(macOSRunMixin, macOSAppMixin, RunCommand):
description = "Run a macOS app."

def run_app(self, app: BaseConfig, **kwargs):
"""
Start the application.

:param app: The config object for the app
:param base_path: The path to the project directory.
"""
print()
print('[{app.app_name}] Starting app...'.format(
app=app
))
try:
print()
self.subprocess.run(
[
'open',
os.fsdecode(self.binary_path(app)),
],
check=True,
)
except subprocess.CalledProcessError:
print()
raise BriefcaseCommandError(
"Unable to start app {app.app_name}.".format(app=app)
)


class macOSAppPackageCommand(macOSPackageMixin, macOSAppMixin, PackageCommand):
description = "Package a macOS app for distribution."
Expand Down
23 changes: 1 addition & 22 deletions src/briefcase/platforms/macOS/xcode.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import os
import subprocess

from briefcase.commands import (
Expand All @@ -11,8 +10,8 @@
)
from briefcase.config import BaseConfig
from briefcase.exceptions import BriefcaseCommandError
from briefcase.platforms.macOS import macOSMixin, macOSRunMixin, macOSPackageMixin
from briefcase.integrations.xcode import verify_xcode_install
from briefcase.platforms.macOS import macOSMixin, macOSPackageMixin


class macOSXcodeMixin(macOSMixin):
Expand Down Expand Up @@ -112,26 +111,6 @@ def build_app(self, app: BaseConfig, **kwargs):
class macOSXcodeRunCommand(macOSRunMixin, macOSXcodeMixin, RunCommand):
description = "Run a macOS app."

def run_app(self, app: BaseConfig, **kwargs):
"""
Start the application.

:param app: The config object for the app
:param base_path: The path to the project directory.
"""
print()
print('[{app.app_name}] Starting app...'.format(
app=app
))
try:
print()
self.subprocess.run(['open', os.fsdecode(self.binary_path(app))], check=True)
except subprocess.CalledProcessError:
print()
raise BriefcaseCommandError(
"Unable to start app {app.app_name}.".format(app=app)
)


class macOSXcodePackageCommand(macOSPackageMixin, macOSXcodeMixin, PackageCommand):
description = "Package a macOS app for distribution."
Expand Down
3 changes: 2 additions & 1 deletion tests/integrations/android_sdk/ADB/test_logcat.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import os
import subprocess
from unittest.mock import MagicMock

Expand All @@ -18,7 +19,7 @@ def test_logcat(mock_sdk):
# Validate call parameters.
mock_sdk.command.subprocess.run.assert_called_once_with(
[
str(mock_sdk.adb_path),
os.fsdecode(mock_sdk.adb_path),
"-s",
"exampleDevice",
"logcat",
Expand Down
31 changes: 24 additions & 7 deletions tests/platforms/macOS/app/test_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,35 @@ def test_run_app(first_app_config, tmp_path):

command.run_app(first_app_config)

command.subprocess.run.assert_called_with(
['open', os.fsdecode(command.binary_path(first_app_config))],
check=True
)
# Calls were made to start the app and to start a log stream.
bin_path = command.binary_path(first_app_config)
command.subprocess.run.assert_has_calls([
mock.call(
['open', '-n', os.fsdecode(bin_path)],
check=True
),
mock.call(
[
'log', 'stream',
'--style', 'compact',
'--predicate',
'senderImagePath=="{sender}"'
' OR (processImagePath=="{sender}"'
' AND senderImagePath=="/usr/lib/libffi.dylib")'.format(
sender=bin_path / "Contents" / "MacOS" / "First App"
)
],
check=True,
)
])


def test_run_app_failed(first_app_config, tmp_path):
"If there's a problem started the app, an exception is raised"
command = macOSAppRunCommand(base_path=tmp_path)
command.subprocess = mock.MagicMock()
command.subprocess.run.side_effect = subprocess.CalledProcessError(
cmd=['open', os.fsdecode(command.binary_path(first_app_config))],
cmd=['open', '-n', os.fsdecode(command.binary_path(first_app_config))],
returncode=1
)

Expand All @@ -35,7 +52,7 @@ def test_run_app_failed(first_app_config, tmp_path):

# The run command was still invoked, though
command.subprocess.run.assert_called_with(
['open', os.fsdecode(command.binary_path(first_app_config))],
['open', '-n', os.fsdecode(command.binary_path(first_app_config))],
check=True
)

Expand All @@ -60,7 +77,7 @@ def test_run_app_log_stream_failed(first_app_config, tmp_path):
bin_path = command.binary_path(first_app_config)
command.subprocess.run.assert_has_calls([
mock.call(
['open', '-n', str(bin_path)],
['open', '-n', os.fsdecode(bin_path)],
check=True
),
mock.call(
Expand Down
48 changes: 24 additions & 24 deletions tests/platforms/macOS/xcode/test_run.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Xcode uses the same run implementation as the base app;
# Run a basic test to ensure coverage, but fall back to
# the app backend for exhaustive tests.
import os
import subprocess
from unittest import mock

from briefcase.platforms.macOS.xcode import macOSXcodeRunCommand
Expand All @@ -12,26 +14,24 @@ def test_run_app(first_app_config, tmp_path):

command.run_app(first_app_config)

command.subprocess.run.assert_called_with(
['open', os.fsdecode(command.binary_path(first_app_config))],
check=True
)


def test_run_app_failed(first_app_config, tmp_path):
"If there's a problem started the app, an exception is raised"
command = macOSXcodeRunCommand(base_path=tmp_path)
command.subprocess = mock.MagicMock()
command.subprocess.run.side_effect = subprocess.CalledProcessError(
cmd=['open', os.fsdecode(command.binary_path(first_app_config))],
returncode=1
)

with pytest.raises(BriefcaseCommandError):
command.run_app(first_app_config)

# The run command was still invoked, though
command.subprocess.run.assert_called_with(
['open', os.fsdecode(command.binary_path(first_app_config))],
check=True
)
# Calls were made to start the app and to start a log stream.
bin_path = command.binary_path(first_app_config)
command.subprocess.run.assert_has_calls([
mock.call(
['open', '-n', os.fsdecode(bin_path)],
check=True
),
mock.call(
[
'log', 'stream',
'--style', 'compact',
'--predicate',
'senderImagePath=="{sender}"'
' OR (processImagePath=="{sender}"'
' AND senderImagePath=="/usr/lib/libffi.dylib")'.format(
sender=bin_path / "Contents" / "MacOS" / "First App"
)
],
check=True,
)
])
You are viewing a condensed version of this merge commit. You can view the full changes here.