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
34 changes: 33 additions & 1 deletion pytest_pyodide/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,39 @@ def run_webworker(self, code):
)

def load_package(self, packages):
self.run_js(f"await pyodide.loadPackage({packages!r})")
# Pyodide's ``loadPackage`` reports failures in two different ways:
#
# * For a known-but-unresolvable package (e.g. a wheel URL that
# 404s), the returned promise resolves normally and the failure
# is only delivered via the ``errorCallback`` option.
Comment on lines +321 to +323
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(No actions required for this PR)

I wonder if we should change this behavior. I am not sure why we designed in that way at the first place.

I guess it was when loadPackage was unstable, we wanted a failure in a single package resolution failure does not block loading other packages. But now, I guess it is weird to make loadPackage silently success even when the package load fails.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Agreed.

# * For an unknown package name, the returned promise rejects
# with a JavaScript ``Error``.
#
# Both paths are easy to miss in tests -- in the first case the
# missing package surfaces later as a confusing
# ``ModuleNotFoundError`` inside Pyodide. We normalize both into a
# single ``RuntimeError`` raised at the call site so load failures
# are always reported immediately and with the problematic package
# reference in the message.
result = self.run_js(
f"""
const __errors = [];
try {{
await pyodide.loadPackage({packages!r}, {{
errorCallback: (msg) => {{ __errors.push(msg); }},
}});
}} catch (e) {{
__errors.push(e.message || String(e));
}}
return __errors;
"""
)
if result:
raise RuntimeError(
"pyodide.loadPackage({!r}) reported errors:\n {}".format(
packages, "\n ".join(result)
)
)


class _SeleniumBaseRunner(_BrowserBaseRunner):
Expand Down
54 changes: 54 additions & 0 deletions tests/test_load_package.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
"""Tests for ``SeleniumBrowserRunner.load_package``.

Pyodide's ``pyodide.loadPackage`` does not raise when a package cannot be
loaded (e.g. the wheel URL 404s or the package name is unknown); it only
invokes the ``errorCallback`` option. Our ``load_package`` wrapper collects
those callback messages and turns them into a ``RuntimeError`` so that the
failure is reported at the call site rather than surfacing later as a
confusing ``ModuleNotFoundError`` inside Pyodide.
"""

import pytest


def test_load_package_succeeds(selenium):
"""Sanity check: loading a package that exists in the Pyodide dist
must not raise, and the package must be importable afterwards."""
selenium.load_package("micropip")
# If the package is really loaded, importing it in Pyodide succeeds.
selenium.run_js("await pyodide.runPythonAsync('import micropip');")


def test_load_package_bad_url_raises(selenium):
"""A URL that 404s must cause load_package to raise, and the error
message must mention the failure reported by Pyodide."""
bad_url = (
f"http://{selenium.server_hostname}:{selenium.server_port}"
"/does-not-exist-pytest_pyodide_test.whl"
)
with pytest.raises(RuntimeError) as exc_info:
selenium.load_package(bad_url)

msg = str(exc_info.value)
assert "loadPackage" in msg
assert bad_url in msg


def test_load_package_unknown_name_raises(selenium):
"""An unknown package name must also cause load_package to raise."""
with pytest.raises(RuntimeError) as exc_info:
selenium.load_package("definitely-not-a-real-package-xyz")

msg = str(exc_info.value)
assert "loadPackage" in msg
assert "definitely-not-a-real-package-xyz" in msg


def test_load_package_partial_failure_raises(selenium):
"""If a list contains both a valid and an invalid package, the call
must still raise so the failure is not silently swallowed."""
with pytest.raises(RuntimeError) as exc_info:
selenium.load_package(["micropip", "definitely-not-a-real-package-xyz"])

msg = str(exc_info.value)
assert "definitely-not-a-real-package-xyz" in msg
Loading