Repository navigation
[UI Tests] PowerOCR new UI tests - #51933
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Gleb Khmyznikov (khmyznikov)
left a comment
There was a problem hiding this comment.
AI-assisted review (iteration 1 @ 6c732ac)
The port is a faithful 10 to 10 migration and follows the ui-tests-migration conventions: .Next project next to the legacy suite, PerMonitorV2 manifest, tests\ output path, warnings-as-errors inherited, ClipboardHelper, retried hotkey activation, and a positively validated negative check. Solution placement, resolver auto-discovery, companion signing, and non-elevated dispatch all look right.
The main concerns are in CI. The new language-provisioning step can block every UI suite in the job, and it changes agent user state without restoring it. On the test side, the toolbar test lost its real-click coverage and the clipboard cleanup has an empty-clipboard bug.
| # | Severity | Location | Finding |
|---|---|---|---|
| 1 | high | .pipelines/v2/templates/job-test-project.yml:504-506 |
Provisioning step is a hard gate: a FOD failure or timeout (~76 of 90 min cold) blocks all UI suites in the job, including full runs |
| 2 | medium | src/modules/PowerOCR/PowerOCR.UITests.Next/PowerOCRTests.cs:128-131 |
An empty clipboard is never restored (SetText("") throws; the helper retries 9 times and logs "clipboard locked") |
| 3 | medium | src/modules/PowerOCR/PowerOCR.UITests.Next/PowerOCRTests.cs:265-270 |
ToolbarModesTest uses UIA Invoke instead of real clicks, so the toolbar exclusion-rect input path is no longer covered |
| 4 | low | src/modules/PowerOCR/PowerOCR.UITests.Next/PowerOCRTests.cs:473-483 |
Dropdown is read in a single snapshot (legacy polled), and visible-subset indices can differ from product indices |
| 5 | low | src/modules/PowerOCR/PowerOCR.UITests.Next/PowerOCRTests.cs:383-385 |
Deterministic fixture but non-empty-only assertion; centering on VirtualScreen breaks on multi-monitor setups |
| 6 | low | src/modules/PowerOCR/PowerOCR.UITests.Next/PowerOCRTests.cs:665-667 |
Cross-thread fixture.Handle read throws under a debugger |
| 7 | low | src/modules/PowerOCR/PowerOCR.UITests.Next/PowerOCRTests.cs:31 |
PreferredLanguage: "en-US" never matches (the module stores and compares NativeName/DisplayName) |
| 8 | medium | .pipelines/v2/templates/job-test-project.yml:478-484 |
en-GB language and UK keyboard are added to the agent user and never restored (no PowerToysUiTestState snapshot) |
| 9 | low | .pipelines/v2/templates/job-test-project.yml:386 |
Pester parity: the signing and non-elevated test lists aren't updated, and the provisioning gate has no test |
| 10 | low | doc/devdocs/modules/textextractor.md:20 |
Doc: two claims not backed by the code, and local setup steps for the second OCR language are missing |
| 11 | info | src/modules/PowerOCR/PowerOCR.UITests.Next/PowerOCRTests.cs:289-294 |
Restore the rationale comments for the interior rectangle and MoveBy(1, 0) |
Validation performed for this review
- Pester 3.4.0 at head on the template-parsing suites (
uiTestSigningSelection,resolveUiTestModules,umdfHidRouting,mwbUiTestFlow,configureLightSwitchLocation): 149 tests, 144 passed. The 5 failures are all inconfigureLightSwitchLocation.Tests.ps1and reproduce identically on base0b1dd814, so they are pre-existing and unrelated (that file isn't in the CI Pester set). - .NET 10.0.14 probes:
Clipboard.SetText("")throwsArgumentNullException. A cross-threadControl.Handleread withCheckForIllegalCrossThreadCalls=truethrowsInvalidOperationException. - Local OCR enumeration:
OcrEngine.AvailableRecognizerLanguagesreturneden-US, ruwhileGet-WinUserLanguageListreturned onlyen-US. The en-GB default input tip is0809:00000809(UK keyboard). - Microsoft's FOD docs say
Language.OCRdepends onLanguage.Basicof the same language, so installingLanguage.Basic~~~en-GBis required and isn't flagged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
AI-generated summaryAddressed all 11 review findings in Validation:
Language setup no longer modifies agent language/keyboard preferences. Its failure path explicitly blocks only PowerOCR while allowing other selected suites to execute. |
✅ PR intakeAll automated intake checks now pass. Thanks for the updates! Automated PR intake; PowerToys maintainers make final decisions. |
Resolves the conflicts with the PowerOCR UI tests (#51933): both suites stay in the non-elevated lists. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Summary of the Pull Request
Migration of Text Extractor UI tests
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed
AI-generated summary
Scope
src/modules/PowerOCR/PowerOCR.UITests.Next, usingMicrosoft.PowerToys.UITest.Nextand the repository-pinned winappcli. Leave the legacy suite unchanged; add no test cases.PowerToys.slnx, embed a PerMonitorV2 manifest, and updatedoc/devdocs/modules/textextractor.md.Verified results
Test source is included in commit
6c732ac43b9f1000cb381be808c7a206c99fa31f.No skipped or other non-passing UI outcomes, and no local evidence-export errors. CI Runner/Settings companion signing was verified in all three test jobs.
Local validation used a prebuilt runtime from source revision
92ec74126257e4cacb2754cd8b4d18cdf7ca47d2and winappcli 0.7.1. Final CI rebuilt the product and tests from the exact pushed commit above.Known setup cost: the pipeline installs missing en-GB basic/OCR capabilities online. Cold Win11 setup took approximately 76 minutes in the successful run; the setup step has a 90-minute limit.
Build commands executed
Each command below passed with exit code 0:
Full local VM commands executed
Working directory:
X:\GitHub\PowerToys. Each unfiltered suite below passed 10/10 with exit code 0:The guests were stopped before changing resource profiles. The four final runs used one identical test/product/tool payload fingerprint.