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.

Sign up for GitHub

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

Jump to bottom

Support more browsers #340

Merged
merged 10 commits into from
Dec 7, 2023
Merged

Support more browsers #340

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4d6194a
Support more browsers
jakethekoenig Dec 1, 2023
e29ab87
test fix
jakethekoenig Dec 1, 2023
cb31ab3
Browser determined from OS
jakethekoenig Dec 5, 2023
8219f15
driver type change
jakethekoenig Dec 5, 2023
b5aa068
try/catch in plaform independent way
jakethekoenig Dec 5, 2023
cce8f4b
Merge branch 'main' into more-browsers
jakethekoenig Dec 5, 2023
47874e0
try/catch fix
jakethekoenig Dec 5, 2023
e689ff3
lint fix
jakethekoenig Dec 5, 2023
7acae16
Tell user to enable remote automation
jakethekoenig Dec 6, 2023
ca501b8
Review feedback
jakethekoenig Dec 6, 2023
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
27 changes: 24 additions & 3 deletions mentat/vision/vision_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@
from selenium import webdriver
from selenium.common.exceptions import NoSuchWindowException
from selenium.webdriver.chrome.service import Service
from selenium.webdriver.edge.service import Service as EdgeService
from selenium.webdriver.firefox.service import Service as FirefoxService
from selenium.webdriver.remote.webdriver import WebDriver
from webdriver_manager.chrome import ChromeDriverManager
from webdriver_manager.firefox import GeckoDriverManager
from webdriver_manager.microsoft import EdgeChromiumDriverManager


class ScreenshotException(Exception):
Expand All @@ -17,12 +22,28 @@ class ScreenshotException(Exception):

@attr.define
class VisionManager:
driver: webdriver.Chrome | None = attr.field(default=None)
driver: Optional[WebDriver] = attr.field(default=None)

def _open_browser(self) -> None:
if self.driver is None or not self.driver_running():
service = Service(ChromeDriverManager().install())
self.driver = webdriver.Chrome(service=service)
try:
self.driver = webdriver.Safari()
except Exception:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
except Exception:
except Exception as e:
if "remote automation" in str(e).lower():
stream.send(f"To use Safari, enable remote automation.", color="red")

The first time I tried /screenshot, it failed (didn't recognize Safari, no backups installed). I found I needed to:

  1. In the Safari menu bar, click 'Safari' and then 'Preferences'.
  2. Go to the 'Advanced' tab and check 'Show Develop menu in menu bar'.
  3. Open the 'Develop' menu in the menu bar and choose 'Allow Remote Automation'.

..and then it would work.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah thanks. I think given that I'll try to open with chrome before telling users to enable remote automation. But if they have safari installed I'll print a different message informing them of that option.

try:
service = Service(ChromeDriverManager().install())
self.driver = webdriver.Chrome(service=service)
except Exception:
try:
service = EdgeService(EdgeChromiumDriverManager().install())
self.driver = webdriver.Edge(service=service)
except Exception:
try:
service = FirefoxService(GeckoDriverManager().install())
self.driver = webdriver.Firefox(service=service)
except Exception:
raise ScreenshotException(
"Please install Chrome or Firefox"
)

def open(self, path: str) -> None:
self._open_browser()
Expand Down
12 changes: 10 additions & 2 deletions tests/vision_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,21 @@

@pytest.fixture
def mock_webdriver():
with patch("selenium.webdriver.Chrome") as mock:
with patch("selenium.webdriver.Safari") as mock:
yield mock


def test_vision_manager_screenshot(mock_webdriver, temp_testbed):
@pytest.fixture
def mock_platform():
with patch("platform.system") as mock:
yield mock


def test_vision_manager_screenshot(mock_platform, mock_webdriver, temp_testbed):
mock_driver_instance = MagicMock()
mock_webdriver.return_value = mock_driver_instance

mock_platform.return_value = "Darwin"
mock_driver_instance.get_screenshot_as_png.return_value = b"fake_screenshot_data"

vision_manager = VisionManager()
Expand Down