Skip to content

Test that https://unpkg.com is used only in the connected mode #442

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Test that https://unpkg.com is used only in the connected mode #442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions docs/changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
ITables ChangeLog
=================

2.5.3-dev (unreleased)
------------------

**Added**
- Added a test to check that the offline mode does not load ITables from https://unpkg.com (the connected mode does) ([#441](https://github.com/mwouts/itables/issues/441))


2.5.2 (2025-09-02)
------------------

Expand Down
2 changes: 1 addition & 1 deletion src/itables/version.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
"""ITables' version number"""

__version__ = "2.5.2"
__version__ = "2.5.3dev"
6 changes: 6 additions & 0 deletions tests/test_connected_notebook_is_small.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@
f"Notebook size is too large: {nb_ipynb.stat().st_size} bytes:\n"
f"{nb_ipynb.read_text()}"
)
assert (
"https://unpkg.com" in nb_ipynb.read_text()

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High test

The string
https://unpkg.com
Loading
may be at an arbitrary position in the sanitized URL.

Copilot Autofix

AI 2 months ago

To more robustly verify that the notebook is importing itables from unpkg.com, the test should look for a JavaScript/CSS import statement or a precise URL string within a code cell or output area that matches the expected pattern, rather than a simple substring match. This can be done by parsing the notebook's JSON, examining the cells for references to "https://unpkg.com/itables" (or whatever is the exact import string).

How to fix:

  • Parse the notebook file (nb_ipynb) as JSON.
  • Iterate through the notebook's code cells (and possibly outputs), looking for code or output containing an exact URL import from unpkg.com, e.g. as a regex or full string match.
  • Make the assertion based on this precise check rather than substring matching.

What to change:

  • In test_connected_notebook_is_small, replace the substring test in line 36 with code that parses the notebook JSON and checks whether any cell source or output contains the exact expected import URL for itables from unpkg.com.
  • Import the json module at the top of the file, if not already imported.
Suggested changeset 1
tests/test_connected_notebook_is_small.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/tests/test_connected_notebook_is_small.py b/tests/test_connected_notebook_is_small.py
--- a/tests/test_connected_notebook_is_small.py
+++ b/tests/test_connected_notebook_is_small.py
@@ -1,7 +1,7 @@
 import pytest
 from jupytext.cli import jupytext
+import json
 
-
 def text_notebook(connected, display_logo_when_loading=True):
     return f"""# %%
 import itables
@@ -32,9 +31,35 @@
         f"Notebook size is too large: {nb_ipynb.stat().st_size} bytes:\n"
         f"{nb_ipynb.read_text()}"
     )
-    assert (
-        "https://unpkg.com" in nb_ipynb.read_text()
-    ), "The connected notebook should import itables from unpkg.com"
+    notebook = json.loads(nb_ipynb.read_text())
+    found = False
+    expected_domain = "https://unpkg.com/itables"
+    for cell in notebook.get("cells", []):
+        # Check source code of each cell for import from unpkg.com/itables
+        if any(
+            expected_domain in line
+            for line in (cell.get("source", "") if isinstance(cell.get("source", ""), list) else [cell.get("source", "")])
+        ):
+            found = True
+            break
+        # Also check cell outputs (HTML/javascript might be injected here)
+        for output in cell.get("outputs", []):
+            # Output text may be a list of lines or a single string
+            data = output.get("data", {})
+            for value in data.values():
+                if isinstance(value, str):
+                    if expected_domain in value:
+                        found = True
+                        break
+                elif isinstance(value, list):
+                    if any(expected_domain in v for v in value if isinstance(v, str)):
+                        found = True
+                        break
+            if found:
+                break
+        if found:
+            break
+    assert found, "The connected notebook should import itables from unpkg.com"
 
 
 def test_offline_notebook_is_not_too_large(tmp_path):
EOF
@@ -1,7 +1,7 @@
import pytest
from jupytext.cli import jupytext
import json


def text_notebook(connected, display_logo_when_loading=True):
return f"""# %%
import itables
@@ -32,9 +31,35 @@
f"Notebook size is too large: {nb_ipynb.stat().st_size} bytes:\n"
f"{nb_ipynb.read_text()}"
)
assert (
"https://unpkg.com" in nb_ipynb.read_text()
), "The connected notebook should import itables from unpkg.com"
notebook = json.loads(nb_ipynb.read_text())
found = False
expected_domain = "https://unpkg.com/itables"
for cell in notebook.get("cells", []):
# Check source code of each cell for import from unpkg.com/itables
if any(
expected_domain in line
for line in (cell.get("source", "") if isinstance(cell.get("source", ""), list) else [cell.get("source", "")])
):
found = True
break
# Also check cell outputs (HTML/javascript might be injected here)
for output in cell.get("outputs", []):
# Output text may be a list of lines or a single string
data = output.get("data", {})
for value in data.values():
if isinstance(value, str):
if expected_domain in value:
found = True
break
elif isinstance(value, list):
if any(expected_domain in v for v in value if isinstance(v, str)):
found = True
break
if found:
break
if found:
break
assert found, "The connected notebook should import itables from unpkg.com"


def test_offline_notebook_is_not_too_large(tmp_path):
Copilot is powered by AI and may make mistakes. Always verify output.
), "The connected notebook should import itables from unpkg.com"


def test_offline_notebook_is_not_too_large(tmp_path):
Expand All @@ -43,3 +46,6 @@
size_in_kb = nb_ipynb.stat().st_size // 1024

assert 850 < size_in_kb < 950
assert (
"https://unpkg.com" not in nb_ipynb.read_text()
), "The connected notebook should import itables locally"
Loading