feat(skills): add windows-desktop-e2e skill#1334
feat(skills): add windows-desktop-e2e skill#1334CodeQC wants to merge 1 commit intoaffaan-m:mainfrom
Conversation
Add a new skill for E2E testing of Windows native desktop applications (WPF, WinForms, Win32/MFC, Qt 5.x/6.x) using pywinauto and Windows UI Automation. Includes: - BasePage with locators (by_id/by_name/by_class), wait helpers (wait_visible, wait_gone, wait_window, wait_until), click/type_text with Qt 5.x set_edit_text fallback, screenshot capture - Three-tier sandbox isolation: · Tier 1: filesystem env redirect via subprocess.Popen + tmp_path · Tier 2: Windows Job Object for process-lifetime containment · Tier 3: Windows Sandbox (.wsb) for full OS isolation - Page Object Model scaffolding and locator strategy guide - Qt-specific notes: QT_ACCESSIBILITY (5.x only), setObjectName/ setAccessibleName, QComboBox popup window class (Qt5/Qt6) - Registered in install manifests as skill:windows-desktop-e2e (--with skill:windows-desktop-e2e) - Updated skill counts in docs (181→182) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
📝 WalkthroughWalkthroughThis PR adds a new Windows Desktop E2E testing skill module ( Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Suggested reviewers
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 |
Greptile SummaryThis PR adds a new
Confidence Score: 4/5Skill content is well-structured but has one P1 defect (broken import in the Tier 1 conftest example) that should be fixed before merging to avoid misleading users. All manifest registrations and documentation updates are clean. The P1 issue is limited to example code in SKILL.md — the Tier 1 fixture imports skills/windows-desktop-e2e/SKILL.md — the Tier 1 conftest and config.py examples need to be made consistent.
|
| Filename | Overview |
|---|---|
| skills/windows-desktop-e2e/SKILL.md | New skill for Windows desktop E2E with pywinauto; Tier 1 conftest imports APP_ARGS and APP_TITLE that are absent from the provided config.py template (immediate ImportError), and the CI YAML omits pytest-timeout from its install step. |
| manifests/install-modules.json | Adds windows-desktop-e2e module entry with correct paths, targets, and defaultInstall: false. |
| manifests/install-components.json | Adds skill:windows-desktop-e2e component referencing the new module; structure follows existing patterns correctly. |
| manifests/install-profiles.json | Adds windows-desktop-e2e to the full profile module list; looks correct. |
| AGENTS.md | Skill count bumped 181→182 in description and directory listing; straightforward counter update. |
| README.md | Skill count updated 181→182 in three places (install blurb, comparison table, multi-tool table); consistent. |
| README.zh-CN.md | Chinese README skill count updated 181→182; mirrors README.md change. |
| docs/zh-CN/AGENTS.md | Simplified Chinese AGENTS.md skill count updated 181→182; straightforward. |
| docs/zh-CN/README.md | Simplified Chinese README skill counts updated in three locations (182); consistent with other doc changes. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[pytest fixture: app] --> B{APP_PATH set?}
B -- No --> X[pytest.exit returncode=1]
B -- Yes --> C[subprocess.Popen with sandbox_env]
C --> D[Application.connect by PID]
D --> E[win.wait visible]
E --> F[yield win to test]
F --> G{test passed?}
G -- No --> H[capture_as_image FAIL screenshot]
G -- Yes --> I[win.close + proc.wait 5s]
H --> I
I -- timeout --> J[proc.kill]
I -- ok --> K[tmp_path auto-cleanup]
subgraph Tier2 [Optional: Tier 2 Job Object]
C --> L[restrict_process pid]
L --> M[CreateJobObjectW]
M --> N[SetInformationJobObject KILL_ON_JOB_CLOSE]
N --> O[AssignProcessToJobObject]
O --> P[return job handle — keep alive]
end
Reviews (1): Last reviewed commit: "feat: add windows-desktop-e2e skill" | Re-trigger Greptile
| # conftest.py — replace the basic `app` fixture with this | ||
| import os, subprocess, pytest | ||
| from pywinauto import Application | ||
| from config import APP_PATH, APP_ARGS, APP_TITLE, LAUNCH_TIMEOUT, ACTION_TIMEOUT, ARTIFACT_DIR |
There was a problem hiding this comment.
APP_ARGS and APP_TITLE missing from config.py
The Tier 1 conftest imports APP_ARGS and APP_TITLE from config, but config.py only defines APP_PATH, MAIN_WINDOW_TITLE, LAUNCH_TIMEOUT, ACTION_TIMEOUT, and ARTIFACT_DIR. Any user who copies this fixture verbatim will get an immediate ImportError on collection. config.py should export both missing names:
| # conftest.py — replace the basic `app` fixture with this | |
| import os, subprocess, pytest | |
| from pywinauto import Application | |
| from config import APP_PATH, APP_ARGS, APP_TITLE, LAUNCH_TIMEOUT, ACTION_TIMEOUT, ARTIFACT_DIR | |
| from config import APP_PATH, APP_ARGS, APP_TITLE, LAUNCH_TIMEOUT, ACTION_TIMEOUT, ARTIFACT_DIR |
And config.py needs to be updated to match:
APP_PATH = os.environ.get("APP_PATH", "")
APP_TITLE = os.environ.get("APP_TITLE", "") # was MAIN_WINDOW_TITLE
APP_ARGS = os.environ.get("APP_ARGS", "") # new
LAUNCH_TIMEOUT = int(os.environ.get("LAUNCH_TIMEOUT", "15"))
ACTION_TIMEOUT = int(os.environ.get("ACTION_TIMEOUT", "10"))
ARTIFACT_DIR = os.path.join(os.path.dirname(__file__), "artifacts")| with: { python-version: "3.11" } | ||
|
|
||
| - name: Install deps | ||
| run: pip install pywinauto pytest pytest-html Pillow | ||
|
|
There was a problem hiding this comment.
pytest-timeout missing from CI dependency install
The Setup & Prerequisites section (line 54) lists pytest-timeout as a required dependency, and the "Prevent hanging tests" section explicitly instructs users to add it. The CI workflow's install step omits it, so tests that rely on timeout in pytest.ini will silently ignore the directive when run in CI.
| with: { python-version: "3.11" } | |
| - name: Install deps | |
| run: pip install pywinauto pytest pytest-html Pillow | |
| run: pip install pywinauto pytest pytest-html Pillow pytest-timeout |
| ```ini | ||
| [pytest] | ||
| testpaths = tests | ||
| markers = | ||
| smoke: fast smoke tests for critical paths | ||
| flaky: known-unstable tests | ||
| addopts = -v --tb=short --html=artifacts/report.html --self-contained-html | ||
| ``` |
There was a problem hiding this comment.
pytest.ini template omits recommended timeout settings
The "Prevent hanging tests" section tells users to set timeout = 60 and timeout_method = thread in pytest.ini, but the template shown earlier doesn't include those keys. Users who copy the template will have pytest-timeout installed but no effective per-test cap, defeating the hang-prevention guidance.
| ```ini | |
| [pytest] | |
| testpaths = tests | |
| markers = | |
| smoke: fast smoke tests for critical paths | |
| flaky: known-unstable tests | |
| addopts = -v --tb=short --html=artifacts/report.html --self-contained-html | |
| ``` | |
| [pytest] | |
| testpaths = tests | |
| timeout = 60 | |
| timeout_method = thread | |
| markers = | |
| smoke: fast smoke tests for critical paths | |
| flaky: known-unstable tests | |
| addopts = -v --tb=short --html=artifacts/report.html --self-contained-html |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
README.md (1)
239-239: Counts are correctly updated; consider centralizing this value.All three
182 skillsupdates are consistent with this PR. Optional improvement: generate these public counts from a single manifest source during docs build/release to avoid future drift.Also applies to: 1158-1158, 1267-1267
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 239, The README embeds hard-coded counts in the string "47 agents, 182 skills, and 79 legacy command shims" which risks drift; centralize these numbers into a single source (e.g., a manifest file like publicCounts.json or manifest.yaml) and change the docs build/release step (e.g., the buildDocs or renderDocs function) to read that manifest and inject the counts into README rendering so all occurrences (the "47 agents, 182 skills, and 79 legacy command shims" string) are generated from one canonical value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/windows-desktop-e2e/SKILL.md`:
- Around line 295-299: The Tier 1 fixture example imports/uses APP_TITLE but
config.py defines MAIN_WINDOW_TITLE, causing an import/unresolved variable
error; update the fixture example to reference MAIN_WINDOW_TITLE (or rename the
config constant to APP_TITLE) so both snippets use the same identifier. Locate
the Tier 1 fixture sample that references APP_TITLE and change it to
MAIN_WINDOW_TITLE (or vice‑versa) ensuring all occurrences in the document
(including the other affected block around lines 403-432) are updated to match
the chosen symbol.
- Around line 11-26: Rename or add the required top-level section headings in
SKILL.md: replace or duplicate the current "When to Activate" block under the
exact heading "When to Use", add a "How It Works" section containing the current
"Core Concepts" content, and add an "Examples" section (even if placeholder) and
move any example snippets there; ensure the file now contains the exact headings
"When to Use", "How It Works", and "Examples" so it matches the skill spec.
- Line 570: The docs mention using psutil in conftest.py and pytest-timeout in
pytest.ini but never instruct users to install them nor add them to the GitHub
Actions workflow; update SKILL.md to include installation instructions (e.g.,
pip install psutil pytest-timeout or add to requirements/dev-requirements),
ensure pytest.ini references timeout and timeout_method as described, and modify
the CI workflow to install both pytest-timeout and psutil (or the appropriate
requirements file) before running tests so conftest.py’s atexit.register cleanup
and pytest-timeout work on CI.
---
Nitpick comments:
In `@README.md`:
- Line 239: The README embeds hard-coded counts in the string "47 agents, 182
skills, and 79 legacy command shims" which risks drift; centralize these numbers
into a single source (e.g., a manifest file like publicCounts.json or
manifest.yaml) and change the docs build/release step (e.g., the buildDocs or
renderDocs function) to read that manifest and inject the counts into README
rendering so all occurrences (the "47 agents, 182 skills, and 79 legacy command
shims" string) are generated from one canonical value.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 726e07de-a1fb-45ec-96b4-5bfd2649442f
📒 Files selected for processing (9)
AGENTS.mdREADME.mdREADME.zh-CN.mddocs/zh-CN/AGENTS.mddocs/zh-CN/README.mdmanifests/install-components.jsonmanifests/install-modules.jsonmanifests/install-profiles.jsonskills/windows-desktop-e2e/SKILL.md
| ## When to Activate | ||
|
|
||
| - Writing or running E2E tests for a Windows native desktop application | ||
| - Setting up a desktop GUI test suite from scratch | ||
| - Diagnosing flaky or failing desktop automation tests | ||
| - Adding testability (AutomationId, accessible names) to an existing app | ||
| - Integrating desktop E2E into a CI/CD pipeline (GitHub Actions `windows-latest`) | ||
|
|
||
| ### When NOT to Use | ||
|
|
||
| - Web applications → use `e2e-testing` skill (Playwright) | ||
| - Electron / CEF / WebView2 apps → the HTML layer needs browser automation, not UIA | ||
| - Mobile apps → use platform-specific tools (UIAutomator, XCUITest) | ||
| - Pure unit or integration tests that don't need a running GUI | ||
|
|
||
| ## Core Concepts |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add explicit required section headers to match skill spec.
The doc is comprehensive, but it doesn’t clearly expose the exact required section names: When to Use, How It Works, and Examples. Please add these headings explicitly (you can keep current content under them).
As per coding guidelines "Skill format must be Markdown with clear sections for When to Use, How It Works, and Examples".
Also applies to: 312-317, 764-788
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/windows-desktop-e2e/SKILL.md` around lines 11 - 26, Rename or add the
required top-level section headings in SKILL.md: replace or duplicate the
current "When to Activate" block under the exact heading "When to Use", add a
"How It Works" section containing the current "Core Concepts" content, and add
an "Examples" section (even if placeholder) and move any example snippets there;
ensure the file now contains the exact headings "When to Use", "How It Works",
and "Examples" so it matches the skill spec.
| MAIN_WINDOW_TITLE = os.environ.get("APP_TITLE", "") | ||
| LAUNCH_TIMEOUT = int(os.environ.get("LAUNCH_TIMEOUT", "15")) | ||
| ACTION_TIMEOUT = int(os.environ.get("ACTION_TIMEOUT", "10")) | ||
| ARTIFACT_DIR = os.path.join(os.path.dirname(__file__), "artifacts") | ||
| ``` |
There was a problem hiding this comment.
Fix config.py key mismatch in Tier 1 fixture example.
config.py defines MAIN_WINDOW_TITLE, but the Tier 1 fixture imports/uses APP_TITLE. If copied as-is, this example breaks with ImportError or unresolved config usage. Align both snippets to one name.
Suggested doc fix
- from config import APP_PATH, APP_ARGS, APP_TITLE, LAUNCH_TIMEOUT, ACTION_TIMEOUT, ARTIFACT_DIR
+ from config import APP_PATH, APP_ARGS, MAIN_WINDOW_TITLE, LAUNCH_TIMEOUT, ACTION_TIMEOUT, ARTIFACT_DIR
...
- if not APP_TITLE:
- pytest.exit("APP_TITLE environment variable is not set", returncode=1)
+ if not MAIN_WINDOW_TITLE:
+ pytest.exit("APP_TITLE environment variable is not set", returncode=1)
...
- win = pw_app.window(title=APP_TITLE)
+ win = pw_app.window(title=MAIN_WINDOW_TITLE)Also applies to: 403-432
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/windows-desktop-e2e/SKILL.md` around lines 295 - 299, The Tier 1
fixture example imports/uses APP_TITLE but config.py defines MAIN_WINDOW_TITLE,
causing an import/unresolved variable error; update the fixture example to
reference MAIN_WINDOW_TITLE (or rename the config constant to APP_TITLE) so both
snippets use the same identifier. Locate the Tier 1 fixture sample that
references APP_TITLE and change it to MAIN_WINDOW_TITLE (or vice‑versa) ensuring
all occurrences in the document (including the other affected block around lines
403-432) are updated to match the chosen symbol.
|
|
||
| ### Prevent hanging tests | ||
|
|
||
| Add `pytest-timeout` to cap any single test. In `pytest.ini` set `timeout = 60` and `timeout_method = thread`. Note: `thread` method cannot kill Qt app subprocesses on Windows — add `atexit.register(lambda: [p.kill() for p in psutil.Process().children(recursive=True)])` in `conftest.py` to reap orphans. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the file and check its structure
find . -name "SKILL.md" -path "*/windows-desktop-e2e/*" -type fRepository: affaan-m/everything-claude-code
Length of output: 112
🏁 Script executed:
# Check the line count first
wc -l ./skills/windows-desktop-e2e/SKILL.mdRepository: affaan-m/everything-claude-code
Length of output: 116
🏁 Script executed:
# Read the section around line 570 to see the psutil mention
sed -n '560,580p' ./skills/windows-desktop-e2e/SKILL.md
# Also search for all pip install commands in the file
echo "=== All pip install commands in the file ==="
rg "pip install" ./skills/windows-desktop-e2e/SKILL.md -nRepository: affaan-m/everything-claude-code
Length of output: 1230
🏁 Script executed:
# Check the context around line 589 for the GitHub Actions workflow
sed -n '585,595p' ./skills/windows-desktop-e2e/SKILL.mdRepository: affaan-m/everything-claude-code
Length of output: 384
Document psutil installation if recommending orphan-process cleanup with it.
This section introduces psutil in executable guidance at line 570, but installation commands don't include it. Additionally, the GitHub Actions workflow at line 589 is missing both pytest-timeout and psutil. Users following this guidance will encounter missing dependency errors.
Suggested doc fixes
- pip install pywinauto pytest pytest-html Pillow pytest-timeout
+ pip install pywinauto pytest pytest-html Pillow pytest-timeout psutil- - name: Install deps
- run: pip install pywinauto pytest pytest-html Pillow
+ - name: Install deps
+ run: pip install pywinauto pytest pytest-html Pillow pytest-timeout psutil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/windows-desktop-e2e/SKILL.md` at line 570, The docs mention using
psutil in conftest.py and pytest-timeout in pytest.ini but never instruct users
to install them nor add them to the GitHub Actions workflow; update SKILL.md to
include installation instructions (e.g., pip install psutil pytest-timeout or
add to requirements/dev-requirements), ensure pytest.ini references timeout and
timeout_method as described, and modify the CI workflow to install both
pytest-timeout and psutil (or the appropriate requirements file) before running
tests so conftest.py’s atexit.register cleanup and pytest-timeout work on CI.
There was a problem hiding this comment.
3 issues found across 9 files
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="skills/windows-desktop-e2e/SKILL.md">
<violation number="1" location="skills/windows-desktop-e2e/SKILL.md:403">
P2: Tier 1 fixture imports `APP_TITLE`/`APP_ARGS` that are missing from the documented `config.py`, causing immediate import failure in copied examples.</violation>
<violation number="2" location="skills/windows-desktop-e2e/SKILL.md:427">
P2: Windows-focused example uses POSIX `shlex.split` defaults for `APP_ARGS`, which can misparse Windows command-line quoting and paths.</violation>
<violation number="3" location="skills/windows-desktop-e2e/SKILL.md:589">
P2: The CI workflow omits `pytest-timeout` (listed as a required dependency in the Setup section and used by the `pytest.ini` timeout guidance) and `psutil` (referenced in the "Prevent hanging tests" section for orphan-process cleanup). Users following this workflow will get silent no-ops for timeout directives and `ModuleNotFoundError` for the `psutil`-based cleanup.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| import shlex | ||
| # Launch via subprocess so we can pass env; connect pywinauto by PID | ||
| proc = subprocess.Popen( | ||
| [APP_PATH] + shlex.split(APP_ARGS), |
There was a problem hiding this comment.
P2: Windows-focused example uses POSIX shlex.split defaults for APP_ARGS, which can misparse Windows command-line quoting and paths.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At skills/windows-desktop-e2e/SKILL.md, line 427:
<comment>Windows-focused example uses POSIX `shlex.split` defaults for `APP_ARGS`, which can misparse Windows command-line quoting and paths.</comment>
<file context>
@@ -0,0 +1,788 @@
+ import shlex
+ # Launch via subprocess so we can pass env; connect pywinauto by PID
+ proc = subprocess.Popen(
+ [APP_PATH] + shlex.split(APP_ARGS),
+ env=sandbox_env,
+ )
</file context>
| # conftest.py — replace the basic `app` fixture with this | ||
| import os, subprocess, pytest | ||
| from pywinauto import Application | ||
| from config import APP_PATH, APP_ARGS, APP_TITLE, LAUNCH_TIMEOUT, ACTION_TIMEOUT, ARTIFACT_DIR |
There was a problem hiding this comment.
P2: Tier 1 fixture imports APP_TITLE/APP_ARGS that are missing from the documented config.py, causing immediate import failure in copied examples.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At skills/windows-desktop-e2e/SKILL.md, line 403:
<comment>Tier 1 fixture imports `APP_TITLE`/`APP_ARGS` that are missing from the documented `config.py`, causing immediate import failure in copied examples.</comment>
<file context>
@@ -0,0 +1,788 @@
+# conftest.py — replace the basic `app` fixture with this
+import os, subprocess, pytest
+from pywinauto import Application
+from config import APP_PATH, APP_ARGS, APP_TITLE, LAUNCH_TIMEOUT, ACTION_TIMEOUT, ARTIFACT_DIR
+
+@pytest.fixture(scope="function")
</file context>
| with: { python-version: "3.11" } | ||
|
|
||
| - name: Install deps | ||
| run: pip install pywinauto pytest pytest-html Pillow |
There was a problem hiding this comment.
P2: The CI workflow omits pytest-timeout (listed as a required dependency in the Setup section and used by the pytest.ini timeout guidance) and psutil (referenced in the "Prevent hanging tests" section for orphan-process cleanup). Users following this workflow will get silent no-ops for timeout directives and ModuleNotFoundError for the psutil-based cleanup.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At skills/windows-desktop-e2e/SKILL.md, line 589:
<comment>The CI workflow omits `pytest-timeout` (listed as a required dependency in the Setup section and used by the `pytest.ini` timeout guidance) and `psutil` (referenced in the "Prevent hanging tests" section for orphan-process cleanup). Users following this workflow will get silent no-ops for timeout directives and `ModuleNotFoundError` for the `psutil`-based cleanup.</comment>
<file context>
@@ -0,0 +1,788 @@
+ with: { python-version: "3.11" }
+
+ - name: Install deps
+ run: pip install pywinauto pytest pytest-html Pillow
+
+ - name: Build app
</file context>
| run: pip install pywinauto pytest pytest-html Pillow | |
| run: pip install pywinauto pytest pytest-html Pillow pytest-timeout psutil |
What Changed
Add a new skill
windows-desktop-e2efor E2E testing of Windows native desktop applications (WPF, WinForms, Win32/MFC, Qt 5.x/6.x)using pywinauto and Windows UI Automation.
Key content:
windows-latest)QT_ACCESSIBILITY, stable identifiers, QComboBox/QDialog/QTableWidget quirksinstall-components.json,install-modules.json,install-profiles.json)Why This Change
There is no existing skill for testing Windows native desktop applications. Web E2E tools (Playwright, Cypress) don't work with
WPF/WinForms/Qt UIA trees. This fills the gap for teams building Windows desktop software.
Testing Done
node tests/run-all.js) — 1 pre-existing Windows symlink failure unrelated to this PRnpx markdownlint-cli skills/windows-desktop-e2e/SKILL.mdpasses with zero errorsType of Change
feat:New featureSecurity & Quality Checklist
Documentation
Summary by cubic
Adds the
windows-desktop-e2eskill for E2E testing of Windows native desktop apps (WPF, WinForms, Win32/MFC, Qt 5/6) usingpywinautoand Windows UI Automation. Registers the skill in install manifests and profiles, and updates docs (181→182 skills).BasePagehelpers: locators (by_id/by_name/by_class), waits, click/type_text (Qt 5 fallback), screenshots.QT_ACCESSIBILITYfor Qt 5, stable identifiers, handlingQComboBox/dialogs/tables; screenshot-mode fallback for non-UIA controls.windows-latest; addedskill:windows-desktop-e2e(windows-desktop-e2emodule) toinstall-components.json,install-modules.json,install-profiles.json; updated README/AGENTS skill counts.Written for commit dad2185. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation