feat: replace Selenium UI tests with WebDriverIO#2746
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates the repository’s UI test coverage from the existing Python/Selenium + code-server container approach to a WebDriverIO-based suite that drives real VS Code Electron, and updates CI/tooling/docs accordingly.
Changes:
- Added a WDIO test suite (
test/wdio/*.spec.ts) + runner configuration (wdio.conf.ts) and dependency extension preinstall script. - Removed the Selenium-based UI test suite and its Podman/docker-compose infrastructure and Python test-only deps/config.
- Updated CI, Taskfile, and docs to run
task wdiounder Xvfb and document the new workflow.
Reviewed changes
Copilot reviewed 43 out of 47 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| wdio.conf.ts | Adds WebDriverIO configuration for running tests against VS Code Electron. |
| uv.lock | Removes Selenium/pytest-cov/podman-compose-related Python lock entries; updates lock metadata. |
| tsconfig.json | Excludes WDIO test sources from the main TS project. |
| test/wdio/webviews.spec.ts | Adds WDIO specs covering devfile/devcontainer/welcome/LLM provider webviews. |
| test/wdio/tsconfig.json | Adds a dedicated TS config for WDIO tests. |
| test/wdio/terminal.spec.ts | Adds WDIO terminal-related integration checks (including adt --version). |
| test/wdio/smoke.spec.ts | Adds smoke coverage for VS Code launch, extension activation, commands, language detection. |
| test/wdio/sidebar.spec.ts | Adds sidebar navigation + settings propagation WDIO coverage. |
| test/wdio/mock-server.ts | Adds an Express mock server to stub Lightspeed API endpoints in tests. |
| test/wdio/mcp.spec.ts | Adds WDIO coverage for MCP server enable/disable + settings consistency. |
| test/wdio/lightspeed.spec.ts | Adds Lightspeed WDIO tests using the mock server + injected session. |
| test/wdio/fixtures/playbook.ansible.yml | Adds a fixture playbook used to trigger activation and drive tests. |
| test/wdio/fixtures/.vscode/settings.json | Adds workspace fixture settings for WDIO runs. |
| test/wdio/commands.spec.ts | Adds WDIO coverage for extension commands (e.g., create empty playbook). |
| test/ui/utils/ui_utils.py | Removes Selenium UI utility module. |
| test/ui/utils/settings_utils.py | Removes Selenium test settings utility module. |
| test/ui/utils/init.py | Removes Selenium UI utils package init. |
| test/ui/test_82_lightspeed_trial.py | Removes Selenium Lightspeed trial test. |
| test/ui/test_81_login.py | Removes Selenium login/SSO-related tests. |
| test/ui/test_80_lightspeed.py | Removes Selenium Lightspeed feature tests. |
| test/ui/test_50_mcp_server.py | Removes Selenium MCP server tests. |
| test/ui/test_03_llm_provider_webview.py | Removes Selenium LLM provider webview tests. |
| test/ui/test_02_welcome_webviews.py | Removes Selenium welcome webview tests. |
| test/ui/test_01_dev_webviews.py | Removes Selenium dev webview tests. |
| test/ui/test_00_commands.py | Removes Selenium command/terminal tests. |
| test/ui/hooks/logging_hook.py | Removes Selenium pytest hook for reporting/screenshots. |
| test/ui/hooks/init.py | Removes Selenium hooks package init. |
| test/ui/fixtures/ui_fixtures.py | Removes Selenium browser/container fixtures. |
| test/ui/fixtures/init.py | Removes Selenium fixtures package init. |
| test/ui/const.py | Removes Selenium container constants. |
| test/ui/conftest.py | Removes Selenium pytest configuration and session lifecycle management. |
| test/ui/init.py | Removes Selenium UI tests package init. |
| test/README.md | Updates test documentation to reference WDIO UI tests instead of Selenium. |
| src/features/lightspeed/lightSpeedOAuthProvider.ts | Adds setMockSession() helper to inject test auth sessions. |
| src/extension.ts | Registers a new ansible.lightspeed.mockSession command for WDIO session injection. |
| scripts/install-test-extensions.mjs | Adds a script to preinstall marketplace dependency extensions into the WDIO extensions dir. |
| pyproject.toml | Removes Selenium/pytest UI testing config/deps and updates codespell skip paths. |
| package.json | Adds WDIO + Express deps and introduces pretest:wdio / test:wdio scripts. |
| docs/development/test_code.md | Updates testing docs to describe WDIO UI tests. |
| docker-compose.yml | Removes the Selenium/code-server container compose definition. |
| Taskfile.yml | Replaces task ui with task wdio and updates the aggregated test task. |
| .gitignore | Ignores .wdio-vscode cache directory. |
| .github/workflows/ci.yaml | Updates CI to run task wdio and removes Podman setup steps. |
| .cursor/skills/wdio-testing/patterns.md | Adds WDIO testing patterns documentation for contributors. |
| .cursor/skills/wdio-testing/SKILL.md | Adds WDIO testing “skill” onboarding/architecture guidance. |
| .config/dictionary.txt | Adds “wdio” to the dictionary. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cidrblock
force-pushed
the
feat/wdio-ui-tests
branch
2 times, most recently
from
c050721 to
e45e6a5
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
cidrblock
force-pushed
the
feat/wdio-ui-tests
branch
from
e45e6a5 to
f53134b
Compare
This comment was marked as outdated.
This comment was marked as outdated.
cidrblock
force-pushed
the
feat/wdio-ui-tests
branch
from
f53134b to
578d808
Compare
cidrblock
force-pushed
the
feat/wdio-ui-tests
branch
from
578d808 to
8263f4a
Compare
cidrblock
force-pushed
the
feat/wdio-ui-tests
branch
from
8263f4a to
6c6655b
Compare
cidrblock
force-pushed
the
feat/wdio-ui-tests
branch
from
6c6655b to
7ad4621
Compare
cidrblock
added a commit
to cidrblock/vscode-ansible-1
that referenced
this pull request
Apr 17, 2026
Multiple didChangeConfiguration notifications arriving in rapid succession each cleared _docsLibrary while a previous initialize() was in flight, orphaning the result and leaving hover requests with an empty module index. Add a 500ms debounce to the onConfigurationChanged callback so that rapid-fire config changes coalesce into a single service invalidation cycle. related: ansible#2746, ansible#2749 Made-with: Cursor
2 tasks
cidrblock
added a commit
to cidrblock/vscode-ansible-1
that referenced
this pull request
Apr 17, 2026
Replace the Python/Selenium container-based UI test suite with TypeScript WebDriverIO tests using wdio-vscode-service, running against real VS Code Electron. Adds dedicated test (linux-wdio) CI runner. related: ansible#2746 Made-with: Cursor
cidrblock
force-pushed
the
feat/wdio-ui-tests
branch
from
998b46b to
5b136f6
Compare
cidrblock
added a commit
to cidrblock/vscode-ansible-1
that referenced
this pull request
Apr 17, 2026
Multiple didChangeConfiguration notifications arriving in rapid succession each cleared _docsLibrary while a previous initialize() was in flight, orphaning the result and leaving hover requests with an empty module index. Add a 500ms debounce to the onConfigurationChanged callback so that rapid-fire config changes coalesce into a single service invalidation cycle. related: ansible#2746, ansible#2749 Made-with: Cursor
cidrblock
added a commit
that referenced
this pull request
Apr 17, 2026
## Summary Multiple `didChangeConfiguration` notifications arriving in rapid succession (e.g. when several settings are updated at once) each cleared `_docsLibrary` while a previous `initialize()` was still in flight. This orphaned the initialization result and left subsequent hover requests with an empty module index. Add a 500ms debounce to the `onConfigurationChanged` callback so rapid-fire config changes coalesce into a single service invalidation cycle. ## Root cause The `docsLibraryReady` notification test in #2749 proved `initialize()` ran to completion, yet module hovers returned empty results. The sequence: 1. Three setting updates fire concurrently (`Promise.all`) 2. ALS receives three `didChangeConfiguration` notifications 3. Notification 1 clears `_docsLibrary`, triggering a new `initialize()` 4. Notification 2 clears `_docsLibrary` again while init is in flight 5. The orphaned init completes (sends `docsLibraryReady`) but its result is discarded 6. A fresh `initialize()` starts but finds a stale cache, producing an empty module index ## Test plan - [ ] `hover-with-EE` e2e tests pass - [ ] No regression in non-EE hover or completion related: #2746, #2749 Made with [Cursor](https://cursor.com)
cidrblock
added a commit
to cidrblock/vscode-ansible-1
that referenced
this pull request
Apr 17, 2026
Replace the Python/Selenium container-based UI test suite with TypeScript WebDriverIO tests using wdio-vscode-service, running against real VS Code Electron. Adds dedicated test (linux-wdio) CI runner. related: ansible#2746 Made-with: Cursor
cidrblock
force-pushed
the
feat/wdio-ui-tests
branch
from
5b136f6 to
5fbfc31
Compare
cidrblock
added a commit
to cidrblock/vscode-ansible-1
that referenced
this pull request
Apr 17, 2026
Replace the Python/Selenium container-based UI test suite with TypeScript WebDriverIO tests using wdio-vscode-service, running against real VS Code Electron. Adds dedicated test (linux-wdio) CI runner. related: ansible#2746 Made-with: Cursor
cidrblock
force-pushed
the
feat/wdio-ui-tests
branch
from
5fbfc31 to
e663de3
Compare
📝 WalkthroughWalkthroughReplaces Selenium/pytest UI tests and containerized test infra with WebdriverIO TypeScript tests against real VS Code Electron, adding WDIO configs, test suites, a mock Lightspeed server, installer script, and test docs; removes Selenium services, Python test fixtures/hooks, and related Docker compose/service entries. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as WDIO Test
participant WD as WebDriver<br/>Service
participant VSCode as VS Code<br/>Electron
participant Ext as Ansible<br/>Extension
participant API as Mock<br/>Lightspeed
Test->>WD: launch VS Code Electron (wdio-vscode)
WD->>VSCode: start instance with workspace & extensionsDir
Test->>VSCode: open fixture file
VSCode->>Ext: file detected -> extension activation
loop waitForExtensionActive
Test->>VSCode: executeWorkbench check extension.isActive
VSCode-->>Test: isActive status
end
Test->>VSCode: executeWorkbench ansible.lightspeed.mockSession(session)
VSCode->>Ext: setMockSession(payload)
Ext->>Ext: store synthetic session (secrets) and emit session change
Test->>API: setResponse / start / reset
Test->>Ext: trigger Lightspeed command (e.g., playbookExplanation)
Ext->>API: HTTP request to mock endpoint
API-->>Ext: stubbed response
Test->>VSCode: verify webview opened / commands result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (11)
eslint.config.mjs (1)
115-123: Consider narrowing thescripts/**ignore.
ignores: ["scripts/**"]completely disables ESLint for all current and future files underscripts/, which is broader than needed for the singlescripts/install-test-extensions.mjsadded in this PR. If feasible, prefer adjusting parser/project settings for that directory over a blanket ignore so future scripts still get baseline linting. Non-blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eslint.config.mjs` around lines 115 - 123, The current ESLint config disables linting for the whole scripts/ folder via the ignores entry; narrow it to only the added file by replacing ignores: ["scripts/**"] with ignores: ["scripts/install-test-extensions.mjs"] (or a more specific glob for just the test-extension installer), or alternatively remove the ignores entry and add a per-folder override that sets appropriate parser/project options for scripts (adjust the override block that currently contains ignores/files) so only that single script is exempt while other scripts continue to be linted; look for the override object containing "ignores" and "files" in eslint.config.mjs and update it accordingly.wdio.conf.ts (3)
24-28: Defaultansible.lightspeed.enabled: truemay trigger real network I/O in non-Lightspeed specs.Setting Lightspeed enabled globally means smoke/commands/terminal/sidebar specs will also activate Lightspeed code paths and potentially reach the real Lightspeed URL on startup (status bar, feedback webview, URI handler registration). Consider defaulting it to
falseand having onlytest/wdio/lightspeed.spec.tsflip it on viaworkbench.getSettingsEditor()or workspace settings before its tests run — keeps other specs hermetic and faster.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wdio.conf.ts` around lines 24 - 28, The default userSettings currently enables "ansible.lightspeed.enabled" which can cause real Lightspeed network activity in unrelated specs; change the default in wdio.conf.ts to "ansible.lightspeed.enabled": false and update the Lightspeed-specific test (test/wdio/lightspeed.spec.ts) to explicitly enable it at test startup via the settings editor or workspace settings (e.g., using workbench.getSettingsEditor() or the equivalent helper) so only that spec flips the flag for its scope.
53-56: Consider addingretriestomochaOptsfor Electron flakiness.VS Code Electron UI tests are notoriously flaky at startup/window focus; adding
retries: 1typically absorbs transient failures (e.g., UI not yet settled, slow extension activation) without hiding real regressions. Optional.mochaOpts: { ui: "bdd", timeout: 120000, + retries: 1, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wdio.conf.ts` around lines 53 - 56, The mochaOpts block in wdio.conf.ts lacks a retries setting which can help absorb transient Electron UI test flakiness; update the mochaOpts object (mochaOpts) to include retries: 1 (e.g., add the property retries: 1 alongside ui and timeout) so failing tests get a single automatic retry, keeping other settings like timeout unchanged.
22-23: AnchorextensionPath/workspacePathto the config file, notprocess.cwd().Using
process.cwd()makes the WDIO run brittle: invokingwdio run wdio.conf.tsfrom any subdirectory (or from a composite task with a different working directory) silently points the extensionPath at the wrong place, causing cryptic "extension not found" failures. Anchoring to the directory ofwdio.conf.tsis idiomatic and robust.♻️ Proposed refactor
-import path from "node:path"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; -const testRoot = path.resolve(process.cwd(), ".wdio-vscode"); +const rootDir = path.dirname(fileURLToPath(import.meta.url)); +const testRoot = path.resolve(rootDir, ".wdio-vscode"); const extensionsDir = path.join(testRoot, "extensions"); ... - extensionPath: path.resolve(process.cwd()), - workspacePath: path.resolve(process.cwd(), "test", "wdio", "fixtures"), + extensionPath: rootDir, + workspacePath: path.resolve(rootDir, "test", "wdio", "fixtures"),(If this file is loaded by
ts-nodein CommonJS mode, use__dirnameinstead offileURLToPath(import.meta.url).)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wdio.conf.ts` around lines 22 - 23, The paths for extensionPath and workspacePath are incorrectly anchored to process.cwd(), causing brittle resolution; change both to be resolved relative to the config file directory instead. Replace path.resolve(process.cwd()) with a directory derived from the config file (use __dirname when running under CommonJS or path.dirname(fileURLToPath(import.meta.url)) for ESM) and compute workspacePath as path.resolve(configDir, "test", "wdio", "fixtures"); update the references for extensionPath and workspacePath so they use that configDir variable instead of process.cwd().scripts/install-test-extensions.mjs (1)
34-41: PreferexecFileSyncwith an args array and batch all installs.Two recommended improvements:
- Use
execFileSyncwith an argv array to avoid shell quoting/escaping edge cases ifcliPath,extensionsDir, orprocess.cwd()ever contain spaces or special characters (more likely on Windows/macOS dev machines than CI).- Batch all extensions into one CLI invocation —
codeaccepts multiple--install-extensionflags and will install them together, shaving several seconds of Electron startup per extra extension.♻️ Proposed refactor
-import { execSync } from "node:child_process"; +import { execFileSync } from "node:child_process"; ... -for (const ext of DEPENDENCY_EXTENSIONS) { - console.log(`Installing: ${ext}`); - execSync( - `"${cliPath}" --install-extension ${ext} --force` + - ` --extensions-dir "${extensionsDir}"`, - { stdio: "inherit" }, - ); -} +const args = ["--force", "--extensions-dir", extensionsDir]; +for (const ext of DEPENDENCY_EXTENSIONS) { + args.push("--install-extension", ext); +} +console.log(`Installing: ${DEPENDENCY_EXTENSIONS.join(", ")}`); +execFileSync(cliPath, args, { stdio: "inherit" });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/install-test-extensions.mjs` around lines 34 - 41, The loop that invokes execSync per extension should be replaced with a single execFileSync call that passes cliPath as the executable and an argv array containing one --extensions-dir "<extensionsDir>" plus multiple --install-extension entries derived from DEPENDENCY_EXTENSIONS; update the code that currently iterates over DEPENDENCY_EXTENSIONS and calls execSync to instead build args = [...DEPENDENCY_EXTENSIONS.flatMap(ext => ["--install-extension", ext]), "--force", "--extensions-dir", extensionsDir] and call execFileSync(cliPath, args, { stdio: "inherit" }) so you avoid shell quoting issues and batch installs in one Electron startup.test/README.md (1)
66-78: LGTM on the WDIO doc refresh.One minor stylistic nit:
npx pnpm test:wdio -- --spec …is unusual —npx pnpm …re-downloads pnpm just to invoke it. If pnpm is already a prerequisite (per the "Installing Remaining Dependencies" section above),pnpm test:wdio -- --spec test/wdio/smoke.spec.tsis cleaner and faster. Same nit applies indocs/development/test_code.md.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/README.md` around lines 66 - 78, Replace the unusual invocation "npx pnpm test:wdio -- --spec test/wdio/smoke.spec.ts" with the simpler "pnpm test:wdio -- --spec test/wdio/smoke.spec.ts" in the UI Tests (WDIO) section and make the same change wherever the same command appears (notably the docs/development/test_code.md file); just drop the leading "npx" so pnpm is invoked directly (keep the rest of the command and spacing intact).Taskfile.yml (2)
235-236: Drop thenpxprefix beforepnpm.
pnpmis a hard requirement perpackage.jsonenginesand is installed before any task runs, sonpx pnpm …is redundant. Invokingpnpmdirectly avoids the extra Node resolution and an unnecessary npm registry hit if the local pnpm is not recognized.♻️ Proposed diff
- - "npx pnpm pretest:wdio" - - "{{.XVFB}}npx pnpm test:wdio" + - "pnpm pretest:wdio" + - "{{.XVFB}}pnpm test:wdio"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Taskfile.yml` around lines 235 - 236, Replace the redundant "npx pnpm ..." invocations in Taskfile.yml with direct "pnpm ..." calls: update the task commands containing the strings "npx pnpm pretest:wdio" and "{{.XVFB}}npx pnpm test:wdio" to remove the "npx " prefix so they read "pnpm pretest:wdio" and "{{.XVFB}}pnpm test:wdio", respectively.
227-228:wdiotask depends onpackage; consider depending onbuildinstead.WDIO loads the extension from
extensionPath: path.resolve(process.cwd())(seewdio.conf.ts), not from a.vsix. Depending on the fullpackagetask forcesvscepackaging plusknip/can-release.shon every WDIO run, which is strictly unnecessary and inflates the 5-minute estimate.build(orbuild:extension) is sufficient.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Taskfile.yml` around lines 227 - 228, wdio task incorrectly depends on the heavy package task; change Taskfile.yml so the wdio task's deps list uses the lighter build (or build:extension) task instead of package. Edit the Taskfile.yml entry for the wdio task (deps: - package) and replace the package dependency with build or build:extension so WDIO uses the local extension folder (wdio.conf.ts uses extensionPath: path.resolve(process.cwd())) and avoids running vsce/knip/can-release.sh on every test run.test/wdio/mcp.spec.ts (1)
107-115: Tautological branch.
assert.ok(sawNotification)in theelsearm can never fail —sawNotificationis alreadytruethere. The intent is presumably to assert that either the notification appeared or the setting is true; the current structure obscures that. Consider:♻️ Proposed diff
- const sawNotification = await notificationMentionsMcpEnabled(workbench); - if (!sawNotification) { - assert.strictEqual( - await readMcpServerEnabledAnsibleSection(), - true, - "Expected ansible.mcpServer.enabled to be true when no matching notification was found", - ); - } else { - assert.ok(sawNotification, "Expected an MCP enable notification"); - } - - assert.strictEqual(await readMcpServerEnabledMcpSection(), true); + const sawNotification = await notificationMentionsMcpEnabled(workbench); + const enabled = await readMcpServerEnabledAnsibleSection(); + assert.ok( + sawNotification || enabled, + "Expected either an MCP enable notification or ansible.mcpServer.enabled === true", + ); + assert.strictEqual(await readMcpServerEnabledMcpSection(), true);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/wdio/mcp.spec.ts` around lines 107 - 115, The current if/else around sawNotification is tautological; replace the branch with a single assertion that either sawNotification is true or readMcpServerEnabledAnsibleSection() returns true. Concretely, remove the if/else and assert that (sawNotification || await readMcpServerEnabledAnsibleSection() === true) using the existing assert utilities so the test verifies "notification appeared OR ansible.mcpServer.enabled is true" in one check; reference sawNotification and readMcpServerEnabledAnsibleSection() to locate the change.test/wdio/mock-server.ts (1)
174-176: Attacherrorlistener before callinglisten.If the port is already in use, Node emits
'error'on thenet.Serveron the next tick afterlisten(). Registering the listener afterapp.listen()works in practice but is racy with any synchronous error path and leaves the intent less clear. Register on the server beforelisten, or pass only the callback after wiring up the error handler:♻️ Proposed diff
- server = app.listen(port, () => resolve()); - server.once("error", reject); + const s = app.listen(port); + s.once("error", reject); + s.once("listening", () => resolve()); + server = s;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/wdio/mock-server.ts` around lines 174 - 176, The error listener is being attached after calling app.listen which is racy; instead obtain the Server from app.listen, attach server.once("error", reject) before waiting for it to start, and then wait for the "listening" event (server.once("listening", resolve)) or call resolve after wiring the listener so the sequence is: assign server from app.listen, attach server.once("error", reject), then attach server.once("listening", resolve) (referencing server, app.listen, reject, resolve).test/wdio/smoke.spec.ts (1)
31-41: Prefer resolving the fixture via the workspace folder instead ofprocess.cwd().Using
path.resolve(process.cwd(), "test", "wdio", "fixtures", "playbook.ansible.yml")assumes the runner is always invoked from the repo root. A futuretask wdiovariant, a PNPM workspace change, or a debugger launch config with a differentcwdwill silently feed a non-existent URI toopenTextDocument, and the failure surfaces only as a later "Ansible extension did not activate in time" timeout.The documented pattern (patterns.md Lines 21–27 / SKILL.md Lines 52–58) uses
vscode.workspace.workspaceFolders[0]withvscode.Uri.joinPath, which matches whateverwdio.conf.tsopens and keeps all specs consistent.🛠️ Proposed fix
-import path from "node:path"; - /** * Wait until the Ansible extension is active, triggering activation if needed. * ... */ @@ before(async function () { this.timeout(90_000); - const fixturePath = path.resolve( - process.cwd(), - "test", - "wdio", - "fixtures", - "playbook.ansible.yml", - ); - await browser.executeWorkbench(async (vscode, uri: string) => { - const doc = await vscode.workspace.openTextDocument(vscode.Uri.file(uri)); - await vscode.window.showTextDocument(doc); - }, fixturePath); + await browser.executeWorkbench(async (vscode, fixture: string) => { + const folder = vscode.workspace.workspaceFolders?.[0]; + if (!folder) throw new Error("WDIO workspace folder is missing"); + const uri = vscode.Uri.joinPath(folder.uri, fixture); + const doc = await vscode.workspace.openTextDocument(uri); + await vscode.window.showTextDocument(doc, { preview: false }); + }, "playbook.ansible.yml"); await waitForExtensionActive(); await browser.pause(3000); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/wdio/smoke.spec.ts` around lines 31 - 41, The test builds fixturePath using process.cwd(), which can be incorrect in different runner CWDs; change the spec to resolve the fixture using the opened workspace folder by retrieving vscode.workspace.workspaceFolders[0] inside the browser.executeWorkbench callback and constructing the URI with vscode.Uri.joinPath (so replace the external fixturePath variable usage and instead pass the workspace-based URI into openTextDocument/openTextDocument(vscode.Uri.joinPath(...)) inside the existing browser.executeWorkbench call), ensuring the same workspace folder that wdio.conf.ts opens is used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yaml:
- Around line 424-428: The workflow is incorrectly referencing
matrix.env.TASKFILE_ARGS (which is undefined) when the TASKFILE_ARGS variable
lives in the job-level env; change all uses of matrix.env.TASKFILE_ARGS (e.g.,
the run step calling task package ${{ matrix.env.TASKFILE_ARGS }}) to reference
the job/env variable instead (use ${{ env.TASKFILE_ARGS }} or directly
TASKFILE_ARGS in the run context), updating every occurrence (including the
similar lines noted) so the second `task package … --status` actually receives
the intended args.
In `@src/extension.ts`:
- Around line 210-223: The mock command registration for
"ansible.lightspeed.mockSession" currently exposes an auth bypass; change the
code that registers this command so it only runs in non-production modes by
gating on context.extensionMode (allow when context.extensionMode ===
vscode.ExtensionMode.Development || context.extensionMode ===
vscode.ExtensionMode.Test) or on a test-only env var (e.g.
process.env.ANSIBLE_WDIO_TEST === "1"); specifically wrap or conditionally
execute the context.subscriptions.push(...) that calls
vscode.commands.registerCommand for "ansible.lightspeed.mockSession" (which
invokes lightSpeedManager.lightSpeedAuthenticationProvider.setMockSession) so it
is not registered in production, and if you keep any runtime path, validate the
session payload shape and reject or throw when not in dev/test.
In `@src/features/lightspeed/lightSpeedOAuthProvider.ts`:
- Around line 175-201: setMockSession currently only writes SESSIONS_SECRET_KEY
which leaves ACCOUNT_SECRET_KEY unset and causes refreshAccessToken (which reads
ACCOUNT_SECRET_KEY) to throw; update setMockSession to also store a synthetic
OAuthAccount (the same shape produced by
requestOAuthAccountFromCode/createSession) under ACCOUNT_SECRET_KEY with the
account id/label and a far-future expiresAtTimestampInSeconds so
refreshAccessToken and related logic see a valid, non-expiring account;
reference setMockSession, createSession, requestOAuthAccountFromCode,
SESSIONS_SECRET_KEY, ACCOUNT_SECRET_KEY, and refreshAccessToken when making the
change.
In `@test/wdio/mcp.spec.ts`:
- Around line 150-173: The test title and assertions are inconsistent: rename
the test or actually verify the MCP quick-pick. Update the spec so either (A)
change the it(...) description to something like "registers
ansible.mcpServer.enabled command" to match the existing assertion referencing
ansible.mcpServer.enabled and remove the unused MCP command invocation, or (B)
drive the "MCP: List Servers" flow: invoke workbench.executeCommand("MCP: List
Servers"), interact with the quick pick returned by VS Code APIs and assert that
the expected MCP server item appears in the quick pick results (keep the
try/catch only around command invocation if necessary), referencing the commands
"MCP: List Servers" and ansible.mcpServer.enabled to locate the code to change.
In `@test/wdio/sidebar.spec.ts`:
- Around line 43-58: The loop that opens each Webview (using
workbench.getAllWebviews(), webview.open()) must ensure webview.close() always
runs; wrap the per-iteration operations (opening the webview, locating the link
via browser.$('a[title="Ansible Development Tools welcome page"]') and calling
isExisting()) in a try/finally so webview.close() is invoked in finally even if
browser.$ or isExisting() throws; update the code around the webview variable
and the link check to use try { ... } finally { await webview.close(); } and
keep the existing break logic when found.
- Around line 60-89: The test modifies ansible.lightspeed.enabled via
browser.executeWorkbench and currently only restores it after assertions, which
can leak state on failure; change the test so the config update to set
"lightspeed.enabled" back to true always runs (wrap the changes and asserts in a
try/finally inside the it block or move the restore into an afterEach hook),
using the same vscode.workspace.getConfiguration("ansible").update call in the
finally/afterEach to reset the value to true; ensure the
assert.strictEqual(disabled, false) and subsequent checks remain inside the try
so the restore always executes even if the assertion fails.
---
Nitpick comments:
In `@eslint.config.mjs`:
- Around line 115-123: The current ESLint config disables linting for the whole
scripts/ folder via the ignores entry; narrow it to only the added file by
replacing ignores: ["scripts/**"] with ignores:
["scripts/install-test-extensions.mjs"] (or a more specific glob for just the
test-extension installer), or alternatively remove the ignores entry and add a
per-folder override that sets appropriate parser/project options for scripts
(adjust the override block that currently contains ignores/files) so only that
single script is exempt while other scripts continue to be linted; look for the
override object containing "ignores" and "files" in eslint.config.mjs and update
it accordingly.
In `@scripts/install-test-extensions.mjs`:
- Around line 34-41: The loop that invokes execSync per extension should be
replaced with a single execFileSync call that passes cliPath as the executable
and an argv array containing one --extensions-dir "<extensionsDir>" plus
multiple --install-extension entries derived from DEPENDENCY_EXTENSIONS; update
the code that currently iterates over DEPENDENCY_EXTENSIONS and calls execSync
to instead build args = [...DEPENDENCY_EXTENSIONS.flatMap(ext =>
["--install-extension", ext]), "--force", "--extensions-dir", extensionsDir] and
call execFileSync(cliPath, args, { stdio: "inherit" }) so you avoid shell
quoting issues and batch installs in one Electron startup.
In `@Taskfile.yml`:
- Around line 235-236: Replace the redundant "npx pnpm ..." invocations in
Taskfile.yml with direct "pnpm ..." calls: update the task commands containing
the strings "npx pnpm pretest:wdio" and "{{.XVFB}}npx pnpm test:wdio" to remove
the "npx " prefix so they read "pnpm pretest:wdio" and "{{.XVFB}}pnpm
test:wdio", respectively.
- Around line 227-228: wdio task incorrectly depends on the heavy package task;
change Taskfile.yml so the wdio task's deps list uses the lighter build (or
build:extension) task instead of package. Edit the Taskfile.yml entry for the
wdio task (deps: - package) and replace the package dependency with build or
build:extension so WDIO uses the local extension folder (wdio.conf.ts uses
extensionPath: path.resolve(process.cwd())) and avoids running
vsce/knip/can-release.sh on every test run.
In `@test/README.md`:
- Around line 66-78: Replace the unusual invocation "npx pnpm test:wdio --
--spec test/wdio/smoke.spec.ts" with the simpler "pnpm test:wdio -- --spec
test/wdio/smoke.spec.ts" in the UI Tests (WDIO) section and make the same change
wherever the same command appears (notably the docs/development/test_code.md
file); just drop the leading "npx" so pnpm is invoked directly (keep the rest of
the command and spacing intact).
In `@test/wdio/mcp.spec.ts`:
- Around line 107-115: The current if/else around sawNotification is
tautological; replace the branch with a single assertion that either
sawNotification is true or readMcpServerEnabledAnsibleSection() returns true.
Concretely, remove the if/else and assert that (sawNotification || await
readMcpServerEnabledAnsibleSection() === true) using the existing assert
utilities so the test verifies "notification appeared OR
ansible.mcpServer.enabled is true" in one check; reference sawNotification and
readMcpServerEnabledAnsibleSection() to locate the change.
In `@test/wdio/mock-server.ts`:
- Around line 174-176: The error listener is being attached after calling
app.listen which is racy; instead obtain the Server from app.listen, attach
server.once("error", reject) before waiting for it to start, and then wait for
the "listening" event (server.once("listening", resolve)) or call resolve after
wiring the listener so the sequence is: assign server from app.listen, attach
server.once("error", reject), then attach server.once("listening", resolve)
(referencing server, app.listen, reject, resolve).
In `@test/wdio/smoke.spec.ts`:
- Around line 31-41: The test builds fixturePath using process.cwd(), which can
be incorrect in different runner CWDs; change the spec to resolve the fixture
using the opened workspace folder by retrieving
vscode.workspace.workspaceFolders[0] inside the browser.executeWorkbench
callback and constructing the URI with vscode.Uri.joinPath (so replace the
external fixturePath variable usage and instead pass the workspace-based URI
into openTextDocument/openTextDocument(vscode.Uri.joinPath(...)) inside the
existing browser.executeWorkbench call), ensuring the same workspace folder that
wdio.conf.ts opens is used.
In `@wdio.conf.ts`:
- Around line 24-28: The default userSettings currently enables
"ansible.lightspeed.enabled" which can cause real Lightspeed network activity in
unrelated specs; change the default in wdio.conf.ts to
"ansible.lightspeed.enabled": false and update the Lightspeed-specific test
(test/wdio/lightspeed.spec.ts) to explicitly enable it at test startup via the
settings editor or workspace settings (e.g., using workbench.getSettingsEditor()
or the equivalent helper) so only that spec flips the flag for its scope.
- Around line 53-56: The mochaOpts block in wdio.conf.ts lacks a retries setting
which can help absorb transient Electron UI test flakiness; update the mochaOpts
object (mochaOpts) to include retries: 1 (e.g., add the property retries: 1
alongside ui and timeout) so failing tests get a single automatic retry, keeping
other settings like timeout unchanged.
- Around line 22-23: The paths for extensionPath and workspacePath are
incorrectly anchored to process.cwd(), causing brittle resolution; change both
to be resolved relative to the config file directory instead. Replace
path.resolve(process.cwd()) with a directory derived from the config file (use
__dirname when running under CommonJS or
path.dirname(fileURLToPath(import.meta.url)) for ESM) and compute workspacePath
as path.resolve(configDir, "test", "wdio", "fixtures"); update the references
for extensionPath and workspacePath so they use that configDir variable instead
of process.cwd().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f97b81d5-435b-4339-aa6a-633622228cd6
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamluv.lockis excluded by!**/*.lock
📒 Files selected for processing (47)
.config/dictionary.txt.cursor/skills/wdio-testing/SKILL.md.cursor/skills/wdio-testing/patterns.md.github/workflows/ci.yaml.gitignoreTaskfile.ymldocker-compose.ymldocs/development/test_code.mdeslint.config.mjsknip.tspackage.jsonpyproject.tomlscripts/install-test-extensions.mjssrc/extension.tssrc/features/lightspeed/lightSpeedOAuthProvider.tstest/README.mdtest/ui/__init__.pytest/ui/conftest.pytest/ui/const.pytest/ui/fixtures/__init__.pytest/ui/fixtures/ui_fixtures.pytest/ui/hooks/__init__.pytest/ui/hooks/logging_hook.pytest/ui/test_00_commands.pytest/ui/test_01_dev_webviews.pytest/ui/test_02_welcome_webviews.pytest/ui/test_03_llm_provider_webview.pytest/ui/test_50_mcp_server.pytest/ui/test_80_lightspeed.pytest/ui/test_81_login.pytest/ui/test_82_lightspeed_trial.pytest/ui/utils/__init__.pytest/ui/utils/settings_utils.pytest/ui/utils/ui_utils.pytest/wdio/commands.spec.tstest/wdio/fixtures/.vscode/settings.jsontest/wdio/fixtures/playbook.ansible.ymltest/wdio/lightspeed.spec.tstest/wdio/mcp.spec.tstest/wdio/mock-server.tstest/wdio/sidebar.spec.tstest/wdio/smoke.spec.tstest/wdio/terminal.spec.tstest/wdio/tsconfig.jsontest/wdio/webviews.spec.tstsconfig.jsonwdio.conf.ts
💤 Files with no reviewable changes (19)
- test/ui/utils/init.py
- test/ui/hooks/init.py
- test/ui/init.py
- test/ui/fixtures/init.py
- test/ui/conftest.py
- test/ui/const.py
- docker-compose.yml
- test/ui/utils/settings_utils.py
- test/ui/hooks/logging_hook.py
- test/ui/test_02_welcome_webviews.py
- test/ui/test_81_login.py
- test/ui/test_00_commands.py
- test/ui/test_82_lightspeed_trial.py
- test/ui/test_01_dev_webviews.py
- test/ui/test_80_lightspeed.py
- test/ui/test_03_llm_provider_webview.py
- test/ui/utils/ui_utils.py
- test/ui/test_50_mcp_server.py
- test/ui/fixtures/ui_fixtures.py
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/features/lightspeed/lightSpeedOAuthProvider.ts (1)
175-201:⚠️ Potential issue | 🟡 Minor
ACCOUNT_SECRET_KEYstill not populated — refresh path will throw.
setMockSessionwritesSESSIONS_SECRET_KEYbut never stores a syntheticOAuthAccountunderACCOUNT_SECRET_KEY. If any WDIO flow triggersrefreshAccessToken(Lines 562-565 guardthrow new Error("Unable to fetch account")), or a stale account from a prior run is present, the refresh path will throw or behave on stale data. Store a syntheticOAuthAccountwith a far-futureexpiresAtTimestampInSecondshere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/lightspeed/lightSpeedOAuthProvider.ts` around lines 175 - 201, setMockSession currently stores only SESSIONS_SECRET_KEY and omits the required synthetic OAuthAccount under ACCOUNT_SECRET_KEY, which causes refreshAccessToken (guard that throws "Unable to fetch account") to fail; update setMockSession to also write a synthetic OAuthAccount object (matching the shape expected by refreshAccessToken) into this.context.secrets.store using ACCOUNT_SECRET_KEY, include account id/label and a far-future expiresAtTimestampInSeconds so the refresh path sees a valid non-expired account, and ensure the stored value is JSON.stringify'd like the sessions entry.
🧹 Nitpick comments (3)
src/features/lightspeed/lightSpeedOAuthProvider.ts (1)
189-190: Hardcoded subscription/admin flags limit test coverage.
rhOrgHasSubscriptionandrhUserIsOrgAdminare fixed tofalse, so WDIO tests cannot exercise licensed/admin-gated UI paths (seegetUserTypeLabel/getLoggedInUserDetailsinsrc/features/lightspeed/utils/webUtils.ts, which branch on these fields to render "Licensed"/"Administrator"). Consider accepting them as optionalsessionParamsto broaden scenario coverage.♻️ Proposed change
public async setMockSession(sessionParams: { accessToken: string; accountId: string; accountLabel: string; + rhOrgHasSubscription?: boolean; + rhUserIsOrgAdmin?: boolean; }): Promise<void> { const previous = await this.getSessions(); const session: LightspeedAuthSession = { id: sessionParams.accountId, accessToken: sessionParams.accessToken, account: { id: sessionParams.accountId, label: sessionParams.accountLabel, }, scopes: [], - rhOrgHasSubscription: false, - rhUserIsOrgAdmin: false, + rhOrgHasSubscription: sessionParams.rhOrgHasSubscription ?? false, + rhUserIsOrgAdmin: sessionParams.rhUserIsOrgAdmin ?? false, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/lightspeed/lightSpeedOAuthProvider.ts` around lines 189 - 190, The code currently hardcodes rhOrgHasSubscription and rhUserIsOrgAdmin to false in lightSpeedOAuthProvider.ts which prevents tests from covering licensed/admin UI paths; update the session creation logic (the code that sets rhOrgHasSubscription and rhUserIsOrgAdmin) to accept optional sessionParams so tests can pass true/false values, propagate those params into the session object used by getUserTypeLabel/getLoggedInUserDetails in src/features/lightspeed/utils/webUtils.ts, and ensure defaults remain false when sessionParams are not provided.test/wdio/smoke.spec.ts (1)
13-44: LGTM — clean activation-wait helper.
waitForExtensionActivecorrectly triggers activation and polls viaexecuteWorkbench, and thebeforehook opens the fixture and waits before any assertion. One small nit: thebrowser.pause(3000)on line 43 afterwaitForExtensionActive()is an arbitrary grace period — if flake surfaces you may want to replace it with an explicit readiness check (e.g., waiting for a known Ansible status bar item or language server ready signal) rather than a fixed sleep.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/wdio/smoke.spec.ts` around lines 13 - 44, Replace the fixed 3s sleep in the before hook with an explicit readiness check: remove browser.pause(3000) and instead wait for a deterministic signal (for example, poll for a known Ansible status bar item, a language server-ready indicator, or a specific DOM/text via browser.executeWorkbench) after waitForExtensionActive() returns; use an appropriate browser.waitUntil timeout and reference the same pattern used in waitForExtensionActive() to ensure the extension is fully ready before proceeding.test/wdio/commands.spec.ts (1)
39-48: Dedupe the extension-activation wait withsmoke.spec.ts.This polling loop is functionally identical to
waitForExtensionActiveintest/wdio/smoke.spec.ts(lines 13-26). The.cursor/skills/wdio-testing/SKILL.mdonboarding doc also calls out a single canonical helper. Consider extracting it into a shared util (e.g.test/wdio/utils/extension.ts) so both specs import the same implementation and future changes (timeouts, retries, error messaging) apply in one place.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/wdio/commands.spec.ts` around lines 39 - 48, Extract the polling loop into a shared helper named waitForExtensionActive and export it from a common test util (e.g., an extensions util module), then replace the inline browser.waitUntil in commands.spec.ts with a call to that helper; specifically, move the anonymous async function that calls browser.executeWorkbench and checks vscode.extensions.getExtension("redhat.ansible")?.isActive into waitForExtensionActive, export the function, update commands.spec.ts to import and await waitForExtensionActive(...), and also update smoke.spec.ts to import the same helper so both specs use the single canonical implementation (preserve existing timeout/interval behavior and surface errors consistently).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/wdio/commands.spec.ts`:
- Around line 29-52: The before hook silently skips opening the fixture when
vscode.workspace.workspaceFolders?.[0] is undefined which masks setup failures;
update the browser.executeWorkbench block used to open "playbook.ansible.yml" to
explicitly throw or assert if no workspace folder is found (so the before hook
fails fast with a clear message), and add a descriptive timeoutMsg to the
browser.waitUntil call that checks extension activation (referencing waitUntil
and the extension check using vscode.extensions.getExtension("redhat.ansible"))
so a failed activation surfaces with a clear error instead of a generic timeout;
keep the existing closeAllEditors command but ensure the new explicit
workspace-folder error is raised before proceeding to
ansible.create-empty-playbook.
---
Duplicate comments:
In `@src/features/lightspeed/lightSpeedOAuthProvider.ts`:
- Around line 175-201: setMockSession currently stores only SESSIONS_SECRET_KEY
and omits the required synthetic OAuthAccount under ACCOUNT_SECRET_KEY, which
causes refreshAccessToken (guard that throws "Unable to fetch account") to fail;
update setMockSession to also write a synthetic OAuthAccount object (matching
the shape expected by refreshAccessToken) into this.context.secrets.store using
ACCOUNT_SECRET_KEY, include account id/label and a far-future
expiresAtTimestampInSeconds so the refresh path sees a valid non-expired
account, and ensure the stored value is JSON.stringify'd like the sessions
entry.
---
Nitpick comments:
In `@src/features/lightspeed/lightSpeedOAuthProvider.ts`:
- Around line 189-190: The code currently hardcodes rhOrgHasSubscription and
rhUserIsOrgAdmin to false in lightSpeedOAuthProvider.ts which prevents tests
from covering licensed/admin UI paths; update the session creation logic (the
code that sets rhOrgHasSubscription and rhUserIsOrgAdmin) to accept optional
sessionParams so tests can pass true/false values, propagate those params into
the session object used by getUserTypeLabel/getLoggedInUserDetails in
src/features/lightspeed/utils/webUtils.ts, and ensure defaults remain false when
sessionParams are not provided.
In `@test/wdio/commands.spec.ts`:
- Around line 39-48: Extract the polling loop into a shared helper named
waitForExtensionActive and export it from a common test util (e.g., an
extensions util module), then replace the inline browser.waitUntil in
commands.spec.ts with a call to that helper; specifically, move the anonymous
async function that calls browser.executeWorkbench and checks
vscode.extensions.getExtension("redhat.ansible")?.isActive into
waitForExtensionActive, export the function, update commands.spec.ts to import
and await waitForExtensionActive(...), and also update smoke.spec.ts to import
the same helper so both specs use the single canonical implementation (preserve
existing timeout/interval behavior and surface errors consistently).
In `@test/wdio/smoke.spec.ts`:
- Around line 13-44: Replace the fixed 3s sleep in the before hook with an
explicit readiness check: remove browser.pause(3000) and instead wait for a
deterministic signal (for example, poll for a known Ansible status bar item, a
language server-ready indicator, or a specific DOM/text via
browser.executeWorkbench) after waitForExtensionActive() returns; use an
appropriate browser.waitUntil timeout and reference the same pattern used in
waitForExtensionActive() to ensure the extension is fully ready before
proceeding.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 53b33ecc-0f8f-490f-8b02-2341b622dfa6
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamluv.lockis excluded by!**/*.lock
📒 Files selected for processing (47)
.config/dictionary.txt.cursor/skills/wdio-testing/SKILL.md.cursor/skills/wdio-testing/patterns.md.github/workflows/ci.yaml.gitignoreTaskfile.ymldocker-compose.ymldocs/development/test_code.mdeslint.config.mjsknip.tspackage.jsonpyproject.tomlscripts/install-test-extensions.mjssrc/extension.tssrc/features/lightspeed/lightSpeedOAuthProvider.tstest/README.mdtest/ui/__init__.pytest/ui/conftest.pytest/ui/const.pytest/ui/fixtures/__init__.pytest/ui/fixtures/ui_fixtures.pytest/ui/hooks/__init__.pytest/ui/hooks/logging_hook.pytest/ui/test_00_commands.pytest/ui/test_01_dev_webviews.pytest/ui/test_02_welcome_webviews.pytest/ui/test_03_llm_provider_webview.pytest/ui/test_50_mcp_server.pytest/ui/test_80_lightspeed.pytest/ui/test_81_login.pytest/ui/test_82_lightspeed_trial.pytest/ui/utils/__init__.pytest/ui/utils/settings_utils.pytest/ui/utils/ui_utils.pytest/wdio/commands.spec.tstest/wdio/fixtures/.vscode/settings.jsontest/wdio/fixtures/playbook.ansible.ymltest/wdio/lightspeed.spec.tstest/wdio/mcp.spec.tstest/wdio/mock-server.tstest/wdio/sidebar.spec.tstest/wdio/smoke.spec.tstest/wdio/terminal.spec.tstest/wdio/tsconfig.jsontest/wdio/webviews.spec.tstsconfig.jsonwdio.conf.ts
💤 Files with no reviewable changes (19)
- test/ui/init.py
- test/ui/fixtures/init.py
- test/ui/utils/init.py
- test/ui/const.py
- test/ui/hooks/init.py
- docker-compose.yml
- test/ui/utils/settings_utils.py
- test/ui/hooks/logging_hook.py
- test/ui/test_82_lightspeed_trial.py
- test/ui/conftest.py
- test/ui/test_01_dev_webviews.py
- test/ui/test_81_login.py
- test/ui/test_02_welcome_webviews.py
- test/ui/test_50_mcp_server.py
- test/ui/test_03_llm_provider_webview.py
- test/ui/test_00_commands.py
- test/ui/test_80_lightspeed.py
- test/ui/fixtures/ui_fixtures.py
- test/ui/utils/ui_utils.py
✅ Files skipped from review due to trivial changes (11)
- .gitignore
- test/wdio/fixtures/.vscode/settings.json
- .config/dictionary.txt
- tsconfig.json
- test/wdio/fixtures/playbook.ansible.yml
- test/README.md
- knip.ts
- docs/development/test_code.md
- test/wdio/tsconfig.json
- test/wdio/sidebar.spec.ts
- .cursor/skills/wdio-testing/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (13)
- src/extension.ts
- eslint.config.mjs
- scripts/install-test-extensions.mjs
- test/wdio/terminal.spec.ts
- wdio.conf.ts
- test/wdio/lightspeed.spec.ts
- test/wdio/webviews.spec.ts
- pyproject.toml
- test/wdio/mcp.spec.ts
- test/wdio/mock-server.ts
- package.json
- Taskfile.yml
- .cursor/skills/wdio-testing/patterns.md
Replace the Python/Selenium container-based UI test suite with TypeScript WebDriverIO tests using wdio-vscode-service, running against real VS Code Electron. Adds dedicated test (linux-wdio) CI runner. related: ansible#2746 Made-with: Cursor
anusshukla
approved these changes
Apr 17, 2026
rockygeekz
approved these changes
Apr 17, 2026
shatakshiiii
approved these changes
Apr 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
test/ui/,docker-compose.yml, Selenium/Podman Python deps) — eliminates the fragileselenium-vscodecontainer, brittle XPath selectors, and DOM mismatches betweencode-serverand desktop VS Code.test/wdio/,wdio.conf.ts) usingwdio-vscode-service, which tests against real VS Code Electron. 30 tests across 7 spec files: smoke, commands, terminal, sidebar, webviews (devfile, devcontainer, welcome page, LLM provider settings), MCP server toggle, and Lightspeed (backed by an Express mock server with injected auth session).test (linux-wdio)matrix entry, independent of unit/e2e. No more cascading failures from unrelated e2e issues blocking UI tests.ci.yaml,Taskfile.yml) to runtask wdiounderxvfb-runinstead of the Selenium container, removing the Podman dependency from Linux CI..cursor/skills/wdio-testing/) with architecture, patterns, and gotchas; updatedtest/README.mdanddocs/development/test_code.md.Why
All 29 Selenium tests on
mainare currently skipped — zero tests actually execute:test_00_commands.pytest_01_dev_webviews.pytest_02_welcome_webviews.pytest_03_llm_provider_webview.pytest_50_mcp_server.pytest_80_lightspeed.pytest_81_login.pytest_82_lightspeed_trial.pyThe Selenium suite had a 100% CI failure rate for the last two weeks on
main(W15-W16) before the tests were skipped, and an accelerating failure rate before that (5% → 16% → 38% → 100% over 8 weeks). Root causes:code-serverDOM divergence from real VS Code, fragile XPath selectors, deeply nested iframe traversal, and container startup timing. See full analysis.WDIO with
wdio-vscode-serviceprovides stable Page Objects against the real VS Code Electron shell,browser.executeWorkbench()for direct access to the VS Code API, and zero container overhead.AAP-67210 Coverage
The 14 Lightspeed/SSO tests tracked by AAP-67210 were all skipped in Selenium. WDIO restores coverage for 5 of them using a mock server approach, and adds 2 new tests (error/timeout handling) that never existed:
test_vscode_widgetshould have Lightspeed commands registeredtest_vscode_playbook_explanationshould open playbook explanation webviewtest_vscode_playbook_generationshould open playbook generation webviewtest_vscode_role_generationshould open role generation webviewtest_vscode_lightspeed_explorertest_vscode_trial_buttontest_unsubed_logintest_unsubed_admin_logintest_no_wca_user_logintest_no_wca_admin_logintest_login_pagetest_sso_auth_flowtest_admin_portal_errortest_vscode_rhsso_auth_flowThe 8 SSO login tests exercise Red Hat SSO infrastructure (external OAuth pages), not extension code. They require live SSO credentials and a running Lightspeed backend, which is why they were never functional in CI.
CI architecture
The WDIO job skips
task package(no.vsixneeded) and runs onlytask build→task wdio. Either job can be re-run independently.Test coverage comparison
Extension source changes
src/extension.ts: registersansible.lightspeed.mockSessioncommand (gated toExtensionMode.Development/Testonly — not available in production)src/features/lightspeed/lightSpeedOAuthProvider.ts: addssetMockSession()to inject fake auth sessionsTest plan
task wdiopasses locally underxvfb-runtest (linux-wdio)job passestask lintpassestask unit/task e2eunaffected ontest (linux)Closes: #2722
Closes: #2743
Closes: #2720
Closes: #2752
related: AAP-67210