Skip to content

test: mock network calls in altair chart dataframe tests#10188

Merged
mscolnick merged 1 commit into
mainfrom
ms/fix/mock-altair-chart-tests
Jul 16, 2026
Merged

test: mock network calls in altair chart dataframe tests#10188
mscolnick merged 1 commit into
mainfrom
ms/fix/mock-altair-chart-tests

Conversation

@mscolnick

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI review requested due to automatic review settings July 15, 2026 18:15
@vercel

vercel Bot commented Jul 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
marimo-docs Ready Ready Preview, Comment Jul 15, 2026 6:16pm

Request Review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 1 file

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tests/_plugins/ui/_impl/test_altair_chart.py">

<violation number="1" location="tests/_plugins/ui/_impl/test_altair_chart.py:780">
P1: The `unittest.mock.patch("urllib.request.urlopen", ...)` in `test_get_dataframe_csv()` does not intercept the actual network call. When the chart data URL ends with `.csv`, the implementation in `_get_dataframe_from_chart` calls `pl.read_csv(url)` (or `pd.read_csv(url)`), both of which handle HTTP natively — they do **not** use `urllib.request.urlopen`. The mock is a no-op, so the test still makes a real HTTP request to the CDN.

This means the test (1) depends on external network availability, (2) will fail in airgapped or offline environments, and (3) defeats the PR's goal of mocking network calls.

To fix, either use a local file path or CSV string instead of a URL, or patch `polars.read_csv` / `pandas.read_csv` directly depending on which library the test environment provides.</violation>
</file>
Architecture diagram
sequenceDiagram
    participant Test as Test Function
    participant Mock as unittest.mock.patch
    participant URL as urllib.request.urlopen
    participant Chart as altair_chart()
    participant DF as DataFrame (pd/pl)

    Note over Test,DF: Mocked Network Call Flow
    Test->>Mock: patch("urllib.request.urlopen", ...)
    Mock-->>URL: replace with io.BytesIO(fake_payload)
    Test->>Chart: altair_chart(alt.Chart(data).mark_point()...)
    Chart->>URL: fetch data from URL (e.g., stocks.csv)
    URL-->>Chart: return fake_payload bytes
    Chart->>Chart: parse CSV/JSON into DataFrame
    Chart-->>Test: chart.dataframe
    Test->>Test: assert isinstance(df, pd.DataFrame or pl.DataFrame)
Loading

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

data = "https://cdn.jsdelivr.net/npm/vega-datasets@v1.29.0/data/stocks.csv"
chart = altair_chart(alt.Chart(data).mark_point().encode(x="values:Q"))
assert isinstance(chart.dataframe, (pd.DataFrame, pl.DataFrame))
fake_payload = b"symbol,price\nMSFT,39.81\nAAPL,28.13\n"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1: The unittest.mock.patch("urllib.request.urlopen", ...) in test_get_dataframe_csv() does not intercept the actual network call. When the chart data URL ends with .csv, the implementation in _get_dataframe_from_chart calls pl.read_csv(url) (or pd.read_csv(url)), both of which handle HTTP natively — they do not use urllib.request.urlopen. The mock is a no-op, so the test still makes a real HTTP request to the CDN.

This means the test (1) depends on external network availability, (2) will fail in airgapped or offline environments, and (3) defeats the PR's goal of mocking network calls.

To fix, either use a local file path or CSV string instead of a URL, or patch polars.read_csv / pandas.read_csv directly depending on which library the test environment provides.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/_plugins/ui/_impl/test_altair_chart.py, line 780:

<comment>The `unittest.mock.patch("urllib.request.urlopen", ...)` in `test_get_dataframe_csv()` does not intercept the actual network call. When the chart data URL ends with `.csv`, the implementation in `_get_dataframe_from_chart` calls `pl.read_csv(url)` (or `pd.read_csv(url)`), both of which handle HTTP natively — they do **not** use `urllib.request.urlopen`. The mock is a no-op, so the test still makes a real HTTP request to the CDN.

This means the test (1) depends on external network availability, (2) will fail in airgapped or offline environments, and (3) defeats the PR's goal of mocking network calls.

To fix, either use a local file path or CSV string instead of a URL, or patch `polars.read_csv` / `pandas.read_csv` directly depending on which library the test environment provides.</comment>

<file context>
@@ -777,8 +777,13 @@ def test_get_dataframe_csv() -> None:
     data = "https://cdn.jsdelivr.net/npm/vega-datasets@v1.29.0/data/stocks.csv"
-    chart = altair_chart(alt.Chart(data).mark_point().encode(x="values:Q"))
-    assert isinstance(chart.dataframe, (pd.DataFrame, pl.DataFrame))
+    fake_payload = b"symbol,price\nMSFT,39.81\nAAPL,28.13\n"
+    with unittest.mock.patch(
+        "urllib.request.urlopen",
</file context>

@mscolnick mscolnick added the internal A refactor or improvement that is not user facing label Jul 16, 2026
@mscolnick
mscolnick merged commit 8373854 into main Jul 16, 2026
39 of 41 checks passed
@mscolnick
mscolnick deleted the ms/fix/mock-altair-chart-tests branch July 16, 2026 16:25
@github-actions

Copy link
Copy Markdown
Contributor

🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.23.15-dev24

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal A refactor or improvement that is not user facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants