Add android-e2e-tester skill (consolidated reports, Attachments, explicit-only trigger), Fixes AB#3696527#452
Open
wzhipan wants to merge 29 commits into
Open
Add android-e2e-tester skill (consolidated reports, Attachments, explicit-only trigger), Fixes AB#3696527#452wzhipan wants to merge 29 commits into
wzhipan wants to merge 29 commits into
Conversation
Adds the android-emulator-e2e-tester skill: takes an in-development Android Auth feature and proves it works on a real emulator end to end - provisions the device, deploys the app, drives the scenario (auto-handling inputs the AI can), reads logcat to decide pass/fail, and loops fix-and-retest until it passes or escalates. Contents: SKILL.md (workflow), references/ (app-and-module-map, log-signals, troubleshooting, ui-interaction), and scripts/ (emulator, appcontrol, deviceui, authlogs). Committed on its own branch so the skill can be iterated without touching existing code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fold hard-won failures from actual E2E runs into the references so the next run doesn't rediscover them: the flights-manager-never-initialized trap (flag reads its source default), local-library publish-to-mavenLocal flow, NDK/CMake toolchain + ABI matching, wrong-keystore AADSTS50011, non-CA-approved app config AADSTS530021, WebView login-field focus/verification, Play Store install-button targeting, BACK-exits-app vs ESCAPE, special-char password typing, time-boxed sink-wait TTLs, and the broker telemetry keys (broker_app_used/request_eligible_for_broker) that are the clearest brokered pass/fail signal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The skill was emulator-only; now the device pool also includes any real device connected over adb (USB or Wi-Fi). emulator.ps1 gains: physical-device enumeration (getprop API level, GMS/Play presence, boot state), a unified 'pool' command (-Json for tooling), physical devices shown in list/status, and ensure support for -PreferPhysical (favor a connected real device), -NoPhysical (emulators only), and -Serial (pin an exact device). Real devices are never created/booted - only used if already connected and booted, and only when they meet the feature's API/Google-APIs/Play requirements. Default ensure order: running emulator -> matching real device -> boot/create AVD. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds devicelease.ps1 so parallel E2E runs don't collide on one device. A lease is a per-device JSON file (outside the repo, under ~/android-e2e-runs/.leases) recording serial, owner (agent/session id), feature, host, pid, and a heartbeat. acquire reaps abandoned leases, atomically claims a free device (create-new file wins the race), respects -MaxPoolSize, and boots/creates an emulator when the pool has no free device and the cap allows. Liveness is heartbeat-based: a lease older than -StaleMinutes (or a dead PID on the same host) is reclaimed on the next acquire/reap, so a device whose owning agent/session died is reused automatically. heartbeat/release/list/reap round out the lifecycle. Wires a Phase 2a (lease) + Phase 7 (release) into SKILL.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds an execution-model section: the parent launches ONE sub-agent to own the long device-driving run, on the SAME model as the parent (e.g. parent on Claude Opus 4.8 -> sub-agent on Claude Opus 4.8), with a self-contained prompt that must end in a PASS/FAIL/BLOCKED verdict + evidence. The parent then WAITS for that verdict (block on completion) and never reports 'done' before it - directly fixing the failure we hit where the parent returned early and the test result was never shown. A watchdog restarts a hung sub-agent (no progress within a timeout and test not done), terminates it once the result is in, caps restarts, and escalates with evidence. Only the parent talks to the user. Reinforced in Guardrails. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
New reference mocking-flights-and-segments.md plus SKILL.md wiring so a run isn't abandoned as 'blocked' when the block isn't a defect. Covers: setting feature flags (runtime override first, else flip the source default temporarily and re-publish); mocking unavailable data/dependencies at the highest-fidelity boundary (local mock server for an undeployed API, inject a not-yet-shipped response field, mock brokers, adb intent extras) matched to the real contract; and segment-testing a flow when a middle piece can't be mocked (prove up to the boundary, then feed boundary inputs and prove the rest). Enforces a revert discipline: temp code changes stay uncommitted, carry a TODO REVERT marker, and are reverted with a clean-tree check. Softens the 'implementation incomplete' blocker to try mock/segment first, and reports what was real vs mocked. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
deviceui.ps1 gains finger-status (best-effort enrollment detection, incl. the modern dumpsys JSON prints/count shape) and finger-enroll (emulator: set a screen-lock PIN, launch the enroll flow, drive it with repeated emu finger touches; if it can't confirm, print clear manual steps and exit 5 to prompt the user). finger (touch) now detects emulator vs real device: it simulates a touch on emulators and, on a real device where emu finger can't help, prompts the user to press the physical sensor. Verified on an emulator: status detection returns no->(after) and the enroll path degrades to the user prompt when the wizard can't be fully driven. Docs updated in ui-interaction.md and the SKILL.md script table. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The lessons captured in fc7b556 mixed generic patterns with details specific to one feature under test (MAM broker-install-resume). Per review, the skill should stay generic; feature-specific steps belong in a test case, not the skill. Removed: the sink-wait/TTL 'parked request / resume path' guidance and the Company-Portal/MAM framing in the Play Store and AADSTS530021 sections (generalized to 'installing an app mid-flow' and 'an unapproved app config blocks the flow'). Also generalized a devicelease example (-Feature mam-resume -> signin-e2e). Kept the genuinely generic fixes: publish-to-mavenLocal, NDK/CMake, AADSTS50011 signing, flights-manager-not-initialized, WebView field focus/verify, BACK-vs-ESCAPE, special-char passwords, Play Store Install-button targeting, and the broker telemetry keys. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Establishes where feature-specific steps come from so the skill stays generic. Phase 1 now lists an ADO Test Case work item and a known test-steps file as first-class sources, with a concrete fetch (az boards work-item show ... --fields Microsoft.VSTS.TCM.Steps) + parse (each <step> = action + expected result -> success criterion), and notes the test-planner -> ADO -> this-skill loop. Adds an explicit principle: anything specific to one feature (app config/client id, account, broker, exact tap sequence, expected markers) belongs in the test case, never hardcoded in the skill or its scripts; read it at run time. Frontmatter description updated to match. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Addresses two real pain points from a Cloud PC run: (1) very slow emulator, (2) emulator not visible in Android Studio's Running Devices. emulator.ps1 now profiles the host and picks fast, correct flags: auto -gpu (host GPU when a real GPU exists, else SwiftShader software), auto -cores/-memory sized from the host (e.g. 6/6144 on a big box vs a stock AVD's 4/1536), with -Gpu/-Cores/-Memory overrides. It detects GPU-less VM/RDP/Cloud-PC hosts and, since a software-rendered emulator is painfully slow there, auto-prefers a connected physical device in ensure (override -NoPhysical) and prints a clear perf note. resolve-sdk now reports the host GPU/perf profile. New reference emulator-performance.md explains why the emulator is slow (no GPU on Cloud PC/VM/RDP -> SwiftShader), the physical-device fast path, emulator tuning, slow Play Store/NAT downloads, and Android Studio integration: AVDs already appear in Device Manager (shared ~/.android/avd); to see one in Running Devices, start it from Studio and let the skill reuse the running emulator. Wired into SKILL.md Phase 2 + troubleshooting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…PI + blocker refs Rework the E2E testing skill based on lessons from the AAD MFA sign-in run: - Rename skill android-emulator-e2e-tester -> android-e2e-tester (real device or emulator). - Make an HTML+Markdown test report MANDATORY for ADO test cases on every outcome; add scripts/report.ps1 and references/test-reporting.md. - Fix non-test-specific blockers: deviceui.ps1 input-text gains -Clear/-CharByChar/-PerCharDelayMs/-Secret to beat the Chrome autofill/passkey overlay; add scripts/labapi.ps1 (create-user/reset/enable-policy/disable-policy/delete-device) via the EasyAuth WAM-SSO workaround. - Add references/common-blockers.md (emulator-vs-device decision, fingerprint/App-Lock, number-match MFA, FLAG_SECURE, session timeouts) and references/lab-api.md (endpoints, usertypes/policies, auth, entitlements). - Add references/run-speed.md analyzing per-step latency (fixed sleeps, per-call process startup, redundant dumps/screenshots) with a faster wait-text/batch-per-screen recipe. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ulk-first typing) Section 5 of the skill-improvement request was documented in run-speed.md as analysis only; this bakes those speed-ups into the scripts and prescribed workflow. deviceui.ps1: - Add shared Wait-ForText helper: immediate first check, then poll every -PollMs (default 600ms) until the -TimeoutSec deadline; returns the instant the anchor appears instead of paying a fixed sleep. - wait-text now uses it (was a hardcoded Start-Sleep -Seconds 2 loop -> reaching a 3rd probe drops from ~4s to ~1.2s). - tap-text / tap-desc gain -Then <anchor>: tap AND wait for the next screen in ONE process (removes the per-tap process/adb startup tax and the fixed-sleep tax), reusing wait-text's exit-4 timeout semantics. - New params -Then / -PollMs; doc header documents them and the bulk-first typing rule. SKILL.md + references/ui-interaction.md: replace the slow loop (act -> reflexive re-dump -> screenshot every step; unconditional -CharByChar) with the fast path - dump once per screen, verify by the next anchor via tap-text -Then, bulk input-text first (add -CharByChar only if it didn't land), screenshot milestones only and skip FLAG_SECURE screens. references/run-speed.md: point the recommendations + fast-path recipe at the now- shipped -Then / -PollMs mechanism. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…, reference enrichments) Fixes real non-test-specific blockers surfaced while running ADO test case 1579381 (AAD MFA cloud-account registration), which finished PARTIAL: registration/pairing PASS on a physical device, but the final biometric-gated number-match verification was BLOCKED by the environment (Authenticator App Lock + unknown device PIN + an emulator that is unusable on this host). labapi.ps1: add a DevTools-protocol (CDP) fallback for the Edge 150 --dump-dom regression (returns 0 bytes). Launches headless Edge with --remote-debugging-port, finds the real http(s) page target, and reads document.body.innerText over a WebSocket. Pure PowerShell; no Node/npm. Validated live (create-user). Encoding: save all non-ASCII scripts (report/deviceui/emulator/devicelease/labapi.ps1) as UTF-8 with BOM so they parse under Windows PowerShell 5.1 (5.1 decodes BOM-less .ps1 as CP1252 and mojibakes em-dashes). Verified parsing under both PS 5.1 and pwsh 7. References: troubleshooting.md (emulator GPU/RAM cold-boot recipe + Bluetooth-HAL/system_server instability abandon signal; arm64-vs-x86_64 install-dexopt crash -> use universal APK; new UTF-8 BOM section). common-blockers.md (non-destructive locksettings verify PIN probe + Gatekeeper throttle caution; PIL blue-pixel detection for WebView pairing links; emulator-unavailable caveat; quick-ref rows). lab-api.md (document the automatic CDP fallback). emulator-performance.md (matching-ABI/universal-APK + when to abandon the emulator). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…PINs never hit the transcript Add a leak-proof way to hand the android-e2e-tester skill test-account passwords, device lock-screen PINs, and keystore passwords, plus a convention for APK/test-file handoff. - scripts/secrets.ps1 (new): DPAPI-encrypted, per-user/per-machine local secret store. Commands: set (masked Read-Host, never echoed), list/test/get-masked (safe -- names/length/asterisks only), remove, path. No command ever prints a stored value. Exposes Resolve-Secret. ASCII-only (no BOM); parses & runs under Windows PowerShell 5.1 and pwsh 7. - scripts/deviceui.ps1: add `-SecretRef <name>` to input-text (resolves from the store, implies -Secret) and a new `unlock` command that types a stored PIN to clear the keyguard. Inline Resolve-SecretValue mirrors secrets.ps1 (env var E2E_SECRET_<NAME> first, then the DPAPI file) so no dot-source scope clobber. - references/secrets-and-files.md (new): secret workflow, env-var alternative, DPAPI at-rest notes, and the APK/file drop-folder convention (paths aren't secret; only file passwords go in the store). - SKILL.md: scripts table (secrets.ps1 row, deviceui unlock/-SecretRef), credential-typing block prefers -SecretRef, References table + Guardrails point at the new doc. Verified: 12-assertion secret round-trip (encrypt->resolve->env-override->mask-> remove->missing-throws) passes under 5.1 and 7 with no value on any stream; all markdown links/anchors resolve; both scripts parse. Skill/tooling + docs only -- no product code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
….ps1 fetch-password) Add a 'fetch-password' command to labapi.ps1 that reads a lab tenant's shared password directly from the msidlabs Key Vault via the operator's signed-in Azure CLI identity and caches it DPAPI-encrypted in the local secret store (default name 'labpw'). The plaintext is never printed, logged, or committed -- only a masked length is shown -- so the user no longer needs to paste lab passwords into chat; the existing 'deviceui.ps1 input-text -SecretRef labpw' path just works. Docs: document the KV-fetch workflow in references/lab-api.md and references/secrets-and-files.md, correct the old 'user supplies the password' guidance, and wire pointers into SKILL.md (scripts table, credential-typing block, References table, Guardrails). Verified: fetch-password round-trips under PS 5.1 and pwsh 7 with no secret on any stream; labapi.ps1 parses on both and keeps its UTF-8 BOM; markdown links/anchors resolve; leak scan clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e/biometric docs deviceui.ps1 `unlock` now verifies the keyguard actually cleared (new read-only Test-KeyguardLocked helper) and stops after -MaxAttempts (default 3), exiting 3 rather than risking an escalating Gatekeeper lockout on a physical device. It short-circuits when already unlocked and still prints only the PIN length. Docs (SKILL.md, common-blockers.md, secrets-and-files.md) add multi-device targeting via unique adb serials/-Serial and clarify PIN-vs-biometric fallback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…r) + unlock auto-resolves devicepin_<serial> - secrets.ps1: new `set-device-pin` command. Enumerates adb devices, auto-selects when one is connected or shows a numbered picker (serial + model) when several are, or targets one directly with -Serial. Prompts for the PIN twice (hidden) and compares them in memory to catch a typo before it wastes the unlock 3-try lockout budget, then DPAPI-encrypts it under devicepin_<serial> (or -Name override). PIN is never printed. - deviceui.ps1: `unlock -Serial <serial>` now auto-resolves the per-device secret devicepin_<serial> when neither -SecretRef nor an explicit -Pin is given, so the serial's PIN name never has to be typed twice. -SecretRef still takes precedence; -Pin remains the throwaway-emulator escape hatch. Adds Test-SecretExists (checks env/file without decrypting) + Get-DevicePinName (shared naming convention). - docs: secrets-and-files.md, common-blockers.md, SKILL.md updated to feature set-device-pin + the unlock auto-resolve (TL;DR, quick-reference tables, examples). Verified: both scripts parse under Windows PowerShell 5.1 and pwsh 7; live helper tests (device enumeration, name derivation, secure-string compare, numbered-menu choice/retry/3-try-abort) pass; no PIN plaintext reaches stdout; all 12 markdown files' links/anchors resolve. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ference=Stop Some Edge builds print benign renderer messages to stderr during the headless --dump-dom launch. With \Continue='Stop' (set at script scope), that stderr surfaced as a terminating NativeCommandError even with 2>\, aborting create-user (exit 1) before the CDP fallback could run. Relax the preference to 'Continue' only around the native Edge invocation and restore it in finally, so provisioning succeeds across Edge versions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…er-char typing
A word-initial '#' passed as a lone `adb shell input text #` is parsed by the
device shell as a comment, yielding an empty argument and an
IllegalArgumentException ("Argument expected after text"), which silently
DROPS that character. This only surfaced in -CharByChar mode (bulk typing keeps
'#' mid-word/literal). Add '#' to the backslash-escape set so it is sent as \#
in both modes. Verified on-device while running WPJ suite 3690852.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…account policy
Five enhancements distilled from the WPJ suite run (planId 3690835):
1. Suite/overall report: report.ps1 gains a `summary` command that recurses a
batch folder for per-case run.json and emits SUMMARY.html + SUMMARY.md
(problems-first table, overall PASS/FAIL/PARTIAL + counts, links to each
TestReport). Wired into SKILL Phase 7, scripts table, and test-reporting.md.
2. Parallel multi-device: new SKILL "Running multiple test cases (batch)"
section + guardrail — split cases across leased devices, one sub-agent per
case, shared batch folder with tc<id> subfolders, then roll up with summary.
3. Clean-state / no-teardown: Phase 3 now uninstalls+reinstalls test apps at
case START (pm clear does not remove work accounts/registrations), and
Phase 7 + a guardrail say do NOT tear down accounts/registrations after a
case. common-blockers "Stale account state" and lab-api delete-device
guidance updated to match.
4. Known provided-APK map: app-and-module-map.md gains a "Known provided APKs"
section (filename -> app/package/role) plus the ECS-vs-Local staging
convention; Phase 3 links it.
5a. Settings-first: common-blockers.md "Doing it yourself in System Settings"
— drive Settings via `am start -a android.settings.*` + uiautomator before
declaring BLOCKED (clock-advance recipe; wall clock needs no root). Corrects
the earlier "advance clock needs root" framing. SKILL Phase 4 + when-to-ask.
5b. Account policy: lab-api.md "Account policy" section — prefer a fresh
ID4SLAB2 temp user per case (capability -> usertype map) even when a case
names a deprecated MSIDLAB4 account. SKILL Phase 4 + guardrail.
Verified: all 8 scripts parse under PowerShell 7; report.ps1 summary produces
correct output against a real batch folder; the two new anchors resolve and no
intra-skill links break; no leftover old skill-name references.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… else ECS) Encode ADO test-point/configuration handling into the android-e2e-tester skill. A Broker-suite test case can have 1-2 test points, each with a configuration; the configuration decides which staged build folder the app-under-test comes from, and the case runs once per test point: * configuration name contains "LocalFlights" -> install from Local\ * any other configuration (plain "RC MSAL - RC Broker") -> install from ECS\ This mapping is counter-intuitive (LocalFlights does NOT mean ECS) and is flagged loudly wherever it appears. Changes: - scripts/report.ps1: thread ado.testPointId/configuration/buildSource through the run schema; summary adds a Config column (MD+HTML) with ECS-before-Local tertiary sort; single render shows Config/Build in the ADO meta row. - SKILL.md: new Phase 1 "Test points and configurations (which build to run)" with the verified Test Plan TestPoint REST API enumeration recipe and the LocalFlights->Local / plain->ECS rule; batch section treats each (case,point) as its own lane with tc<id>-<local|ecs> subfolders; Phase 3 install folder = the point's configuration; Phase 7 records configuration/buildSource; new guardrail bullet. - references/app-and-module-map.md: replace the stale "test ECS by default" paragraph with a config-driven "ECS vs Local builds (test-point configuration)" table + counter-intuitive warning. - references/test-reporting.md: ado schema gains testPointId/configuration/ buildSource; per-test-point subfolder layout; Config column note; worked example updated. Verified: all 8 skill scripts parse under pwsh7; synthetic 2-point suite renders the Config column and problems-first ordering; single render carries Config/Build; all markdown links + the two new cross-anchors resolve. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ing and additional scope details
- Updated JSON schema to allow for a `testPoints` array, enabling multiple test points to be reported in a single case. - Modified report rendering logic to handle both single and multiple test points, ensuring backward compatibility. - Improved summary generation to reflect individual test point outcomes, linking to consolidated case reports. - Enhanced markdown and HTML output to include detailed sections for each test point, along with a shared proposed test steps section. - Updated documentation within the script to reflect new features and usage examples.
Change the skill's trigger (frontmatter description + 'When this runs') so it only starts when the user explicitly asks to run an E2E test. Removes the auto-invocation path (after a feature finishes development, when a coding-agent PR lands, or as an implicit step of another workflow). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
❌ Work item link check failed. Description does not contain AB#{ID}. Click here to Learn more. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new VS Code skill, android-e2e-tester, providing an end-to-end Android Auth test-automation workflow (device pool + leasing, UI driving via adb/uiautomator, LAB account provisioning, logcat-based verdicting, and mandatory HTML/Markdown reporting for ADO-driven runs).
Changes:
- Adds PowerShell tooling to manage devices (emulators + adb-connected physical devices), lease devices for parallelism, build/install apps, drive UI interactions, and capture/scan auth logs.
- Adds report rendering (
TestReport.html/TestReport.md) including consolidated multi-test-point case reporting plus an overall batchSUMMARY. - Adds detailed reference docs covering UI driving strategy, common blockers, lab API usage, performance/troubleshooting guidance, and reporting schema/rules.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/skills/android-e2e-tester/SKILL.md | Skill definition and end-to-end workflow/phases, including explicit-only trigger guidance |
| .github/skills/android-e2e-tester/scripts/appcontrol.ps1 | Build/install/launch/reset helpers for the app-under-test |
| .github/skills/android-e2e-tester/scripts/authlogs.ps1 | Logcat capture + heuristic PASS/FAIL classification and evidence extraction |
| .github/skills/android-e2e-tester/scripts/devicelease.ps1 | Device leasing to prevent concurrent runs colliding on the same device |
| .github/skills/android-e2e-tester/scripts/deviceui.ps1 | UI automation helpers (dump/find/tap/type/unlock/fingerprint/screenshot) via adb + uiautomator |
| .github/skills/android-e2e-tester/scripts/emulator.ps1 | SDK resolution + emulator/physical-device pool enumeration + AVD creation/boot/ensure |
| .github/skills/android-e2e-tester/scripts/labapi.ps1 | LAB user-manager API helper + Key Vault password fetch into DPAPI secret store |
| .github/skills/android-e2e-tester/scripts/report.ps1 | Report rendering for per-case runs and batch summaries (HTML + Markdown) |
| .github/skills/android-e2e-tester/scripts/secrets.ps1 | Local DPAPI-encrypted secret store CLI (no plaintext printing) |
| .github/skills/android-e2e-tester/references/app-and-module-map.md | Mapping from feature area/module to which app(s) to build/install/drive |
| .github/skills/android-e2e-tester/references/common-blockers.md | Known blockers/hiccups and emulator-vs-physical decision guidance |
| .github/skills/android-e2e-tester/references/emulator-performance.md | Host GPU/VM considerations and emulator performance best practices |
| .github/skills/android-e2e-tester/references/lab-api.md | LAB API authentication model, endpoints, and account policy guidance |
| .github/skills/android-e2e-tester/references/log-signals.md | Log interpretation guidance and pass/fail signal taxonomy |
| .github/skills/android-e2e-tester/references/mocking-flights-and-segments.md | Guidance for flag/mocking/segment-testing when full E2E is blocked |
| .github/skills/android-e2e-tester/references/run-speed.md | Analysis and recommendations to reduce harness overhead and improve run time |
| .github/skills/android-e2e-tester/references/secrets-and-files.md | Safe handling of secrets and APK paths without leaking into transcripts |
| .github/skills/android-e2e-tester/references/test-reporting.md | Run-JSON schema + multi-test-point reporting rules + suite summary behavior |
| .github/skills/android-e2e-tester/references/troubleshooting.md | Environment/build/install/tooling troubleshooting guide |
| .github/skills/android-e2e-tester/references/ui-interaction.md | UI selector strategy + which inputs the AI should handle vs. prompt the user |
Comments suppressed due to low confidence (3)
.github/skills/android-e2e-tester/references/secrets-and-files.md:214
- These examples use
-ApkPath, butscripts/appcontrol.ps1expects-Apk. The example commands should be updated so they can be copy/pasted successfully.
`C:\Users\zhipanwang\Downloads\APKs\app-production-arm64-v8a-release-signed.apk`.
```powershell
./scripts/appcontrol.ps1 install -ApkPath 'C:\Users\zhipanwang\Downloads\APKs\app-production-arm64-v8a-release-signed.apk' -Serial <serial>
- Newest-match in a known folder. If you'll drop a fresh build repeatedly, say "use the newest
**.github/skills/android-e2e-tester/references/secrets-and-files.md:228**
* This tip recommends `-ApkPath`, but `scripts/appcontrol.ps1` uses `-Apk` for the direct-install parameter. Keeping the flag name consistent avoids confusion.
- Only a file's associated password (e.g. a keystore password) is a secret — store that with
secrets.ps1 set -Name kspw; the keystore file itself just needs a path. - Prefer
-ApkPathover pasting file contents; never paste a binary or a large config into chat.
**.github/skills/android-e2e-tester/references/secrets-and-files.md:245**
* Quick reference still uses `-ApkPath`, but `scripts/appcontrol.ps1` expects `-Apk`. This entry should match the actual script parameter name.
| Unlock a device (auto-uses saved PIN) | deviceui.ps1 unlock -Serial <serial> |
| Env-var alternative | set E2E_SECRET_LABPW, then -SecretRef labpw |
| Install an APK | appcontrol.ps1 install -ApkPath <path> |
</details>
| } | ||
|
|
||
| $adb = Get-Adb | ||
| function Wait-Booted { param([string]$S) & $adb -s $S wait-for-device 2>$null; $b = (& $adb -s $S shell getprop sys.boot_completed 2>$null | Out-String).Trim(); return ($b -eq '1') } |
Comment on lines
+289
to
+293
| $m = @(Find-ByField 'Desc' $Text) | ||
| if (-not $m) { Write-Host "NOT FOUND (content-desc): '$Text'"; exit 2 } | ||
| $t = $m[[Math]::Min($Index, $m.Count - 1)] | ||
| Adb shell input tap $t.Cx $t.Cy | Out-Null | ||
| Write-Host "Tapped desc '$Text' at ($($t.Cx),$($t.Cy))" |
| | A lab/account **password** (any other) | `scripts/secrets.ps1 set -Name labpw` (type it at the masked prompt) | `deviceui.ps1 input-text -SecretRef labpw` | | ||
| | A device **lock-screen PIN** | `scripts/secrets.ps1 set-device-pin` (picks the device, then type it at the masked prompt) | `deviceui.ps1 unlock -Serial <serial>` (auto-uses the saved PIN) | | ||
| | A **keystore** password | `scripts/secrets.ps1 set -Name kspw` | build step reads `E2E_SECRET_KSPW` / `-SecretRef kspw` | | ||
| | An **APK** or other file | Drop it in a folder, give the **path** (paths aren't secret) | `appcontrol.ps1 install -ApkPath <path>` | |
|
❌ Invalid work item number: AB#3696527 ##. Work item number must be a valid integer. Click here to learn more. |
|
✅ Work item link check complete. Description contains link AB#3696527 to an Azure Boards work item. |
github-actions
Bot
changed the title
github-actions
Bot
changed the title
cacosta33
reviewed
Jul 24, 2026
| @@ -0,0 +1,571 @@ | |||
| --- | |||
| name: android-e2e-tester | |||
| description: "Execute and iterate end-to-end (E2E) tests for an in-development Android Auth feature (MSAL, Broker, Common, ADAL, Authenticator) on an Android emulator or a connected real device. Builds a device pool from emulators and any adb-connected hardware, finds or creates a suitable AVD (or reuses a running emulator / real device), leases the device so concurrent tests don't collide, builds/installs the right test app, drives the UI, and verifies via logcat. Delegates the actual run to a sub-agent (same model as the parent) and waits for its verdict; when several test cases and several devices are available, splits the cases across devices and runs them in parallel (one sub-agent per case), then writes a per-case report plus an overall run summary. Only use this skill when the user explicitly asks to run an end-to-end test — e.g. 'run the E2E test', 'test this feature end to end', 'verify the feature on a device or emulator', 'does this work on a device', 'run this ADO test case', or 'try the sign-in flow'. Do NOT auto-start it after a feature finishes development, when a coding-agent PR lands, or as an implicit step of any other workflow; a human has to ask for an E2E run. Discovers what to test from user-provided steps, an ADO Test Case work item, a known test-steps file, the session's design spec, or the implementation diff — feature-specific steps live in the test case, not the skill. Auto-performs inputs the AI can handle (typing lab test credentials, tapping buttons, granting permissions, simulating a fingerprint, provisioning lab accounts via the LAB API), mocks unavailable dependencies and sets feature flags via temporary code changes when needed, and asks the user when the intent is unclear or a step is a real blocker (push MFA, hardware, missing credentials, implementation gaps). Checks logs to decide pass/fail, drives a fix-and-retest loop until it passes, and — for ADO test cases — always writes an HTML + Markdown test report at the end. Prefers an emulator when a step needs an injectable fingerprint/biometric (App Lock, number-match with biometric gate)." | |||
Contributor
There was a problem hiding this comment.
🟡 Suggestion: this description is 2,098 characters — about 4.5× every other skill in the repo (next largest is copilot-review-analyst at 607; the median across the other 17 is ~470).
Because description is loaded for every skill-selection decision in every session, an outlier this large is a permanent context cost paid by all sessions, not just E2E ones. It can also work against itself: the explicit-only guard ("do NOT auto-start after a feature finishes development…") is the highest-value sentence here, and it's currently surrounded by a lot of prose that reads more like body content.
Suggest trimming to ~2–4 sentences — what the skill does, plus the explicit-only guard — and moving the rest into the body under ## When this runs, which already covers this ground.
Comment on lines
+144
to
+158
| $prof = if ($Fresh) { Join-Path $env:TEMP ("labapi_cdp_" + [guid]::NewGuid().ToString('N')) } | ||
| else { Join-Path $env:TEMP 'labapi_edge_cdp_profile' } | ||
| $port = Get-Random -Minimum 9300 -Maximum 9599 | ||
| $proc = Start-Process -FilePath $edge -PassThru -WindowStyle Hidden -ArgumentList @( | ||
| '--headless=new', '--disable-gpu', '--disable-sync', | ||
| "--remote-debugging-port=$port", "--user-data-dir=$prof", | ||
| '--no-first-run', '--no-default-browser-check', $Endpoint) | ||
| try { | ||
| $deadline = (Get-Date).AddSeconds([Math]::Max(12, $TimeoutSec)) | ||
| $wsUrl = $null | ||
| while ((Get-Date) -lt $deadline -and -not $wsUrl) { | ||
| Start-Sleep -Milliseconds 400 | ||
| try { $targets = Invoke-RestMethod "http://127.0.0.1:$port/json/list" -TimeoutSec 3 } catch { continue } | ||
| # Only the real content page (http/https); skip edge:// dialogs (e.g. sync-confirmation). | ||
| $page = $targets | Where-Object { $_.type -eq 'page' -and $_.url -like 'http*' } | Select-Object -First 1 |
Contributor
There was a problem hiding this comment.
🟡 Suggestion: the headless-Edge/CDP path isn't safe under concurrency, which sits awkwardly with parallel multi-device execution being a headline feature of the skill.
Three things compound when two labapi calls overlap:
- Shared profile dir by default — L144–145 uses a fixed
$env:TEMP\labapi_edge_cdp_profileunless-Fresh. Two concurrent Edge launches against the same--user-data-dirgenerally don't both start; the second hands off to the first and exits. - Random port, no bind check — L146 draws from a 299-port range, and nothing verifies Edge actually bound that port.
- No target validation — L158 takes the first
type -eq 'page'with anhttp*URL and never checks it corresponds to$Endpoint. If it attaches to another instance, the call silently returns that instance's page content as this call's API response.
Stop-Process in the finally (L172) can then also tear down a browser the other run is mid-eval on.
The silent-wrong-answer case is the dangerous one. Two small changes remove it: always use a unique profile dir (drop the shared-dir branch, or default -Fresh on), and assert $page.url starts with $Endpoint before taking webSocketDebuggerUrl.
Comment on lines
+226
to
+234
| function Show-Result { | ||
| param($Result) | ||
| if ($Result.Json) { | ||
| $Result.Json | ConvertTo-Json -Depth 8 | ||
| } | ||
| else { | ||
| Write-Host $Result.Raw | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🟡 Suggestion: Show-Result does $Result.Json | ConvertTo-Json -Depth 8 and prints the whole response, and it's used by create-user, reset, enable-policy, disable-policy, and delete-device.
To be clear, today's responses are safe — references/lab-api.md:142–152 shows create-user returns passwordUri / credentialVaultKeyName, i.e. URIs and key names, not values. The concern is structural: the skill's stated invariant is that passwords never reach the chat transcript, and right now that invariant depends on an external service's response shape never gaining a password/secret field.
A ~3-line key filter (password|secret|token|key) before the dump would make the guarantee hold by construction rather than by coincidence.
Comment on lines
+254
to
+263
| function Save-DpapiSecret { | ||
| # Cache a plaintext into the local DPAPI store in the exact format scripts/secrets.ps1 uses, so it | ||
| # resolves via `-SecretRef <Name>`. Nothing is printed here; the caller emits only a masked summary. | ||
| param([Parameter(Mandatory)][string]$Name, [Parameter(Mandatory)][string]$Value) | ||
| $dir = Join-Path $env:USERPROFILE '.android-e2e-secrets' | ||
| if (-not (Test-Path $dir)) { New-Item -ItemType Directory -Force -Path $dir | Out-Null } | ||
| $ss = ConvertTo-SecureString $Value -AsPlainText -Force | ||
| $enc = ConvertFrom-SecureString $ss # DPAPI encrypt (per-user, per-machine) | ||
| Set-Content -Path (Join-Path $dir ("{0}.sec" -f $Name)) -Value $enc -NoNewline -Encoding ASCII | ||
| } |
Contributor
There was a problem hiding this comment.
🟢 Nit: Save-DpapiSecret re-implements the secrets.ps1 store rather than reusing it — it hardcodes '.android-e2e-secrets' a second time (the first is secrets.ps1:80 Get-StoreDir) and re-derives the .sec / ConvertFrom-SecureString format.
The comment says "in the exact format scripts/secrets.ps1 uses", which names the coupling without enforcing it. Worth noting secrets.ps1:196 already has a dot-source guard (if ($MyInvocation.InvocationName -ne '.')) specifically so its functions can be reused, so dot-sourcing looks like the intended pattern here.
Low impact today; the risk is silent drift if the store path or encoding ever changes on only one side.
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
Adds the
android-e2e-testerVS Code skill — an end-to-end test-automation workflow for Android Auth features (MSAL, Broker, Common, ADAL, Authenticator) on emulators and real devices, driven from Azure DevOps test cases. This branch also contains the latest refinements to the skill's reporting and triggering.The PR is scoped entirely to
.github/skills/android-e2e-tester/(20 files, additions only against the merge base).Work item: AB#3696527
Use cases
LocalFlights→ Local and plain configs → ECS builds, so a feature is checked against both flighting states in one go.What the skill does
Recent refinements included here
testPoints[]), a derived overall verdict, and a suite summary that expands points into one row each. Backward compatible with the flat single-point run.json.Sample test report
A consolidated report from a sample run of the "Cross-Apps SSO with System Browser" ADO case (TC 497038) across both test points (ECS + Local/LocalFlights builds) → PASS, with per-test-point sections, evidence, and screenshots:
Notes
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com