fix(workbench): skip IDE tests gracefully when IDE is unavailable#208
fix(workbench): skip IDE tests gracefully when IDE is unavailable#208ian-flores merged 4 commits intomainfrom
Conversation
- Detect the case where the IDE tab is present in the New Session dialog but the Launch button never appears (IDE not installed/functional), and pytest.skip before the subsequent click times out 30s on #modalCancelBtn. - Cancel-button click now uses TIMEOUT_QUICK with a try/except so a missing cancel button no longer masks the real skip reason. - positron_console_accessible now uses the existing _expect_ide_or_skip helper so a missing `.positron-console` element skips instead of hard-failing — matches the pattern used for the Positron workbench. Fixes #186 Fixes #187
There was a problem hiding this comment.
Pull request overview
Improves Workbench IDE-launch Playwright tests by converting common “IDE unavailable / selector drift” failures into clear pytest.skip outcomes instead of hard timeouts.
Changes:
- Add a fast post-tab-check in session launch to skip when the IDE tab opens but the Launch button never appears.
- Introduce a best-effort helper to dismiss the “New Session” dialog before skipping.
- Switch Positron console visibility assertion to use the shared “expect-or-skip” helper pattern.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cancel = page.locator(NewSessionDialog.CANCEL_BUTTON) | ||
| if cancel.count() > 0: | ||
| try: | ||
| cancel.click(timeout=TIMEOUT_QUICK) | ||
| except PlaywrightTimeoutError: | ||
| pass |
There was a problem hiding this comment.
_dismiss_dialog_and_skip is described as “best-effort”, but it only suppresses PlaywrightTimeoutError from cancel.click(). If the cancel button exists but is detached/covered/not actionable, Playwright can raise a generic playwright.sync_api.Error (or other exceptions), which would still fail the test and mask the intended pytest.skip. Consider catching playwright.sync_api.Error (or even a broad Exception) here, since the whole point is to avoid any cancel-click failure hiding the skip reason.
| selector never appears, this may be an older Positron version or the | ||
| DOM structure may have changed — skip rather than hard-fail, matching | ||
| the ``_expect_ide_or_skip`` pattern used for the IDE workbench itself. | ||
| """ | ||
| console_panel = page.locator(PositronSession.CONSOLE_PANEL) | ||
| expect(console_panel).to_be_visible(timeout=TIMEOUT_CODE_EXEC) | ||
| _expect_ide_or_skip(page, PositronSession.CONSOLE_PANEL, "Positron console") |
There was a problem hiding this comment.
positron_console_accessible now calls _expect_ide_or_skip, which waits up to TIMEOUT_IDE_LOAD (60s). Previously this step used TIMEOUT_CODE_EXEC (30s), so a missing/drifted console selector will take twice as long to skip. Also, _expect_ide_or_skip’s skip message implies “the IDE may not be installed”, which doesn’t match this docstring’s stated causes (older Positron / DOM drift). Consider extending _expect_ide_or_skip to accept a timeout and custom reason/message (or add a separate helper for optional UI panels) and use TIMEOUT_CODE_EXEC + a console-specific skip reason here.
- Annotate _dismiss_dialog_and_skip as -> NoReturn to make control-flow guarantee explicit to type checkers and readers - Document intentional timeout change in positron_console_accessible: now uses TIMEOUT_IDE_LOAD (60s) vs previous TIMEOUT_CODE_EXEC (30s), which is deliberately more generous for slow deployments
📸 Preview Screenshots – PR #208
Total screenshots uploaded: 9 (2 failed) 🌐 Website PreviewWebsite home page Getting Started Shiny App Test Inventory Feature Matrix Report page Example Report (viewport preview) 📊 Validation Report PreviewValidation Report home (viewport preview) Validation Report details
|
|
Preview Screenshots for PR #208Triggering workflow: Website Preview (run 24789594756) — conclusion:
Summary
Captured URLs
ScreenshotsBoth preview URLs returned the GitHub 404 page (shown below). The previews were cleaned up when the PR was merged. Website preview (
Report preview (
Screenshots captured by the preview-screenshot-gallery workflow.
|
Summary
Two IDE-launch tests hard-failed with cryptic Playwright timeouts when the IDE wasn't installed or the DOM selector drifted. Both now convert those cases into a clean
pytest.skipwith a descriptive reason.test_launch_jupytertimed out 30s waiting for#modalCancelBtnwhen Jupyter was not actually available: the tab was present but clicking it showed an error state without a Launch button. Added a shortwait_for(state="visible")on the Launch button after the tab click; onPlaywrightTimeoutErrorwe dismiss the dialog and skip with a clear message. Also hardened the existing "tab absent" branch so a missing cancel button no longer triggers the 30-second default timeout.test_launch_positronhard-failed on.positron-consolevisibility when the selector had drifted or Positron was not available.positron_console_accessiblenow uses the existing_expect_ide_or_skiphelper (same pattern used for the Positron workbench element).Test plan
just checkpassesuv run pytest selftests/passes (303/303 locally)https://dev.ganso.lab.staging.posit.team, OIDC interactive auth):test_launch_positron— skipped with my new message and parameterized 30s timeout: "Positron console element not found within timeout — selector may have changed or Positron may not be fully available (Locator.wait_for: Timeout 30000ms exceeded ... waiting for locator('.positron-console'))". Directly validates the_expect_ide_or_skipparameterization added in response to Copilot's feedback.test_launch_jupyter— skipped via the pre-existing_expect_ide_or_skippath on.jp-Launcher(Jupyter UI didn't load inside the session). My new "Launch button did not appear" guard was not exercised on this Workbench because the Launch button appeared normally; the guard is defensive code for a specific misconfiguration (tab present but no Launch button) that doesn't manifest on ganso01. Selftests + code review cover that path.Fixes #186
Fixes #187