Skip to content

Commit

Permalink
Correct the fallback handling when an icon cannot be loaded.
Browse files Browse the repository at this point in the history
  • Loading branch information
freakboy3742 committed May 7, 2024
1 parent 0c6a2b8 commit e8be59a
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 16 deletions.
1 change: 1 addition & 0 deletions changes/2558.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
If an icon file exists but cannot be loaded, or an application icon cannot be found, Toga will fall back to a default icon.
5 changes: 2 additions & 3 deletions core/src/toga/icons.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ def __init__(
:param system: **For internal use only**
"""
self.factory = get_platform_factory()

try:
# Try to load the icon with the given path snippet. If the request is for the
# app icon, use ``resources/<app name>`` as the path.
Expand Down Expand Up @@ -130,7 +129,7 @@ def __init__(
)

self._impl = self.factory.Icon(interface=self, path=full_path)
except FileNotFoundError:
except (FileNotFoundError, ValueError):
# Icon path couldn't be loaded. If the path is the sentinel for the app
# icon, and this isn't running as a script, fall back to the application
# binary
Expand All @@ -143,7 +142,7 @@ def __init__(
try:
# Use the application binary's icon
self._impl = self.factory.Icon(interface=self, path=None)
except FileNotFoundError:
except (FileNotFoundError, ValueError):
# Can't find the application binary's icon.
print(
"WARNING: Can't find app icon; falling back to default icon"
Expand Down
66 changes: 61 additions & 5 deletions core/tests/test_icons.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,15 @@ def test_create(monkeypatch, app, path, system, sizes, extensions, final_paths):
assert icon._impl.path == final_paths


def test_create_fallback(app, capsys):
def test_create_fallback_missing(monkeypatch, app, capsys):
"""If a resource doesn't exist, a fallback icon is used."""
# Prime the dummy so the app icon cannot be loaded
monkeypatch.setattr(
DummyIcon,
"ICON_FAILURE",
FileNotFoundError(),
)

icon = toga.Icon("resources/missing")

assert icon._impl is not None
Expand All @@ -127,6 +134,27 @@ def test_create_fallback(app, capsys):
)


def test_create_fallback_unloadable(monkeypatch, app, capsys):
"""If a resource exists, but can't be loaded, a fallback icon is used."""
# Prime the dummy so the app icon cannot be loaded
monkeypatch.setattr(
DummyIcon,
"ICON_FAILURE",
ValueError("Icon could not be loaded"),
)

icon = toga.Icon("resources/sample")

assert icon._impl is not None
assert icon._impl.interface == toga.Icon.DEFAULT_ICON

# A warning was printed; allow for windows separators
assert (
"WARNING: Can't find icon resources/sample"
in capsys.readouterr().out.replace("\\", "/")
)


def test_create_fallback_variants(monkeypatch, app, capsys):
"""If a resource with size variants doesn't exist, a fallback icon is used."""
monkeypatch.setattr(DummyIcon, "SIZES", [32, 72])
Expand Down Expand Up @@ -164,7 +192,7 @@ def test_create_app_icon(monkeypatch, app, capsys):


def test_create_app_icon_missing(monkeypatch, app, capsys):
"""The app icon can be constructed"""
"""If the app icon is missing, a fallback is used"""
# When running under pytest, code will identify as running as a script

# Load the app default icon.
Expand Down Expand Up @@ -195,10 +223,38 @@ def test_create_app_icon_non_script(monkeypatch, app, capsys):
assert capsys.readouterr().out == ""


def test_create_app_icon_missing_non_script(monkeypatch, app, capsys):
"""The binary executableThe app icon can be reset to the default"""
def test_create_app_icon_unloadable_non_script(monkeypatch, app, capsys):
"""If the icon from binary executable cannot be loaded, the app icon is reset to the default"""
# Prime the dummy so the app icon cannot be loaded
monkeypatch.setattr(DummyIcon, "ICON_EXISTS", False)
monkeypatch.setattr(
DummyIcon,
"ICON_FAILURE",
ValueError("Icon could not be loaded"),
)

# Patch sys.executable so the test looks like it's running as a packaged binary
monkeypatch.setattr(sys, "executable", "/path/to/App")

# Load the app default icon
icon = toga.Icon(_APP_ICON)

assert isinstance(icon, toga.Icon)
# App icon path reports as `resources/<app_name>`; impl is the default toga icon
assert icon.path == Path("resources/icons")
assert icon._impl.path == Path(TOGA_RESOURCES / "toga.png")

# A warning was printed; allow for windows separators
assert "WARNING: Can't find app icon" in capsys.readouterr().out.replace("\\", "/")


def test_create_app_icon_missing_non_script(monkeypatch, app, capsys):
"""If the icon from binary executable cannot be found, the app icon is reset to the default"""
# Prime the dummy so the app icon cannot be found
monkeypatch.setattr(
DummyIcon,
"ICON_FAILURE",
FileNotFoundError(),
)

# Patch sys.executable so the test looks like it's running as a packaged binary
monkeypatch.setattr(sys, "executable", "/path/to/App")
Expand Down
17 changes: 9 additions & 8 deletions dummy/src/toga_dummy/icons.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,19 @@


class Icon(LoggedObject):
ICON_EXISTS = True
ICON_FAILURE = None
EXTENSIONS = [".png", ".ico"]
SIZES = None

def __init__(self, interface, path):
super().__init__()
self.interface = interface
if not self.ICON_EXISTS:
raise FileNotFoundError("Couldn't find icon")
elif path is None:
self.path = "<APP ICON>"
elif path == {}:
raise FileNotFoundError("No image variants found")
if self.ICON_FAILURE:
raise self.ICON_FAILURE
else:
self.path = path
if path is None:
self.path = "<APP ICON>"
elif path == {}:
raise FileNotFoundError("No image variants found")
else:
self.path = path

0 comments on commit e8be59a

Please sign in to comment.