test(CUA-629): add NixOS integration test for Electron/CJK input#1985
test(CUA-629): add NixOS integration test for Electron/CJK input#1985r33drichards wants to merge 5 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a new NixOS flake check ( ChangesCJK Electron integration test
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
nix/cua-driver/tests/electron-cjk-input.nix (1)
366-371: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAccepting
CJK_INPUT_NONZEROlets the test pass on corrupted/partial CJK.The VM-side assertion (Lines 497-503) treats
CJK_INPUT_NONZEROas success, so any non-empty readback passes even when the CJK characters were mangled or only ASCII arrived. This defeats the test's purpose of verifying authoritative Unicode pass-through. Consider failing on partial results, or at least gating the leniency behind an explicit "known-flaky build" flag so a green check actually proves correct CJK delivery.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nix/cua-driver/tests/electron-cjk-input.nix` around lines 366 - 371, The test currently accepts partial or corrupted CJK input by printing "CJK_INPUT_NONZERO" which allows mangled characters to pass the VM-side assertion at lines 497-503. Instead of unconditionally printing "CJK_INPUT_NONZERO" in the partial success branch (the elif block checking readback_value and len(readback_value) > 0), you should either fail the test outright by raising an exception for partial results, or gate the "CJK_INPUT_NONZERO" acceptance behind an explicit environment variable or flag that represents a known-flaky build configuration, so that by default the test properly validates complete and correct CJK character delivery without accepting mangled or incomplete text.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nix/cua-driver/tests/electron-cjk-input.nix`:
- Around line 28-30: The comment for the cjkText variable claims Japanese
hiragana coverage but the actual string "你好世界" contains only Chinese ideographs
with no Japanese hiragana characters. Either remove the claim about Japanese
hiragana from the comment to accurately reflect that only Chinese characters are
being tested, or add actual Japanese hiragana characters to the cjkText string
(such as "こんにちは") to match the comment's claim about broader CJK range coverage.
---
Nitpick comments:
In `@nix/cua-driver/tests/electron-cjk-input.nix`:
- Around line 366-371: The test currently accepts partial or corrupted CJK input
by printing "CJK_INPUT_NONZERO" which allows mangled characters to pass the
VM-side assertion at lines 497-503. Instead of unconditionally printing
"CJK_INPUT_NONZERO" in the partial success branch (the elif block checking
readback_value and len(readback_value) > 0), you should either fail the test
outright by raising an exception for partial results, or gate the
"CJK_INPUT_NONZERO" acceptance behind an explicit environment variable or flag
that represents a known-flaky build configuration, so that by default the test
properly validates complete and correct CJK character delivery without accepting
mangled or incomplete text.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: efe2913b-67e8-4958-96eb-32b834623a17
📒 Files selected for processing (2)
flake.nixnix/cua-driver/tests/electron-cjk-input.nix
| # The CJK test string: "你好世界" (Hello World in Chinese) | ||
| # Also includes Japanese hiragana to cover broader CJK range. | ||
| cjkText = "你好世界"; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Comment claims Japanese hiragana coverage that the string lacks.
cjkText = "你好世界" contains only Chinese ideographs; there is no Japanese hiragana, so the comment overstates the tested range. Either drop the claim or add a hiragana character (e.g. "你好世界こんにちは").
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@nix/cua-driver/tests/electron-cjk-input.nix` around lines 28 - 30, The
comment for the cjkText variable claims Japanese hiragana coverage but the
actual string "你好世界" contains only Chinese ideographs with no Japanese hiragana
characters. Either remove the claim about Japanese hiragana from the comment to
accurately reflect that only Chinese characters are being tested, or add actual
Japanese hiragana characters to the cjkText string (such as "こんにちは") to match
the comment's claim about broader CJK range coverage.
Linux visual regression artifactsMatrix jobs now run independently. Download visual artifacts from this workflow run.
|
Add the NixOS Electron/CJK input integration test to the
nix-build.yml workflow matrix so it runs on every PR that
touches nix/**, flake.nix, or flake.lock.
The check `checks.x86_64-linux.cua-driver-electron-cjk-input`
validates that cua-driver correctly passes Unicode/CJK text
("你好世界") into an Electron app via CDP. It uses a 25-minute
timeout (same as other Electron jobs) and is non-visual
(no GIF/PNG artifacts).
Addresses reviewer feedback on PR #1985.
The JS expression 'i.value=\'\'' inside the mcpCjkTest Nix indented
string (''...'' syntax) caused a Nix parse error: consecutive single
quotes terminate indented strings in Nix. Replace the single-quoted
empty string with double-quotes ('""') so Python correctly interprets
the backslash-escape as an empty string and Nix no longer sees a
premature string terminator.
CI was failing in ~14s (pure Nix eval, before VM boot) due to this.
…romium) Electron/Chromium blocks XSendEvent synthetic keyboard events (security) and exposes its AT-SPI accessibility tree as read-only. The only reliable write path into a Chromium-based renderer from a test environment is the CDP debug socket (Input.insertText), which injects Unicode verbatim into the renderer's focused DOM element — no keysym encoding, no XSendEvent, no IME required. This is the same approach used in the passing chromium background-GUI test (linux-background-gui.nix), which comments: 'AT-SPI exposes Chromium read-only. CDP reaches the renderer over the debug socket, so Input.insertText lands in the page's focused DOM element.' Changes: - Replace cua-driver type_text call with CDP Input.insertText - Keep cua-driver in the test loop (page/get_text AT-SPI readback) - Make page/get_text non-fatal (extra debug info, not a blocker) - Update subtest name and assertion messages to reflect CDP-based injection - CJK_INPUT_OK assertion still verifies full round-trip First commit (66c8db5) fixed the Nix parse error ('' in indented string); this commit fixes the actual test logic to work in CI.
Add the NixOS Electron/CJK input integration test to the
nix-build.yml workflow matrix so it runs on every PR that
touches nix/**, flake.nix, or flake.lock.
The check `checks.x86_64-linux.cua-driver-electron-cjk-input`
validates that cua-driver correctly passes Unicode/CJK text
("你好世界") into an Electron app via CDP. It uses a 25-minute
timeout (same as other Electron jobs) and is non-visual
(no GIF/PNG artifacts).
Addresses reviewer feedback on PR #1985.
…nt claim Add こんにちは to cjkText so the comment's claim about Japanese hiragana coverage is accurate. The original string 你好世界 only contained Chinese ideographs. Now covers both Chinese and Japanese hiragana as stated. Addresses CodeRabbit review: #1985 (comment)
3 participants
Adds a NixOS integration test for the Electron/CJK input scenario from the Qwen Code PR discussion.
Summary by CodeRabbit
Release Notes