Skip to content

Make tests more resilient #124

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

Merged
lfoppiano merged 6 commits into from
Mar 21, 2026
Merged

Make tests more resilient #124

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
50f7377
feat: make tests work more consistently
lfoppiano Mar 20, 2026
220aa2f
fix: improve visibility detection
lfoppiano Mar 20, 2026
857a464
fix: rendering
lfoppiano Mar 21, 2026
547a1ed
fix: make test more meaningful, try to make them work
lfoppiano Mar 21, 2026
31dcc55
chore: update CI
lfoppiano Mar 21, 2026
b37859f
fix: failing test
lfoppiano Mar 21, 2026
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
15 changes: 15 additions & 0 deletions streamlit_pdf_viewer/frontend/src/PdfViewer.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export default {
const pdfContainer = ref(null);
const manualZoomInput = ref(100);
const isRendering = ref(false);
const wasVisible = ref(false);

const renderText = props.args.render_text === true;
const allowClickableAnnotationsWithTextRendering = props.args.allow_clickable_annotations_with_text_rendering === true;
Expand Down Expand Up @@ -464,15 +465,29 @@ export default {

const debouncedHandleResize = debounce(handleResize, 200);

const observer = new IntersectionObserver((entries) => {
const entry = entries[0];
if (entry.isIntersecting && !wasVisible.value) {
wasVisible.value = true;
debouncedHandleResize();
} else if (!entry.isIntersecting) {
wasVisible.value = false;
}
});

@coderabbitai coderabbitai Bot Mar 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether IntersectionObserver is already polyfilled/mocked in this repo.
rg -n -C2 'IntersectionObserver|mock.*IntersectionObserver|polyfill' -g '!**/node_modules/**'

Repository: lfoppiano/streamlit-pdf-viewer

Length of output: 545

🏁 Script executed:

#!/bin/bash
# Check package.json for dependencies and browser targets
head -50 package.json

# Check for browserslist or similar browser target config
fd -e '.browserslistrc|browserslist|package.json' . --max-depth 2

# Search for test setup that might use JSDOM or similar
rg -l 'jest|vitest|mocha|testEnvironment' --max-count 5

Repository: lfoppiano/streamlit-pdf-viewer

Length of output: 145

🏁 Script executed:

#!/bin/bash
# Find package.json in the frontend directory
find . -name "package.json" -type f | head -5

# Look for test configuration files
find . -name "vitest.config.*" -o -name "jest.config.*" -o -name "vite.config.*" | head -10

# Check if tests exist and what environment they use
rg -l 'describe|it\(|test\(' --type javascript --type typescript -g '**/*.test.*|**/*.spec.*' | head -5

Repository: lfoppiano/streamlit-pdf-viewer

Length of output: 170

🏁 Script executed:

#!/bin/bash
# Read the frontend package.json
cat ./streamlit_pdf_viewer/frontend/package.json

# Look for vitest/jest config in frontend
find ./streamlit_pdf_viewer/frontend -maxdepth 2 -name "vitest.config.*" -o -name "jest.config.*" -o -name "vite.config.*"

# Check for test files
find ./streamlit_pdf_viewer/frontend -name "*.test.*" -o -name "*.spec.*" | head -5

Repository: lfoppiano/streamlit-pdf-viewer

Length of output: 865

🏁 Script executed:

#!/bin/bash
# Check the main entry point for core-js imports
find ./streamlit_pdf_viewer/frontend/src -name "main.*" -o -name "index.*" | head -5

# Search for core-js imports
rg 'core-js|import.*polyfill' ./streamlit_pdf_viewer/frontend/src

# Check if IntersectionObserver is used elsewhere in the codebase with guards
rg -B3 -A3 'IntersectionObserver' ./streamlit_pdf_viewer/frontend/src

# Look for any browser support documentation
find ./streamlit_pdf_viewer -maxdepth 3 -name "README*" -o -name "BROWSER*" | xargs cat 2>/dev/null | head -100

Repository: lfoppiano/streamlit-pdf-viewer

Length of output: 838

🏁 Script executed:

#!/bin/bash
# Read main.ts to check for polyfill setup
cat ./streamlit_pdf_viewer/frontend/src/main.ts

Repository: lfoppiano/streamlit-pdf-viewer

Length of output: 163

Guard IntersectionObserver initialization to prevent runtime failures in unsupported environments.

Line 468 creates new IntersectionObserver(...) unconditionally. This will crash at component setup time in environments lacking the API (older browsers, server-side rendering, or test runners using JSDOM). Add a feature check before instantiation.

🔧 Proposed fix 
-    const observer = new IntersectionObserver((entries) => {
-      const entry = entries[0];
-      if (entry.isIntersecting && !wasVisible.value) {
-        wasVisible.value = true;
-        debouncedHandleResize();
-      } else if (!entry.isIntersecting) {
-        wasVisible.value = false;
-      }
-    });
+    let observer = null;
+    if (typeof window !== "undefined" && "IntersectionObserver" in window) {
+      observer = new IntersectionObserver((entries) => {
+        const entry = entries[0];
+        if (entry.isIntersecting && !wasVisible.value) {
+          wasVisible.value = true;
+          debouncedHandleResize();
+        } else if (!entry.isIntersecting) {
+          wasVisible.value = false;
+        }
+      });
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const observer = new IntersectionObserver((entries) => {
const entry = entries[0];
if (entry.isIntersecting && !wasVisible.value) {
wasVisible.value = true;
debouncedHandleResize();
} else if (!entry.isIntersecting) {
wasVisible.value = false;
}
});
let observer = null;
if (typeof window !== "undefined" && "IntersectionObserver" in window) {
observer = new IntersectionObserver((entries) => {
const entry = entries[0];
if (entry.isIntersecting && !wasVisible.value) {
wasVisible.value = true;
debouncedHandleResize();
} else if (!entry.isIntersecting) {
wasVisible.value = false;
}
});
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@streamlit_pdf_viewer/frontend/src/PdfViewer.vue` around lines 468 - 476, The
IntersectionObserver is created unconditionally which can crash in environments
that don't support it; before instantiating the const observer = new
IntersectionObserver(...), add a feature check for window?.IntersectionObserver
(or typeof IntersectionObserver !== 'undefined') and only create and use
observer when available—otherwise ensure wasVisible/value and any teardown logic
(e.g., observer.disconnect) gracefully skip usage; keep references to
wasVisible.value and debouncedHandleResize inside the guarded block so their
calls only occur when IntersectionObserver exists.


onMounted(() => {
debouncedHandleResize();
window.addEventListener("resize", debouncedHandleResize);
document.addEventListener('click', handleClickOutside);
if (pdfContainer.value) {
observer.observe(pdfContainer.value);
}
});

onUnmounted(() => {
window.removeEventListener("resize", debouncedHandleResize);
document.removeEventListener('click', handleClickOutside);
observer.disconnect();
});

return {
Expand Down
6 changes: 3 additions & 3 deletions tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import pytest
from pathlib import Path
from typing import Generator, Any, Dict
from playwright.sync_api import Browser, BrowserContext, Page
from playwright.sync_api import Browser, BrowserContext, Page, expect

from tests import ROOT_DIRECTORY
from tests.e2e_utils import StreamlitRunner
Expand Down Expand Up @@ -143,7 +143,7 @@ def default_go_to_app(page: Page, default_streamlit_app: StreamlitRunner):
"""Navigate to the default streamlit app and wait for it to load."""
page.goto(default_streamlit_app.server_url)
# Wait for app to load
page.get_by_role("img", name="Running...").is_hidden()
expect(page.get_by_role("img", name="Running...")).not_to_be_visible()


@pytest.fixture(autouse=True, scope="module")
Expand All @@ -164,4 +164,4 @@ def go_to_app(page: Page, streamlit_app: StreamlitRunner):
"""
page.goto(streamlit_app.server_url)
# Wait for app to load
page.get_by_role("img", name="Running...").is_hidden()
expect(page.get_by_role("img", name="Running...")).not_to_be_visible()
2 changes: 1 addition & 1 deletion tests/e2e_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ def is_server_running(self, timeout: int = 30) -> bool:
if response.text == "ok":
return True
time.sleep(3)
if time.time() - start_time > 60 * timeout:
if time.time() - start_time > timeout:
return False

@property
Expand Down
8 changes: 3 additions & 5 deletions tests/test_accessibility.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,9 @@ def test_accessibility_high_contrast_mode(page: Page):
}
""")

page.wait_for_timeout(1000)

iframe_components = page.locator('iframe[title="streamlit_pdf_viewer.streamlit_pdf_viewer"]')
expect(iframe_components).to_have_count(1)

# Check that the viewer is still visible and functional in high contrast
iframe_frame = page.frame_locator('iframe[title="streamlit_pdf_viewer.streamlit_pdf_viewer"]').nth(0)
pdf_viewer = iframe_frame.locator('div[id="pdfViewer"]')
Expand All @@ -109,8 +107,8 @@ def test_accessibility_responsive_text_sizing(page: Page):

for viewport in viewports:
page.set_viewport_size(viewport)
page.wait_for_timeout(1000)
page.wait_for_timeout(500)

iframe_components = page.locator('iframe[title="streamlit_pdf_viewer.streamlit_pdf_viewer"]')
expect(iframe_components).to_have_count(1)

Expand Down
2 changes: 1 addition & 1 deletion tests/test_alignment_center.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def streamlit_app():
def go_to_app(page: Page, streamlit_app: StreamlitRunner):
page.goto(streamlit_app.server_url)
# Wait for app to load
page.get_by_role("img", name="Running...").is_hidden()
expect(page.get_by_role("img", name="Running...")).not_to_be_visible()


def test_should_render_with_center_alignment(page: Page):
Expand Down
2 changes: 1 addition & 1 deletion tests/test_alignment_left.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def streamlit_app():
def go_to_app(page: Page, streamlit_app: StreamlitRunner):
page.goto(streamlit_app.server_url)
# Wait for app to load
page.get_by_role("img", name="Running...").is_hidden()
expect(page.get_by_role("img", name="Running...")).not_to_be_visible()


def test_should_render_with_left_alignment(page: Page):
Expand Down
2 changes: 1 addition & 1 deletion tests/test_alignment_right.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def streamlit_app():
def go_to_app(page: Page, streamlit_app: StreamlitRunner):
page.goto(streamlit_app.server_url)
# Wait for app to load
page.get_by_role("img", name="Running...").is_hidden()
expect(page.get_by_role("img", name="Running...")).not_to_be_visible()


def test_should_render_with_right_alignment(page: Page):
Expand Down
3 changes: 0 additions & 3 deletions tests/test_browser_compatibility.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ def test_basic_pdf_viewer_compatibility(page: Page):
@pytest.mark.compatibility
def test_pdf_viewer_renders_correctly(page: Page):
"""Test that PDF viewer renders correctly across browsers."""
# Wait for content to load
page.wait_for_timeout(3000)

# Check that the PDF viewer is present
iframe_components = page.locator('iframe[title="streamlit_pdf_viewer.streamlit_pdf_viewer"]')
expect(iframe_components).to_have_count(1)
Expand Down
4 changes: 2 additions & 2 deletions tests/test_edge_cases.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@ def test_edge_case_viewport_resize(page: Page):

for viewport in viewports:
page.set_viewport_size(viewport)
page.wait_for_timeout(1000)
page.wait_for_timeout(500)

# Check that all viewers remain functional
for i in range(1):
iframe_frame = page.frame_locator('iframe[title="streamlit_pdf_viewer.streamlit_pdf_viewer"]').nth(i)
Expand Down
54 changes: 0 additions & 54 deletions tests/test_error_handling.py
View file
Open in desktop

This file was deleted.

85 changes: 3 additions & 82 deletions tests/test_interactive_features.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,94 +6,15 @@
def test_zoom_controls_visibility(page: Page):
"""Test that zoom controls are visible and functional."""
expect(page.get_by_text("Test PDF Viewer with auto zoom (fit to width)")).to_be_visible()

# Check that all three PDF viewers are present
iframe_components = page.locator('iframe[title="streamlit_pdf_viewer.streamlit_pdf_viewer"]')
expect(iframe_components).to_have_count(1)
Comment on lines 10 to 12

@coderabbitai coderabbitai Bot Mar 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Update stale inline comment about iframe count.

Line 10 says “all three PDF viewers,” but Line 12 asserts a count of 1. Please align the comment with the actual expectation to avoid confusion during future maintenance.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/test_interactive_features.py` around lines 10 - 12, Update the
misleading inline comment to match the actual assertion: change the comment
referencing “all three PDF viewers” to reflect that the test expects a single
PDF viewer. Locate the test variable iframe_components and the locator
'iframe[title="streamlit_pdf_viewer.streamlit_pdf_viewer"]' in
tests/test_interactive_features.py and revise the comment above that assertion
so it accurately describes expect(iframe_components).to_have_count(1).


# Test the first viewer (with zoom controls)
iframe_frame = page.frame_locator('iframe[title="streamlit_pdf_viewer.streamlit_pdf_viewer"]').nth(0)

# Check for zoom controls
zoom_controls = iframe_frame.locator('button.zoom-button, .zoom-controls, [class*="zoom"]')
# Note: The exact selector depends on your frontend implementation
# This is a flexible approach that looks for common zoom control patterns
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated


@pytest.mark.skip(reason="Test expects 3 PDF viewers with different zoom levels (1.0, 2.0, 0.5) but the test app example_zoom_auto.py only contains 1 PDF viewer with auto zoom. Test app content doesn't match test expectations.")
@pytest.mark.interactive
def test_different_zoom_levels_render(page: Page):
"""Test that different zoom levels render correctly."""
iframe_components = page.locator('iframe[title="streamlit_pdf_viewer.streamlit_pdf_viewer"]')

# Test each viewer with different zoom levels
for i in range(3):
iframe_frame = page.frame_locator('iframe[title="streamlit_pdf_viewer.streamlit_pdf_viewer"]').nth(i)

# Check that each PDF container is visible
pdf_container = iframe_frame.locator('div[id="pdfContainer"]')
expect(pdf_container).to_be_visible()

# Check that each PDF viewer has content
pdf_viewer = iframe_frame.locator('div[id="pdfViewer"]')
expect(pdf_viewer).to_be_visible()

# Check for canvas elements (rendered PDF content)
canvas_elements = pdf_viewer.locator("canvas")
expect(canvas_elements).to_have_count(1) # At least one canvas per viewer


@pytest.mark.skip(reason="Test expects 3 PDF viewers with different zoom levels (1.0, 2.0, 0.5) to compare canvas sizes, but the test app example_zoom_auto.py only contains 1 PDF viewer with auto zoom. Cannot test zoom level differences without multiple viewers.")
@pytest.mark.interactive
def test_zoom_level_differences(page: Page):
"""Test that different zoom levels produce visually different results."""
iframe_components = page.locator('iframe[title="streamlit_pdf_viewer.streamlit_pdf_viewer"]')

# Get the first viewer (zoom level 1.0)
iframe_frame_1 = page.frame_locator('iframe[title="streamlit_pdf_viewer.streamlit_pdf_viewer"]').nth(0)
canvas_1 = iframe_frame_1.locator("canvas").first
canvas_1_box = canvas_1.bounding_box()

# Get the second viewer (zoom level 2.0)
iframe_frame_2 = page.frame_locator('iframe[title="streamlit_pdf_viewer.streamlit_pdf_viewer"]').nth(1)
canvas_2 = iframe_frame_2.locator("canvas").first
canvas_2_box = canvas_2.bounding_box()

# Get the third viewer (zoom level 0.5)
iframe_frame_3 = page.frame_locator('iframe[title="streamlit_pdf_viewer.streamlit_pdf_viewer"]').nth(2)
canvas_3 = iframe_frame_3.locator("canvas").first
canvas_3_box = canvas_3.bounding_box()

# The canvases should have different sizes due to different zoom levels
# High zoom (2.0) should be larger than normal zoom (1.0)
# Low zoom (0.5) should be smaller than normal zoom (1.0)
assert canvas_2_box['width'] > canvas_1_box['width'], "High zoom should produce larger canvas"
assert canvas_3_box['width'] < canvas_1_box['width'], "Low zoom should produce smaller canvas"


@pytest.mark.skip(reason="Test expects 3 PDF viewers to test responsiveness, but the test app example_zoom_auto.py only contains 1 PDF viewer. Cannot test multiple viewer responsiveness with single viewer.")
@pytest.mark.interactive
def test_interactive_features_responsiveness(page: Page):
"""Test that interactive features remain responsive."""
# Wait for all content to load
page.wait_for_timeout(3000)

iframe_components = page.locator('iframe[title="streamlit_pdf_viewer.streamlit_pdf_viewer"]')
expect(iframe_components).to_have_count(1)

# Test that all viewers are interactive
for i in range(3):
iframe_frame = page.frame_locator('iframe[title="streamlit_pdf_viewer.streamlit_pdf_viewer"]').nth(i)

# Check that the PDF viewer is interactive (has content)
pdf_viewer = iframe_frame.locator('div[id="pdfViewer"]')
expect(pdf_viewer).to_be_visible()

# Check for canvas elements that indicate rendered content
canvas = pdf_viewer.locator("canvas").first
expect(canvas).to_be_visible()

# Verify the canvas has reasonable dimensions
canvas_box = canvas.bounding_box()
assert canvas_box['width'] > 0, f"Canvas {i} should have positive width"
assert canvas_box['height'] > 0, f"Canvas {i} should have positive height"
2 changes: 1 addition & 1 deletion tests/test_invalid_params.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def streamlit_app():
def go_to_app(page: Page, streamlit_app: StreamlitRunner):
page.goto(streamlit_app.server_url)
# Wait for app to load
page.get_by_role("img", name="Running...").is_hidden()
expect(page.get_by_role("img", name="Running...")).not_to_be_visible()


def test_should_fail_rendering(page: Page):
Expand Down
84 changes: 0 additions & 84 deletions tests/test_multiple_instances.py
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion tests/test_page_separator_disabled.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def streamlit_app():
def go_to_app(page: Page, streamlit_app: StreamlitRunner):
page.goto(streamlit_app.server_url)
# Wait for app to load
page.get_by_role("img", name="Running...").is_hidden()
expect(page.get_by_role("img", name="Running...")).not_to_be_visible()


def test_should_render_with_page_separators_disabled(page: Page):
Expand Down
Loading
Loading