Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 25 additions & 5 deletions foxpuppet/windows/browser/notifications/addons.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,38 @@ def close(self) -> None:
BaseNotification.close(self)


class AddOnInstallRestart(BaseNotification):
"""Add-on install restart notification."""


class AddOnInstallFailed(BaseNotification):
"""Add-on install failed notification."""

@property
def error_message(self):
"""Provide access to the error message.

Returns:
str: The error message explaining why the installation failed.
"""
with self.selenium.context(self.selenium.CONTEXT_CHROME):
return self.find_description().text

def close(self):
"""Close the failed installation notification."""
with self.selenium.context(self.selenium.CONTEXT_CHROME):
self.find_primary_button().click()

Comment on lines +73 to +77
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add this to the Failed test?


class AddOnProgress(BaseNotification):
"""Add-on progress notification."""

@property
def is_downloading(self):
"""Check if the add-on is currently downloading.

Returns:
bool: True if the download and verification is in progress.
"""
with self.selenium.context(self.selenium.CONTEXT_CHROME):
return "Downloading and verifying add-on…" in self.find_description().text
Comment on lines +83 to +90
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what's the idea with this. I can see some use cases but I would like to know you're thinking. How would the API look for a user who wants to use this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this as property here because I think this is a good identifier of the Firefox Progress notification. The idea is for the property to return a boolean. A simple true or false could be used to easily determine if the Progress notification is active or not. In the context of the test for notification, if progress_notification returns the AddOnProgress class, then the API will be progress_notification.is_downloading.



# Clean up of these notifications will happen once Firefox ESR is past version 63
# https://github.com/mozilla/FoxPuppet/issues/212
Expand All @@ -76,7 +97,6 @@ class AddOnProgress(BaseNotification):
"addon-install-confirmation-notification": AddOnInstallConfirmation,
"addon-install-complete-notification": AddOnInstallComplete,
"appMenu-addon-installed-notification": AddOnInstallComplete,
"addon-install-restart-notification": AddOnInstallRestart,
"addon-install-failed-notification": AddOnInstallFailed,
"addon-installed-notification": AddOnInstallComplete,
"addon-progress-notification": AddOnProgress,
Expand Down
15 changes: 9 additions & 6 deletions foxpuppet/windows/browser/window.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
from foxpuppet.windows.browser.navbar import NavBar
from foxpuppet.windows.browser.notifications import BaseNotification
from selenium.webdriver.remote.webelement import WebElement
from typing import Any, Optional, Union
from typing import Any, Optional, Union, TypeVar, Type

T = TypeVar("T", bound="BaseNotification")


class BrowserWindow(BaseWindow):
Expand Down Expand Up @@ -66,8 +68,9 @@ def notification(self) -> BaseNotification | Any:
return None # no notification is displayed

def wait_for_notification(
self, notification_class: Optional[type["BaseNotification"]] = BaseNotification
) -> BaseNotification | Any:
self,
notification_class: Optional[Type[T]] = BaseNotification, # type: ignore
) -> Optional[T]:
"""Wait for the specified notification to be displayed.

Args:
Expand All @@ -77,7 +80,7 @@ def wait_for_notification(
`BaseNotification`.

Returns:
:py:class:`BaseNotification`: Firefox notification.
Optional[:py:class:`BaseNotification`]: Firefox notification or None.

"""
if notification_class:
Expand All @@ -89,13 +92,13 @@ def wait_for_notification(
lambda _: isinstance(self.notification, notification_class),
message=message,
)
return self.notification
return self.notification # type: ignore
else:
self.wait.until(
lambda _: self.notification is None,
message="Unexpected notification shown.",
)
return None
return None

@property
def is_private(self) -> bool | Any:
Expand Down
120 changes: 102 additions & 18 deletions tests/test_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,18 @@

import pytest
from selenium.common.exceptions import TimeoutException
from selenium.webdriver.support.ui import WebDriverWait
from selenium.webdriver.support import expected_conditions as EC
from selenium.webdriver.common.by import By
from typing import Any

from foxpuppet.windows.browser.notifications import BaseNotification
from selenium.webdriver.common.by import By
from foxpuppet.windows.browser.notifications.addons import (
AddOnInstallBlocked,
AddOnInstallComplete,
AddOnInstallConfirmation,
AddOnInstallFailed,
AddOnProgress,
)
from selenium.webdriver.remote.webdriver import WebDriver
from foxpuppet.windows import BrowserWindow
Expand All @@ -21,57 +25,88 @@


@pytest.fixture
def firefox_options(firefox_options: FirefoxOptions) -> FirefoxOptions:
def firefox_options(request, firefox_options: FirefoxOptions) -> FirefoxOptions:
"""Fixture for configuring Firefox."""
# Due to https://bugzilla.mozilla.org/show_bug.cgi?id=1329939 we need the
# initial browser window to be in the foreground. Without this, the
# notifications will not be displayed.
firefox_options.add_argument("-foreground")
if getattr(request, "param", {}).get("page_load_strategy_none", False):
firefox_options.set_capability("pageLoadStrategy", "none")
return firefox_options


class AddOn:
"""Class representing an add-on."""

def __init__(self, name: str, path: str):
def __init__(self, name: str, path_key: str = "default"):
self.name = name
self.path = path
self._paths = {
"default": "webextension.xpi",
"corrupt": "corruptwebextension.xpi",
"large": "largewebextension.xpi",
}
if path_key not in self._paths:
raise ValueError(f"Invalid path key: {path_key}")
self._path_key = path_key
Comment on lines +44 to +51
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the paths dict here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!


@property
def path(self):
"""Returns the current path based on the selected key."""
return self._paths.get(self._path_key)

@path.setter
def path(self, ext_path):
"""Sets the current path key if it exists in paths."""
if ext_path in self._paths:
self._path_key = ext_path
else:
raise ValueError(f"Invalid path key: {ext_path}")
Comment on lines +63 to +64
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a test for this error case?



@pytest.fixture
def addon() -> AddOn:
"""Fixture for creating an installable add-on.

Returns:
:py:class:`AddOn`: Add-on object containing a name and a path to the
add-on.
"""Fixture for creating an installable add-on."""
return AddOn(name="WebExtension")

"""

# https://github.com/ambv/black/issues/144#issuecomment-392149599
@pytest.fixture
def progress_notification(
addon: AddOn, browser: BrowserWindow, webserver: WebServer, selenium: WebDriver
) -> AddOnProgress | None:
"""Fixture that triggers the download progress notification.

return AddOn(name="WebExtension", path="webextension.xpi")
Returns:
:py:class:AddOnProgress: Firefox notification.
"""
addon.path = "large"
selenium.get(webserver.url)
element = WebDriverWait(selenium, 10).until(
EC.element_to_be_clickable((By.LINK_TEXT, addon.path))
)
element.click()
return browser.wait_for_notification(AddOnProgress)


@pytest.fixture
def blocked_notification(
addon: AddOn, browser: BrowserWindow, webserver: WebServer, selenium: WebDriver
) -> BaseNotification:
) -> AddOnInstallBlocked | None:
"""Fixture causing a blocked notification to appear in Firefox.

Returns:
:py:class:`AddOnInstallBlocked`: Firefox notification.

"""
selenium.get(webserver.url())
selenium.get(webserver.url)
selenium.find_element(By.LINK_TEXT, addon.path).click()
return browser.wait_for_notification(AddOnInstallBlocked)


@pytest.fixture
def confirmation_notification(
browser: BrowserWindow, blocked_notification: AddOnInstallBlocked
) -> BaseNotification:
) -> AddOnInstallConfirmation | None:
"""Fixture that allows an add-on to be installed.

Returns:
Expand All @@ -85,7 +120,7 @@ def confirmation_notification(
@pytest.fixture
def complete_notification(
browser: BrowserWindow, confirmation_notification: AddOnInstallConfirmation
) -> BaseNotification:
) -> AddOnInstallComplete | None:
"""Fixture that installs an add-on.

Returns:
Expand All @@ -96,9 +131,29 @@ def complete_notification(
return browser.wait_for_notification(AddOnInstallComplete)


@pytest.fixture
def failed_notification(
addon: AddOn, browser: BrowserWindow, webserver: WebServer, selenium: WebDriver
) -> AddOnInstallFailed | None:
"""Fixture that triggers a failed installation notification.

Returns:
:py:class:`AddOnInstallFailed`: Firefox notification.
"""
addon.path = "corrupt"
selenium.get(webserver.url)
selenium.find_element(By.LINK_TEXT, addon.path).click()
return browser.wait_for_notification(AddOnInstallFailed)


def test_add_on_path(addon: AddOn) -> None:
with pytest.raises(ValueError, match="Invalid path key: doesNotExist"):
addon.path = "doesNotExist"


def test_open_close_notification(
browser: BrowserWindow, blocked_notification: AddOnInstallBlocked
) -> BaseNotification | None:
) -> None:
"""Trigger and dismiss a notification."""
assert blocked_notification is not None
blocked_notification.close()
Expand Down Expand Up @@ -136,7 +191,9 @@ def test_notification_with_origin(
blocked_notification: AddOnInstallBlocked,
) -> None:
"""Trigger a notification with an origin."""
assert blocked_notification.origin is not None
assert (
blocked_notification.origin is not None
), "Notification origin should not be None"
assert f"{webserver.host}" in blocked_notification.origin
assert blocked_notification.label is not None

Expand Down Expand Up @@ -174,3 +231,30 @@ def test_addon_install_complete(
"""Complete add-on installation and close notification."""
complete_notification.close()
browser.wait_for_notification(None)


def test_failed_installation_notification(
failed_notification: AddOnInstallFailed,
) -> None:
"""Test that a failed installation notification is shown for a corrupt add-on."""
error_text = "The add-on downloaded from this site could not be installed because it appears to be corrupt."
assert failed_notification.error_message == error_text


def test_close_failed_notification(
browser: BrowserWindow, failed_notification: AddOnInstallFailed
) -> None:
"""Close Failed Notification"""
failed_notification.close()
browser.wait_for_notification(None)


@pytest.mark.parametrize(
"firefox_options", [{"page_load_strategy_none": True}], indirect=True
)
Comment on lines +252 to +254
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice use of parrametrization!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

def test_progress_notification_downloading(
browser: BrowserWindow, progress_notification: AddOnProgress
) -> None:
"""Verify downloading status is reported correctly."""
description = progress_notification.is_downloading
assert description is True
Binary file added tests/web/corruptwebextension.xpi
Binary file not shown.
Binary file added tests/web/largewebextension.xpi
Binary file not shown.
19 changes: 6 additions & 13 deletions tests/webserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,12 @@ def port(self) -> int:
"""
return self.server.server_address[1]

def start(self) -> None:
@property
def url(self) -> str:
"""Web server URL."""
return "http://{0.host}:{0.port}/".format(self)

def start(self):
"""Start web server."""
self.thread.start()

Expand All @@ -64,18 +69,6 @@ def stop(self) -> None:
self.server.shutdown()
self.thread.join()

def url(self, path="/") -> str:
"""Web server URL.

Args:
path (str, optional): Path to append to the web server URL.

Returns:
str: URL of web server.

"""
return "http://{0.host}:{0.port}{1}".format(self, path)

@classmethod
def get_free_port(cls):
"""Find and return a free port on the system."""
Expand Down
Loading