fix(workbench): disambiguate IDE session names under xdist#220
fix(workbench): disambiguate IDE session names under xdist#220ian-flores merged 2 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Improves reliability of Workbench IDE-launch E2E tests under pytest-xdist by making session names unambiguous and tightening Playwright selectors to avoid strict-mode collisions when multiple sessions have similar names.
Changes:
- Generate IDE session names using
PYTEST_XDIST_WORKER+time.time_ns()to avoid timestamp collisions. - Update Workbench homepage session selectors to use ends-with (
$=) matching onaria-labelrather than substring (*=). - Expand selector documentation to explain the
aria-labelformat and why$=is used.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/vip_tests/workbench/test_ide_launch.py |
Makes IDE-launch session names unique across workers/time to prevent duplicate session rows. |
src/vip_tests/workbench/pages/homepage.py |
Tightens session-row / status / rename selectors to avoid substring-based collisions under strict Playwright locators. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def session_rename_button(name: str) -> str: | ||
| """Selector for session rename button.""" | ||
| return f"[aria-label*='{name}'] button[aria-label='Rename session']" | ||
| return f"[aria-label$='{name}'] button[aria-label='Rename session']" |
There was a problem hiding this comment.
session_rename_button uses [aria-label$='{name}'] ... without constraining the element type, while the related helpers (session_row, session_row_status) explicitly scope to tr[aria-label$=...]. For consistency and to reduce the chance of accidentally matching a non-row element if the page adds additional aria-labels ending with the session name, consider scoping this selector to the session row (e.g. build it from session_row(name) or prefix with tr).
| return f"[aria-label$='{name}'] button[aria-label='Rename session']" | |
| return f"tr[aria-label$='{name}'] button[aria-label='Rename session']" |
There was a problem hiding this comment.
Good call — scoped to tr in 5832382 for consistency with session_row and session_row_status.
| worker = os.environ.get("PYTEST_XDIST_WORKER", "main") | ||
| session_name = f"VIP {_FILENAME} - {worker}-{time.time_ns()}" | ||
| session_context["name"] = session_name | ||
| session_context["ide_type"] = ide_name | ||
| _start_session(page, ide_name, session_name) |
There was a problem hiding this comment.
_start_ide_session now generates a unique name for IDE-launch tests, but other Workbench tests still generate session_name with int(time.time()) (e.g. test_data_sources.py, test_packages.py, test_sessions.py). With xdist enabled by default, second-precision names can still collide across tests (or within the same worker if two sessions are started in the same second), and the tightened $= locators will make such collisions more likely to surface as strict-mode failures. Consider extracting a shared helper (e.g. in workbench/conftest.py) for unique session name generation and using it across Workbench tests to avoid repeating this class of bug.
There was a problem hiding this comment.
Done in 5832382 — extracted unique_session_name(filename) to workbench/conftest.py and migrated test_ide_launch, test_packages, test_sessions, and test_data_sources to use it. Removes four independent int(time.time()) callsites that would otherwise silently regress once the $= locators are in place.
📸 Preview Screenshots — PR #22010 screenshots captured across 8 website pages and 2 report pages.
🌐 Website PreviewWebsite Home
Website Getting Started
Website Shiny App
Website Tests
Website Feature Matrix
Website Report
Website Example Report
Website Example Report Details
📊 Report PreviewReport Home
Report Details
Screenshots captured automatically by the preview screenshot workflow.
|
|
📸 PR Preview Screenshots — PR #220Preview URLs captured:
Total screenshots uploaded: 10 (9 successful pages + 1 failed page captured as 404) 🌐 Website PreviewHome
Getting Started
Shiny App
Tests
Feature Matrix
Report Page
Example Report (full-page)
Example Report — Details
📊 Report PreviewReport Index (full-page)
|
| URL | Reason |
|---|---|
https://posit-dev.github.io/vip/pr-preview/pr-220/details.html |
HTTP 404 — details page not present in this report preview |
https://posit-dev.github.io/vip/pr-preview-site/pr-220/example-report/details.html/ |
HTTP 404 — trailing slash added to .html path (link navigation artifact) |
i️ The
details.htmllink was followed during crawl but returned 404 for the report preview. This is expected if the report only contains an index page in this build. The website preview'sdetails.htmlwas captured successfully without trailing slash.
Generated by Capture preview screenshots for PRs · ● 3M · ◷
Summary
_start_ide_sessionusedint(time.time()), producing identical values across xdist workers that enter within the same second. Two concurrent tests then requested the same session row viatr[aria-label*='…'], hitting Playwright strict-mode: "resolved to N elements."unique_session_name(filename)intosrc/vip_tests/workbench/conftest.py(worker id +time.time_ns()), and migrate all four Workbench tests that create sessions by name:test_ide_launch.py,test_packages.py,test_sessions.py,test_data_sources.py.Homepage(session_row,session_rename_button,session_row_status) from substring match (*=) to ends-with ($=), scoped totr[…]for consistency. Workbench setsaria-labelto"No project: <session_name>", so ends-with anchors cleanly on the session name and prevents collisions when one name is a substring of another.Test plan
uv run ruff check src/ selftests/uv run ruff format --check src/ selftests/uv run pytest selftests/ -q— 317 passed (occasionaltest_load_engine::test_1k_userstiming flake is sandbox-side and unrelated)"No project: VIP test_ide_launch.py - 1776873251"— confirms ends-with match is correct.vip verify --workbench-url https://dev.ganso.lab.staging.posit.team --interactive-auth --categories workbench -- -k "test_launch_rstudio or test_launch_vscode"under default parallel xdist (12 workers) — 2 passed in 37.99s, no strict-mode violations.test_launch_jupyter or test_launch_positron— tests progressed past session creation and row detection, then auto-skipped at the IDE-content step (JupyterLab/Positron not installed on ganso). Clean environmental skips confirm the session row resolves to a unique match under parallel xdist; a broken locator would have raised strict-mode errors before reaching the IDE iframe wait.Fixes #215