diff --git a/opensafely/utils.py b/opensafely/utils.py index 46b1284..2bae8fd 100644 --- a/opensafely/utils.py +++ b/opensafely/utils.py @@ -176,11 +176,12 @@ def open_browser(url, timeout=60.0): # wait for port to be open debug("open_browser: waiting for port") start = time.time() + response = None while time.time() - start < timeout: try: response = requests.get(url, timeout=1) except Exception: - pass + time.sleep(0.5) else: break diff --git a/tests/test_rstudio.py b/tests/test_rstudio.py index 77c7149..c2de83b 100644 --- a/tests/test_rstudio.py +++ b/tests/test_rstudio.py @@ -19,7 +19,7 @@ def test_rstudio(run, tmp_path, monkeypatch, gitconfig_exists): # windows monkeypatch.setitem(os.environ, "USERPROFILE", str(home)) - # mock the webbrowser.open call + # mock the open_browser call mock_open_browser = mock.Mock(spec=utils.open_browser) monkeypatch.setattr(utils, "open_browser", mock_open_browser) diff --git a/tests/test_utils.py b/tests/test_utils.py index 4c3ee9d..1b639fc 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -125,3 +125,44 @@ def test_run_docker_interactive_tty(run, no_user, monkeypatch): ], ) utils.run_docker([], "ehrql:v1", [], interactive=True) + + +def test_open_browser_timeout(monkeypatch, capsys): + monkeypatch.setitem(os.environ, "DEBUG", "TRUE") + + mock_open = mock.Mock(spec=utils.webbrowser.open) + monkeypatch.setattr(utils.webbrowser, "open", mock_open) + + port = utils.get_free_port() + url = f"http://localhost:{port}/" + utils.open_browser(url, timeout=0.01) + + assert not mock_open.called + _, err = capsys.readouterr() + assert f"Could not connect to {url}" in err + + +def test_open_browser_error(monkeypatch, capsys): + # turn on debug logging + monkeypatch.setitem(os.environ, "DEBUG", "TRUE") + + # return successful response from poll + mock_get = mock.Mock(spec=utils.requests.get) + response = utils.requests.Response() + response.status_code = 200 + mock_get.return_value = response + monkeypatch.setattr(utils.requests, "get", mock_get) + + # raise exception when calling webbrowser.open + mock_open = mock.Mock(spec=utils.webbrowser.open) + mock_open.side_effect = Exception("TEST ERROR") + monkeypatch.setattr(utils.webbrowser, "open", mock_open) + + port = utils.get_free_port() + url = f"http://localhost:{port}/" + utils.open_browser(url, timeout=0.01) + + mock_open.assert_called_with(url, new=2) + out, err = capsys.readouterr() + assert out == "" + assert "TEST ERROR" in err