Skip to content

Commit 52c588e

Browse files
committed
Add browser tests that the widget does not restyle its host
Cover the scoping end-to-end in real Marimo and JupyterLab sessions, asserting the document-level NDL stylesheet has no bare-element or root theme-state rules, and guard the shipped stylesheet directly. Move marimo to the dev group so the Marimo integration test runs in CI (it is not a notebook-only dependency). The marimo example recipes rely on the implicit default group, matching the streamlit recipe.
1 parent 5a6f72d commit 52c588e

7 files changed

Lines changed: 386 additions & 16 deletions

File tree

‎justfile‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,12 +120,12 @@ streamlit:
120120
marimo:
121121
#!/usr/bin/env bash
122122
set -e
123-
cd {{py_dir}} && uv run --group notebook marimo run {{root_dir}}/examples/marimo-example.py
123+
cd {{py_dir}} && uv run marimo run {{root_dir}}/examples/marimo-example.py
124124
125125
marimo-edit:
126126
#!/usr/bin/env bash
127127
set -e
128-
cd {{py_dir}} && uv run --group notebook marimo edit {{root_dir}}/examples/marimo-example.py
128+
cd {{py_dir}} && uv run marimo edit {{root_dir}}/examples/marimo-example.py
129129
130130
ref-docs:
131131
./scripts/render_antora_docs.sh

‎python-wrapper/pyproject.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ dev = [
6565
"streamlit==1.64.0",
6666
"matplotlib>=3.9.4",
6767
"jupyterlab>=4.5.7",
68+
"marimo",
6869
"anywidget[dev]",
6970
"python-dotenv",
7071
]
@@ -86,7 +87,6 @@ notebook = [
8687
"matplotlib>=3.9.4",
8788
"snowflake-snowpark-python==1.55.0",
8889
"requests",
89-
"marimo",
9090
"python-dotenv"
9191
]
9292

‎python-wrapper/tests/browser_harness.py‎

Lines changed: 161 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
import urllib.error
2222
import urllib.request
2323
import uuid
24-
from collections.abc import Iterator, Sequence
24+
from collections.abc import Callable, Iterator, Sequence
2525
from contextlib import contextmanager
2626
from dataclasses import dataclass
2727
from pathlib import Path
@@ -71,6 +71,14 @@ class JupyterServer:
7171
log_path: Path
7272

7373

74+
@dataclass
75+
class MarimoServer:
76+
url: str
77+
root: Path
78+
process: subprocess.Popen[bytes]
79+
log_path: Path
80+
81+
7482
def _free_port() -> int:
7583
with socket.socket() as sock:
7684
sock.bind(("127.0.0.1", 0))
@@ -121,7 +129,45 @@ def jupyter_server(root: Path) -> Iterator[JupyterServer]:
121129
)
122130
server = JupyterServer(url=f"http://127.0.0.1:{port}", root=root, process=process, log_path=log_path)
123131
try:
124-
_wait_until_ready(server)
132+
_wait_until_ready(server, "/api")
133+
yield server
134+
finally:
135+
process.terminate()
136+
process.wait(timeout=10)
137+
138+
139+
@contextmanager
140+
def marimo_server(root: Path, app: Path) -> Iterator[MarimoServer]:
141+
"""Boot a disposable, tokenless Marimo server running ``app``.
142+
143+
The server process is terminated when the context exits. Marimo executes
144+
every cell of the app on load, so the app must be self-contained (no
145+
kernel-side interaction needed from the test).
146+
"""
147+
log_path = root / "marimo-server.log"
148+
port = _free_port()
149+
with log_path.open("wb") as log_file:
150+
process = subprocess.Popen(
151+
[
152+
sys.executable,
153+
"-m",
154+
"marimo",
155+
"run",
156+
str(app),
157+
"--headless",
158+
"--no-token",
159+
"--host",
160+
"127.0.0.1",
161+
"--port",
162+
str(port),
163+
],
164+
stdout=log_file,
165+
stderr=subprocess.STDOUT,
166+
cwd=root,
167+
)
168+
server = MarimoServer(url=f"http://127.0.0.1:{port}", root=root, process=process, log_path=log_path)
169+
try:
170+
_wait_until_ready(server, "/health")
125171
yield server
126172
finally:
127173
process.terminate()
@@ -147,15 +193,17 @@ def create_notebook(server: JupyterServer, cells: Sequence[str]) -> str:
147193
return name
148194

149195

150-
def _wait_until_ready(server: JupyterServer) -> None:
196+
def _wait_until_ready(server: JupyterServer | MarimoServer, path: str) -> None:
151197
deadline = time.monotonic() + SERVER_START_TIMEOUT
152198
while time.monotonic() < deadline:
153199
try:
154-
with urllib.request.urlopen(f"{server.url}/api", timeout=2):
200+
with urllib.request.urlopen(f"{server.url}{path}", timeout=2):
155201
return
156202
except (urllib.error.URLError, OSError):
157203
time.sleep(0.25)
158-
raise RuntimeError(f"JupyterLab did not start within {SERVER_START_TIMEOUT}s; log:\n{server.log_path.read_text()}")
204+
raise RuntimeError(
205+
f"server at {server.url} did not start within {SERVER_START_TIMEOUT}s; log:\n{server.log_path.read_text()}"
206+
)
159207

160208

161209
def _launch_chromium(playwright: Playwright) -> Browser:
@@ -243,20 +291,92 @@ def _await_notebook_markers(server: JupyterServer) -> None:
243291
raise RuntimeError(f"notebook did not finish within {NOTEBOOK_RUN_TIMEOUT}s; log:\n{server.log_path.read_text()}")
244292

245293

246-
def _dump_debug_artifacts(server: JupyterServer, page: Page, console_messages: Sequence[str]) -> None:
294+
def _dump_debug_artifacts(server: JupyterServer | MarimoServer, page: Page, console_messages: Sequence[str]) -> None:
247295
page.screenshot(path=str(server.root / "debug-screenshot.png"))
248296
(server.root / "debug-page.html").write_text(page.content())
249297
(server.root / "debug-console.log").write_text("\n".join(console_messages))
250298

251299

252-
def run_notebook_in_browser(server: JupyterServer, cells: Sequence[str]) -> None:
300+
# Finds the NDL stylesheet among the document's stylesheets by a marker only it
301+
# defines, then reports any rule whose selector is a bare host element. The
302+
# host's own stylesheets (JupyterLab's / Marimo's) legitimately contain bare
303+
# selectors, so they are ignored. Used to assert the widget's stylesheet is
304+
# scoped to the widget wrapper (GDS-440).
305+
NDL_BARE_SELECTOR_SCRIPT = """\
306+
() => {
307+
const NDL_MARKER = "--theme-color-primary";
308+
const BARE = new Set([
309+
"h1", "h2", "h3", "h4", "h5", "h6", "a", "ul", "ol", "li", "p", "body",
310+
"html", "table", "button", "input", "select", "textarea", "img", "svg",
311+
"strong", "em", "pre", "code", "hr", "blockquote", "dl", "dd", "fieldset",
312+
"legend", "summary", "dialog", "*",
313+
]);
314+
const THEME_PROPS = ["color-scheme", "--lightningcss-light", "--lightningcss-dark"];
315+
const offenders = [];
316+
const themeOffenders = [];
317+
const scan = (rules) => {
318+
for (const rule of rules) {
319+
if (rule.selectorText) {
320+
const parts = rule.selectorText.split(",").map((s) => s.trim());
321+
for (const sel of parts) {
322+
if (BARE.has(sel)) offenders.push(sel);
323+
}
324+
// NDL's `color-scheme`/lightningcss toggles on the document root would
325+
// flip the host's own light/dark resolution.
326+
const isRootSel = (p) =>
327+
p === "html" || p === "body" || p.includes(":root") || p.includes(":host");
328+
if (rule.style && parts.some(isRootSel)) {
329+
for (const prop of THEME_PROPS) {
330+
if (rule.style.getPropertyValue(prop)) themeOffenders.push(`${rule.selectorText.trim()} {${prop}}`);
331+
}
332+
}
333+
} else if (rule.cssRules) {
334+
scan(rule.cssRules);
335+
}
336+
}
337+
};
338+
const sheets = [...document.styleSheets, ...(document.adoptedStyleSheets || [])];
339+
let ndlSheets = 0;
340+
for (const sheet of sheets) {
341+
let rules;
342+
try {
343+
rules = [...sheet.cssRules];
344+
} catch {
345+
continue;
346+
}
347+
const isNdl = rules.some((rule) => (rule.cssText || "").includes(NDL_MARKER));
348+
if (!isNdl) continue;
349+
ndlSheets += 1;
350+
scan(rules);
351+
}
352+
return { ndlSheets, offenders: [...new Set(offenders)], themeOffenders: [...new Set(themeOffenders)] };
353+
}
354+
"""
355+
356+
357+
def assert_no_ndl_leak(page: Page) -> None:
358+
"""Assert the NDL stylesheet applied to the host document cannot restyle it."""
359+
result = page.evaluate(NDL_BARE_SELECTOR_SCRIPT)
360+
assert result["ndlSheets"] >= 1, "the NDL stylesheet was never applied to the document"
361+
assert result["offenders"] == [], (
362+
f"the NDL stylesheet leaks bare-element selectors to the host page: {result['offenders']}"
363+
)
364+
assert result["themeOffenders"] == [], (
365+
f"the NDL stylesheet leaks theme state to the host page: {result['themeOffenders']}"
366+
)
367+
368+
369+
def run_notebook_in_browser(
370+
server: JupyterServer, cells: Sequence[str], check: Callable[[Page], None] | None = None
371+
) -> None:
253372
"""Create a notebook from ``cells`` on ``server`` and run it in headless Chrome.
254373
255374
The last cell must write the ``_done.txt``/``_error.txt`` markers (see
256375
``DONE_MARKER``/``ERROR_MARKER``); the second-to-last is expected to display
257-
a widget, which is awaited via its canvas before the last cell runs. Any
258-
browser console error fails the run, and failures dump debug artifacts
259-
(screenshot, page HTML, console log) into the server root.
376+
a widget, which is awaited via its canvas before the last cell runs. When
377+
``check`` is given it runs after the widget has mounted. Any browser console
378+
error fails the run, and failures dump debug artifacts (screenshot, page
379+
HTML, console log) into the server root.
260380
"""
261381
for marker in (DONE_MARKER, ERROR_MARKER):
262382
(server.root / marker).unlink(missing_ok=True)
@@ -283,6 +403,8 @@ def record_console(message: ConsoleMessage) -> None:
283403
workspace = uuid.uuid4().hex[:8]
284404
page.goto(f"{server.url}/lab/workspaces/{workspace}/tree/{notebook_name}")
285405
_execute_notebook_in_browser(page, cells)
406+
if check is not None:
407+
check(page)
286408
_await_notebook_markers(server)
287409
except Exception:
288410
_dump_debug_artifacts(server, page, console_messages)
@@ -291,3 +413,32 @@ def record_console(message: ConsoleMessage) -> None:
291413
browser.close()
292414
if console_errors:
293415
raise RuntimeError(f"browser console errors: {console_errors}")
416+
417+
418+
def run_marimo_app_in_browser(server: MarimoServer, check: Callable[[Page], None]) -> None:
419+
"""Open the Marimo app served by ``server`` in headless Chrome and run ``check``.
420+
421+
Waits for the graph widget to mount (its canvas inside the scope wrapper)
422+
before invoking ``check``, which performs the assertions. Failures dump
423+
debug artifacts (screenshot, page HTML, console log) into the server root.
424+
"""
425+
console_messages: list[str] = []
426+
427+
def record_console(message: ConsoleMessage) -> None:
428+
console_messages.append(f"{message.type}: {message.text}")
429+
430+
with sync_playwright() as playwright:
431+
browser = _launch_chromium(playwright)
432+
try:
433+
page = browser.new_page(viewport={"width": 1280, "height": 900})
434+
page.on("console", record_console)
435+
page.on("pageerror", lambda error: console_messages.append(f"uncaught page error: {error}"))
436+
try:
437+
page.goto(server.url, wait_until="load")
438+
page.wait_for_selector("[data-neo4j-viz-ndl] canvas", timeout=WIDGET_MOUNT_TIMEOUT * 1000)
439+
check(page)
440+
except Exception:
441+
_dump_debug_artifacts(server, page, console_messages)
442+
raise
443+
finally:
444+
browser.close()
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
"""Guard that the shipped NDL stylesheet is scoped to the widget wrapper.
2+
3+
The stylesheet is shipped as the widget's ``_css`` and applied at the host
4+
document level (as the overlay copy in document.head), so any bare element
5+
selector in it (``h1``, ``a``, ``*``, …) would reset the host page, and any
6+
theme-state declaration on the document root (``color-scheme``,
7+
``--lightningcss-*``) would flip the host's own light/dark resolution. The build
8+
scopes/relocates both; this test fails if an unscoped selector or a root-level
9+
theme-state declaration ever ships again (see GDS-440).
10+
"""
11+
12+
from __future__ import annotations
13+
14+
import re
15+
from pathlib import Path
16+
17+
import neo4j_viz
18+
19+
STYLE_CSS = Path(neo4j_viz.__file__).parent / "resources" / "nvl_entrypoint" / "style.css"
20+
21+
# A class or id selector, e.g. `.ndl-btn`. Requires a name after the sigil and
22+
# is not preceded by an attribute/string operator, so `[href*=".com"]` is not
23+
# mistaken for a class.
24+
_CLASS_OR_ID = re.compile(r"""(?<![="'*~|^$])[.#][A-Za-z_-]""")
25+
_THEME_STATE_PROPS = ("color-scheme", "--lightningcss-light", "--lightningcss-dark")
26+
27+
28+
def _top_level_rules(css: str) -> list[tuple[str, str]]:
29+
"""``(selector_list, declaration_block)`` for every style rule at the top level."""
30+
rules: list[tuple[str, str]] = []
31+
depth = 0
32+
buf = ""
33+
i = 0
34+
n = len(css)
35+
while i < n:
36+
ch = css[i]
37+
if ch in "\"'":
38+
quote = ch
39+
buf += ch
40+
i += 1
41+
while i < n and css[i] != quote:
42+
if css[i] == "\\":
43+
buf += css[i]
44+
i += 1
45+
buf += css[i]
46+
i += 1
47+
if i < n:
48+
buf += css[i]
49+
i += 1
50+
continue
51+
buf += ch
52+
if ch == "{":
53+
depth += 1
54+
elif ch == "}":
55+
depth -= 1
56+
if depth == 0:
57+
selector, _, body = buf.partition("{")
58+
body = body.rstrip("}")
59+
if not selector.strip().startswith("@"):
60+
rules.append((selector.strip(), body))
61+
buf = ""
62+
i += 1
63+
return rules
64+
65+
66+
def _split_top_level(selector_list: str) -> list[str]:
67+
"""Split a selector list on commas outside parentheses/brackets."""
68+
parts: list[str] = []
69+
depth = 0
70+
start = 0
71+
for i, ch in enumerate(selector_list):
72+
if ch in "([":
73+
depth += 1
74+
elif ch in ")]":
75+
depth -= 1
76+
elif ch == "," and depth == 0:
77+
parts.append(selector_list[start:i])
78+
start = i + 1
79+
parts.append(selector_list[start:])
80+
return parts
81+
82+
83+
def _is_root_selector(selector: str) -> bool:
84+
return selector in ("html", "body") or ":root" in selector or ":host" in selector
85+
86+
87+
def test_shipped_style_css_has_no_unscoped_bare_selectors() -> None:
88+
css = STYLE_CSS.read_text()
89+
90+
offenders = []
91+
for selector_list, _ in _top_level_rules(css):
92+
for selector in _split_top_level(selector_list):
93+
core = selector.strip()
94+
if not core:
95+
continue
96+
if ":root" in core or ":host" in core:
97+
continue
98+
if _CLASS_OR_ID.search(core):
99+
continue
100+
if "data-neo4j-viz-ndl" in core:
101+
continue
102+
offenders.append(core)
103+
104+
assert offenders == [], f"unscoped bare-element selectors leaked into style.css: {offenders}"
105+
106+
107+
def test_shipped_style_css_has_no_root_theme_state() -> None:
108+
css = STYLE_CSS.read_text()
109+
110+
offenders = []
111+
for selector_list, body in _top_level_rules(css):
112+
if not any(_is_root_selector(s.strip()) for s in _split_top_level(selector_list)):
113+
continue
114+
for prop in _THEME_STATE_PROPS:
115+
if re.search(rf"(?:^|;)\s*{re.escape(prop)}\s*:", body):
116+
offenders.append(f"{selector_list.strip()} {{{prop}}}")
117+
118+
assert offenders == [], f"theme state leaked onto the document root in style.css: {offenders}"

0 commit comments

Comments
 (0)