Skip to content

Commit 3d875d2

Browse files
authored
Tae conversion gates (#440)
* efftriage: map a failed run to the gotcha that explains it (MTE-5827) HARNESS-GOTCHAS is at 57 entries and CONVERSION-LESSONS ~40. Nobody recalls that mid-debug, so the same failures get re-derived on device at 1-2 cycles each with the answer already written down. This reads what the loop already produced and names the likely cause, citing the gotcha id to go read. Read-only — it never touches the tree or the device, so it can be run on every failure without risk. It also encodes the two triage rules people get wrong: scan the FIRST attempt (later attempts die on leftover state, somewhere later and more confusingly), and treat status.json as authoritative for pass/fail while the trace explains why. Eleven rules, all keyed to patterns from real runs: T0 absence assertion that can never come true (selector matching another surface), T1 arrival check satisfied behind an overlay, T2 clickAndSync reporting a landed click as failed, T3 mozEnterText's non-asserting locate, T4 ambiguous Compose match reported as not-found, T5 system overlay masking as absence, T6 long press behaving as a tap, T7 stale PageStateTracker routing through a destructive edge, T8 merged-vs-unmerged text miss, T9 arrival timeout, T10 crash rather than assertion failure. Hardened against 8 labelled failures from the search batch, each with a cause established by hand on device: b4/b6-privhist -> A39, b9b -> A40, b13 -> A3/A42, b14d -> A42, b17b -> A39, b17e -> A22, b10-searchclass -> A37. All 8 now match. Three defects were found and fixed during that exercise, which is the argument for keeping the corpus: - it confidently mis-diagnosed b14d as an arrival timeout, because the absence-assertion rule was keyed to the thrown-exception wording rather than the phrasing the trace actually logs, and the arrival rule matched the harmless pre-check every navigateToPage emits. A confidently wrong triage is worse than none. - a crash was reported as a footnote, so the one failure mode that also fools effverify printed "no rule matched". - it counted one "Started try #1" block per test as a retry, marking every passing class run as retried — the exact mistake CONVERSION-LESSONS warns about. * Stamp the toolchain: VERSION, CHANGELOG, and --version on every tool (MTE-5770) Tools and docs are versioned together, since they cite each other by gotcha/lesson id and a split version would only create skew. CalVer rather than semver: these are internal scripts read from a checkout, nobody pins them, and the only question anyone asks is "is my copy older than the thing being discussed". - VERSION at the tae-conversion root, currently 2026.08.13. - CHANGELOG.md, curated rather than a commit log: entries are changes a consumer depends on (a new gate, changed verdict semantics, changed flags), each citing its gotcha ids and Jira keys so "when did this gate appear" is answerable. - --version on all 12 tools (10 python, 2 shell), handled before any other arg parsing. The version lookup deliberately resolves symlinks. These tools are routinely invoked through another checkout's tools/ dir, where the eff* entries are symlinks into this repo; an unresolved path would look for VERSION in the wrong repo and report "unknown" in exactly the situation a staleness check exists for. That symlink layout is also how MTE-5764 stayed invisible to us while being broken for everyone else, which is the argument for the stamp. Verified: all 12 tools print a version, and effnext/effverify/efftriage/effcheck/effloop.sh still behave normally (effloop's no-arg exit 1 is the pre-existing ${1:?} behaviour). * effdoctor slice 1: the toolchain map, so the queue stops drifting (MTE-5766) The eff* tools exist once but are reachable from two checkouts — testops-tools/tae-conversion/tools and ui-test-modernization/tools, where the entries are symlinks into the former. effwatch.sh computes its queue from `dirname $0`, which does NOT resolve symlinks, so the queue it watches is a function of the path it was launched from while the tools themselves are identical either way. Drop a request in the other checkout's queue and it sits there forever: nothing consumes it, nothing errors, and the run simply never happens. That has bitten us more than once, most recently this batch, so this stops relying on anyone remembering which checkout was used this time. effdoctor reads the live process list and prints the queue to actually use. Slice 1 checks, all read-only: - the toolchain map: canonical tools + queue, and per live effwatch its pid, launch path and the queue it is therefore watching, flagged canonical or not - more than one effwatch running (two gradle builds on one device read as flakiness, MTE-5768) - the alt checkout diverging: an eff* entry that is a SEPARATE copy rather than a symlink, or missing entirely — which is how a tool silently ages out of one checkout - effpretty resolvable under $REPO (MTE-5764 / gotcha A24) - a stale mach_android.lockfile with no process holding it (gotcha A38) - an adb device present Exit 1 on anything needing attention, so it can gate the start of a session. Run against the live setup it immediately reported both real problems: effwatch was launched from ui-test-modernization so the canonical queue was the wrong place to queue (the exact failure from earlier today), and effdoctor/efftriage were missing from the alt checkout. The latter is now symlinked, and resolves to the right VERSION through the symlink, which is what the realpath-based version lookup exists for. * effdoctor: resolve the watched queue against effwatch's cwd, not ours (MTE-5766) Its first live run misdiagnosed the very thing it exists to prevent. effwatch had been launched as `./tae-conversion/tools/effwatch.sh`, and the relative path was resolved with os.path.abspath — i.e. against effdoctor's cwd. Run from the firefox repo, it reported the live queue as firefox/tae-conversion/conversion-runs/_queue, a directory that has never existed, and advised queueing into it. A request dropped there is never consumed and never errors, which is the exact failure mode this tool was written to catch. Read the watcher's own cwd via lsof and resolve against that, so the answer no longer depends on where effdoctor was invoked from. Absolute launch paths are used as-is. Two guards for the class rather than the instance: a computed queue that is not a directory on disk is a FAIL, and a watcher whose cwd cannot be read is reported as undetermined with the lsof command to run. Both refuse to name a path instead of naming a wrong one, since a confidently wrong answer sends the reader somewhere silent. Verified: correct and identical from two different cwds, and the FAIL and multi-watcher paths exercised with a decoy watcher whose computed queue does not exist. * Unit-test the conversion toolchain, and fix the four defects that found (MTE-5766, MTE-5827) The toolchain had no tests. Its verdicts are what we decide "done" on, and the way we checked them was to run a tool, read its output, and judge whether it looked right — which is how two confidently wrong diagnoses survived review. Both were in the gates written to prevent exactly that. 44 tests, stdlib unittest, no device or Firefox checkout needed: ps/lsof/adb/git are stubbed and the edge cases build synthetic batch dirs. The efftriage rule tests instead run against REAL labelled conversion runs, now checked in under tests/fixtures/corpus with a labels.json manifest. That distinction is the point: hand-written traces only exercise the shapes their author imagined, so they go green while the tool mismatches real output — a false green in the suite meant to prevent false greens. The corpus also existed only as batch dirs in one person's conversion-runs/, unversioned and impossible to run in CI, which is a poor home for the campaign's main diagnostic asset. Four defects, three of them in code already reviewed by eye and pronounced fine: - efftriage diagnosed GREEN runs. Traces normally carry non-fatal [ERR] lines (nav-graph polling, tolerated absence checks) and the rules scanned them whatever the outcome, so a passing run was reported as an absence-assertion failure. Two labelled runs in our own corpus were affected. Still triaged on purpose: a retry-pass, where attempt 1 did fail and clean=false means flaky-not-done, and a crash, since crash mode is where `outcome: pass` is itself the lie (MTE-5822). - efftriage blamed the harness for a typo: a nonexistent batch dir came back as gotcha A24, sending the reader to check effpretty resolution and pull dumps off a device. It also tracebacked. - effdoctor named the wrong queue — the failure it exists to prevent. effwatch is launched by a relative path, resolved with abspath against effdoctor's cwd instead of the watcher's, so from a Firefox checkout it reported a queue under firefox/tae-conversion/ that has never existed and told us to use it. Requests there are never consumed and never error. Now reads the watcher's own cwd (/proc, then lsof); a computed queue that is not on disk is a FAIL and an unreadable cwd is "undetermined", because declining to answer beats answering wrongly. - effdoctor's self-exclusion skipped any ps line containing the substring "effdoctor", so a watcher under a directory with that name was invisible. Unnoticed watchers are the whole point of the tool. Recorded as known gaps rather than papered over: rules T3/T4/T5 have no labelled example in the 53-run corpus, so they are unvalidated rather than known-good, and 3 of 28 failures still get no diagnosis. Both are asserted, so the lists cannot go stale in either direction. * A skip is not a pass: fix the fifth false-green shape (A44, MTE-5827, MTE-5766) Baselining a legacy test for batch 2 produced outcome=pass, failures=0 — for a test that never ran. MainMenuTest.verifyTheMoreMainMenuSummarizePageButtonTest is @ignore'd on bug 2059592, so it skipped, and effloop's verdict was `failures == 0 and tests > 0`, which a skip satisfies. efftriage then read that outcome and said "run passed — nothing to triage". effverify was the only tool that got it right (ok:false, passed:false), so the disagreement between them was the tell. A skip asserts nothing, so it must never read as green: - effloop now counts what actually ran: outcome=skipped when nothing did, partial when only some did, both exiting 5 — a new code meaning "ran, but the verdict is incomplete", distinct from 1 (failed) because the test did not fail, it never started. - efftriage rule T11 diagnoses it from the per-test statuses rather than from `outcome`, since the field that should have raised the alarm was the one lying. It names the gate candidates: an Assume()/assumeTrue, an @ignore, a Nimbus flag, a pref, absent hardware. - Documented as gotcha A44, with the standing instruction not to read failures:0 as success on its own but to confirm results[].status == "pass" for every test expected to run. Run over the existing corpus, T11 immediately found 2 silently SKIPPED tests (browserSearchBarItemsTest, homeSearchBarItemsTest) inside an earlier 10-test run that nothing had ever reported — that run was recorded as a plain failure of one other test. The labelled fixture keeps both shapes: the skip scored as pass, and the mixed run. Verified end-to-end on device, not just in unit tests: re-running the same @ignore'd test now yields outcome=skipped with effloop_exit=5. Suite is 48 tests, green with ResourceWarnings as errors. * efftriage: rules for a compile failure and a wrong-screen arrival check (T12, T13, A45) Both found while converting the first batch-2 test, and both cases where the tool sent the reader somewhere unhelpful. T12 — a compile failure was answered with "no rule matched". status.json states compiled:false outright, so there is never a reason to make someone hunt through a trace that was never produced. It now names build-report.txt, where effloop already puts the Kotlin errors with file:line. This also re-labels an existing fixture: b18-fxsuggest was recorded as an "empty report" case and drew the misleading A24 advice (check effpretty resolution, pull dumps off the device) when it had simply failed to compile. T13 — the arrival check passed on the WRONG screen. Both requiredForPage anchors for a new Settings sub-page also resolved on the parent Settings screen (the row carrying the destination's own title, plus the ubiquitous Navigate up), so mozIsOnPageNow() reported arrival and navigateToPage skipped its own click. Documented as gotcha A45, alongside A42 and the nav-graph gap it rhymes with. T5 was tightened in the same pass. Its matcher keyed on "non-application window(s) present", but the status bar is itself a SYSTEM window, so that warning is in nearly every dump and the rule fired on almost any not-found. It answered the A45 case above with "a system overlay was over the app", blaming the status bar — its first real firing was a false positive, which is exactly why it was listed as unvalidated rather than known-good. It now requires that the app NOT hold focus. T13's first draft keyed on "not found" and stole the diagnosis from six other rules; the corpus tests caught that immediately. It is now narrowed to the group-level failure line and placed last so anything more specific wins first — a reminder that a new rule's blast radius needs checking against the whole corpus, not just the case that motivated it. * docs: gotcha A46, a Compose toggle whose state is selected on the merged row Cost a cycle on the never-translate conversion, and it is the reason legacy tests fall back to positional UiSelector().index(n) for checked state: in the unmerged a11y tree the stateful row has no text, content description or resource-id. Two traps stack — the Switch is blanked with clearAndSetSemantics so no ToggleableState exists at all (assert selected, not checked), and the state lives on the merging row, so COMPOSE_BY_TEXT resolves a stateless descendant. Also records that a surviving testTag is not evidence of assertable state: RadioButtonListItem tags the icon before the clearAndSetSemantics that strips selected/role/onClick. * efftriage: rule T14 for a checked-state assertion on the wrong node (A46, MTE-5827) The never-translate conversion's real failure — mozVerifyElementIsChecked against a node that carries no state — came back as "no rule matched". It now names gotcha A46 and both halves of the fix: assert `selected` rather than `checked`, and match on the MERGED tree. This is the shape behind every legacy test that asserts checked state via positional UiSelector().index(n), so it will come up again. The first draft over-matched. Keying on "' is (not )?checked" also hit the `[CMD] Verifying ... is checked...` and `[OK] ... is checked` lines of runs that PASSED that assertion and failed for unrelated reasons, and it silently stole the diagnosis from two labelled gap cases (a keyboard-close PerformException and the ads-client NoHandle crash). The corpus tripwire caught it immediately — the second time in this batch that a new rule's blast radius was wider than the case that motivated it, so the matcher now requires the [ERR] marker. Fixture keeps the run as labelled evidence, including the detail that it predates the ScreenDump those verbs now emit, which is what made it opaque in the first place. * changelog: correct the rule and test counts Both were consumer-facing and stale: 11 rules -> 15 (T0-T14), 44 tests -> 48. Also notes that only 12 of the 15 are trace-matching entries in RULES, since T10/T11/T12 are status-based checks in triage() and anyone counting RULES would otherwise think three were missing. * effbug: let update post a comment, so a filed bug can be corrected (MTE-5763) I filed a conversion bug with a TestRail id I had transcribed wrong. Comment 0 cannot be edited through the Bugzilla API, and update only whitelisted summary/status/keywords/whiteboard/priority/severity plus relations and assignee — so there was no way to annotate the bug at all, and the error would have stood in a public bug indefinitely. On create, `comment` remains the description; on update it is now a new comment. Blank is ignored rather than posting an empty comment, and still counts as "nothing to change". Adds effbug_tests.py, which stubs the one function that talks to Bugzilla and asserts what would be sent. Worth pinning precisely because a wrong field writes to a public bug and there is no undo: it also covers the existing rule that relations are wrapped as {"add": [...]} rather than sent as a bare list, which would REPLACE the set and silently drop every other bug a meta bug tracks. * efftriage: point at the in-tree screen-dump tool when a selector does not resolve (MTE-5827) EffScreenDumpRunner already exists in ui/efficiency/devtools and does exactly what is needed when a selector fails — an on-demand dump of a screen's live Compose tree, so selectors are authored against ground truth rather than inferred from legacy robots whose locators are often stale. It is easy to miss. Not knowing it was there, I spent several device cycles provoking failures with throwaway assertions purely to obtain a dump. efftriage now surfaces it, with the queue invocation, whenever the report contains a "not found", plus the caveat from its own docstring: graph-navigable pages only, since a launch-reached page like customTabs has no nav edge. This is a discoverability fix rather than a new capability, which is the cheaper half of most tooling problems here. * efftriage: add a report-level rule tier, fix three rules that could never fire, and record A47-A53 Hardened live against the batch-2 conversions (MTE-5827, MTE-5831). Every change here came from a real failure during those conversions, and each new rule has a labelled corpus fixture taken from the run that motivated it. RULE DEFECTS FIXED. All three looked correct and could never fire, which is worse than a missing rule because "no rule matched" at least sends you to read the report: * T4 claimed to detect an AMBIGUOUS Compose match but its regex matched Compose's ZERO-match messages ("Expected exactly '1' node but could not find any node...", "Can't retrieve node at index '0'"). Those appear in T1-arrival-timeout, T7-multi and T2-selector-surface, so had T4 ever been ordered earlier it would have confidently misdiagnosed three labelled runs. Now requires "but found 'N' nodes" with N > 1. * T14 matched only "is (not) checked", so following the advice in its own gotcha (A46: switch to mozVerifyElementIsSelected) produced "is not selected" -- a message the rule could not see. The gotcha silently stopped applying the moment you obeyed it. Now matches both state words, and its fix text covers what to do when the failure is already "is not selected". * T9's first branch expected "Navigation to 'X' failed", but the ordinary arrival failure throws "Failed to navigate to X", which only ever lands in the FAILURES summary and never inside an [ERR] window. The branch was dead. Removed rather than extended, and the case is now handled by T18 in the report tier. NEW REPORT-LEVEL TIER. Rules in REPORT_RULES match against status.json's primary failure message instead of a window of trace around an [ERR] line. This exists because some failures leave NO [ERR] line at all -- a crashed instrumentation process produces an ~11-line report -- and others put the real exception outside the window (failure_windows takes 25 lines after the [ERR]; the ads-client stack lands 36 lines later). Widening the window was rejected: it drags stack text into unrelated windows, which is the same cross-contamination that makes T6 and T9 fire on the wrong evidence today (filed as MTE-5832). * T16 / A37 - the instrumentation process crashed; there is no assertion to read. * T17 / K13 - a UniFFI fake built with NoHandle had an un-overridden method called. Recurs in 3 of 19 corpus runs and can fail a test AFTER its assertions have passed. Deliberately NOT matched report-wide: the same text appears as a Suppressed exception in T0-absence-assert and T9-permission, so a substring match would steal both. * T18 / A6+A45+B7 - the thrown form of an arrival failure, pointing at the anchor's STRATEGY first (an ESPRESSO selector cannot see a Compose node, and Settings screens are routinely a View toolbar over Compose content). TEST GUARD FIXED. efftriage_tests.rule_ids() only inspected RULES, so the three new report-tier rules were admitted with no labelled example -- exactly what that guard exists to prevent. It now covers both tiers. Three fixtures added from real runs: T14-selected-not-checked, T15-external-app-blocked, T18-arrival-wrong-tree. Corpus 19 -> 21 labelled runs; two long-standing known_gap runs are now diagnosed. Suite: 58 tests, green. DOCS. HARNESS-GOTCHAS gains a "How to write an entry" convention, which is the most important change here. Claims about the HARNESS generalise and may be stated plainly; claims about PRODUCT code must be scoped to the file they were observed in, must say what to verify first, and should carry a counter-example. A46 asserted that Compose toggle rows expose state as `selected`; that is true of ListItem.kt and false of SearchEngineShortcuts.kt, and reading it as a law cost two cycles. A46 is rescoped accordingly, with its counter-example named. New entries: A47 (requiresScroll is inert for the singular mozVerify, and mozSwipeTo's direction default is the opposite of the precondition path's), A48 (an external-app assertion blocked by a system dialog, and the legacy twin that swallows its AssertionFailedError), A49 (a rule deliberately NOT mechanised, with the reason recorded so it does not read as an oversight), A50 (the stylus prompt steals keystrokes and causes no locate miss, so OverlayRegistry never fires), A51 (mozEnterText can leave a Compose field looking filled while the app never saw it), A52 (a custom search engine survives a retry but MockWebServer's port does not), A53 (some suggestion rows sit outside the Compose test tree -- read the right block of the dump). Also: efftriage's header no longer hard-codes a gotcha count. It said "57" and was wrong twice, which is the same failure mode the rules are meant to prevent. It now states the two standing rules instead -- match only text OBSERVED in a real run, and prefer "no rule matched" to a confident mismatch. * tae-conversion 2026.08.19: efftriage T19, effnext other-branch warning, gotchas A54-A59 Comes out of converting the seven site-permissions smoke tests (bugs 2064810/2064815/2064823/2064833). efftriage gains rule T19 for "No navigation path found from X to Y", which it previously left unmatched while its generic hint pointed at selectors and dumps - misleading, because that failure is a navigation graph gap where nothing was searched for on screen. Labelled corpus fixture T19-no-nav-path is a real run, so the rule is validated rather than listed as unvalidated. effnext now warns when a pick is already converted on another local branch. Its in-tree filter only sees the checked-out branch, so it offered CustomTabsTest.verifyLoginSaveInCustomTabTest, which has been converted and awaiting review on efficiency-batch-2 since last week. Advisory rather than a filter, and backup/* is excluded, because branches also carry abandoned work - the two faker conversions - and filtering on any branch would silently drop outstanding tests from the pool. That check shipped broken twice while being written, both times reporting a clean "nothing on other branches" while checking nothing at all: git grep -E cannot parse \b or \s, and subprocess was never imported so the NameError was swallowed by a bare except. Both paths now surface as branches_unchecked, and tests/effnext_tests.py covers them. HARNESS-GOTCHAS A54-A59 record what the device runs taught: option text is label plus subtext joined by a newline; permission-screen res-ids are shared across screens and ESPRESSO_BY_ID ignores visibility; BrowserPage has inbound edges only from HomePage and itself; a page-content text selector can resolve a non-clickable heading, giving "found" then a failed click; a system app-permission row title is present whether the permission is allowed or denied, so assert checkSelfPermission instead; and a page object missing from PageContext never registers its navigation. docs/SMOKE-CONVERSION-AUDIT.md is new: the standing burndown of remaining legacy @smoketest methods, what blocks each, and the parser to reproduce the count.
1 parent f02e082 commit 3d875d2

72 files changed

Lines changed: 12921 additions & 11 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
name: tae-conversion-tests
2+
3+
on:
4+
workflow_dispatch:
5+
push:
6+
paths:
7+
- "tae-conversion/**"
8+
- ".github/workflows/tae-conversion-tests.yml"
9+
pull_request:
10+
paths:
11+
- "tae-conversion/**"
12+
- ".github/workflows/tae-conversion-tests.yml"
13+
14+
jobs:
15+
test:
16+
name: Run unit tests
17+
runs-on: ubuntu-latest
18+
19+
strategy:
20+
fail-fast: false
21+
matrix:
22+
python-version: ["3.9", "3.12"]
23+
24+
steps:
25+
- name: Checkout code
26+
uses: actions/checkout@v7
27+
28+
- name: Set up Python
29+
uses: actions/setup-python@v7
30+
with:
31+
python-version: ${{ matrix.python-version }}
32+
33+
# No dependency install step: the eff* tools are stdlib-only, and the tests
34+
# need no device, no emulator, no Firefox checkout and no network. The rule
35+
# tests run against real conversion runs checked in under
36+
# tae-conversion/tests/fixtures/corpus; everything that shells out to ps,
37+
# lsof, adb or git is stubbed.
38+
- name: Run tests
39+
run: python -m unittest discover -s tae-conversion/tests -p '*tests.py' -v
40+
41+
# ResourceWarnings are errors here: an unclosed file in a tool that is run
42+
# hundreds of times a session is a real leak, and warning noise in the output
43+
# is what makes people stop reading it.
44+
- name: Run tests with warnings as errors
45+
run: python -W error::ResourceWarning -m unittest discover -s tae-conversion/tests -p '*tests.py'

‎tae-conversion/CHANGELOG.md‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
# Changelog — tae-conversion toolchain and docs
2+
3+
Tools and docs are versioned **together** (they cite each other by gotcha/lesson id, so a split version
4+
would only create skew). **CalVer** (`YYYY.MM.DD`), because the only question anyone asks of an internal
5+
script read from a checkout is "is my copy older than the thing being discussed" — not "is this a breaking
6+
change". Every tool answers `--version`.
7+
8+
This file is curated, not a commit log: entries are changes a *consumer* depends on — a new gate, changed
9+
verdict semantics, changed flags. `git log` has the rest.
10+
11+
## 2026.08.19
12+
13+
### Added
14+
- **efftriage rule T19** (gotcha **A56**) — `No navigation path found from 'X' to 'Y'`. Previously unmatched, and
15+
worse, the generic "a selector did not resolve, author against a dump" hint fired instead, which is misleading:
16+
this failure is a navigation GRAPH gap where nothing was searched for on screen. The rule names both halves of
17+
the fix (a return edge on the settings page **and** an `on.home.navigateToPage()` hop, because `findPath` only
18+
searches from the currently tracked page).
19+
- **Corpus fixture `T19-no-nav-path`** — a real labelled run (`sp02-autoplay`), so T19 is validated rather than
20+
listed as unvalidated. Suite is 58 tests, green.
21+
- **HARNESS-GOTCHAS A54–A59**, all from device runs during the site-permissions conversions: option text is
22+
label+`\n`+subtext (A54); permission-screen res-ids are shared and `ESPRESSO_BY_ID` ignores visibility (A55);
23+
`BrowserPage` has inbound edges only from `HomePage`/itself (A56); a page-content text selector can resolve a
24+
non-clickable heading, giving "found" then `Failed to click UiObject` (A57); a system app-permission row title
25+
exists whether allowed or denied, so assert `checkSelfPermission` instead (A58); a page object missing from
26+
`PageContext` never registers its navigation (A59).
27+
- **`docs/SMOKE-CONVERSION-AUDIT.md`** — the standing burndown: how many legacy `@SmokeTest` methods remain, which
28+
ones, what blocks each, and the parser to reproduce the count. Includes the reconciliation that
29+
`grep -c '@Converted'` is **not** a smoke ratio (three annotations sit on non-smoke tests).
30+
31+
- **`effnext` warns when a pick is already converted on another local branch** (`also_on_branches` in JSON).
32+
Its in-tree filter only sees the CHECKED-OUT branch, so it happily offered `CustomTabsTest.verifyLoginSaveInCustomTabTest`
33+
— already converted and awaiting review on `efficiency-batch-2`. Advisory, not a filter, and `backup/*` is
34+
excluded: branches also carry ABANDONED work (the two faker conversions), and filtering on any branch would
35+
silently drop genuinely outstanding tests from the pool.
36+
- **`tests/effnext_tests.py`** — 7 tests, including the two silent-failure paths. The check shipped broken twice
37+
in one sitting, both times reporting a clean "nothing on other branches" while checking nothing:
38+
`git grep -E` cannot parse `\b`/`\s`, and `subprocess` was unimported so the NameError was eaten by a bare
39+
`except`. A per-ref timeout or an unlistable ref set now reports `branches_unchecked` instead of a false clean.
40+
41+
### Changed
42+
- `docs/CONVERSION-LOOP.md` — verify step now states that `effverify` keys off **method** names: passing a class
43+
name reports `status: not-run` / `clean: false` for a fully green run. Also records that a cherry-picked base
44+
whose Phabricator revision is **closed** makes `moz-phab submit` fail for the whole stack.
45+
- `docs/CONVERSION-LESSONS.md` — site-permission screens, the prompt/label selectors, and the TestRail-id
46+
verification step (three of six ids were wrong when read from grep context instead of the line above the test).
47+
48+
## 2026.08.13
49+
50+
### Added
51+
- `efftriage.py` — reads a batch's `run-report.txt` + `status.json` and names the likely cause of a failure
52+
with its HARNESS-GOTCHAS id and the fix. Read-only, so it is safe to run on every failure. Scans the FIRST
53+
attempt only, because a failed attempt leaves state behind and later attempts die somewhere later and more
54+
confusingly. 15 rules (T0–T14): 12 match the trace and live in `RULES`, while **T10 (crash), T11 (skip) and T12
55+
(compile failure) are status-based checks** in `triage()` — so counting `RULES` alone gives 12, not 15.
56+
Hardened against labelled failures from the search batch and then against batch 2's own, which is where
57+
T11–T14 came from. (MTE-5827)
58+
- `effdoctor.py` slice 1 — read-only preflight, and specifically the **toolchain map**. The eff* tools exist
59+
once but are reachable from two checkouts, and `effwatch.sh` derives its queue from `dirname $0`, which does
60+
not resolve symlinks: the queue it watches depends on the path it was launched from, while the tools are
61+
identical either way. A request dropped in the other checkout's queue is never consumed and never errors.
62+
effdoctor now prints which watcher is live and which queue to use, flags a diverged alt checkout, and checks
63+
effpretty resolution, a stale gradle lock and the device. The watched queue is resolved against the *watcher
64+
process's* cwd (read via `lsof`), not effdoctor's, because effwatch is normally launched by a relative path —
65+
resolving it against the caller's cwd yields a directory that never existed. A computed queue that is not on
66+
disk is reported as a failure, and an unreadable cwd as "undetermined": naming the wrong queue confidently is
67+
worse than declining to name one. (MTE-5766)
68+
- **Unit tests** (`tae-conversion/tests/`, 58 of them) and a CI job, where previously the toolchain had none.
69+
Run them with `python -m unittest discover -s tae-conversion/tests -p '*tests.py'`; no device, emulator,
70+
Firefox checkout or network needed, since `ps`/`lsof`/`adb`/`git` are stubbed. The efftriage rule tests run
71+
against **real labelled conversion runs** checked in under `tests/fixtures/corpus` with a `labels.json`
72+
manifest: hand-written traces only test the shapes their author imagined, so they pass while the tool
73+
mismatches real output — a false green in the suite meant to prevent false greens. That corpus previously
74+
existed only as batch dirs in one person's `conversion-runs/`, unversioned and unrunnable in CI. Writing
75+
these found four defects, three of them in code that had already been reviewed by eye and pronounced fine.
76+
- `effbug.py` **update now accepts `comment`**, posting a new comment on an existing bug. Comment 0 cannot be
77+
edited through the Bugzilla API, so a correction previously had nowhere to go — found the hard way after a
78+
bug was filed with a wrong TestRail id. On `create` `comment` is still the description; on `update` it is a
79+
new comment.
80+
- `ALT_TOOLS` env var on `effdoctor`, so the alt-checkout divergence check works for checkouts that are not at
81+
`~/Workspace/ui-test-modernization`.
82+
- `VERSION` + this changelog, and `--version` on all 12 tools. Version lookup resolves symlinks, so it stays
83+
correct when a tool is invoked through another checkout's `tools/` dir. (MTE-5770 slice 1)
84+
- Docs: lessons **K13** (ad surfaces are testable offline by faking the app-services client at the seam
85+
app-services itself uses in its unit tests) and gotchas **A37–A43**.
86+
87+
### Fixed
88+
- `effloop.sh` — **a SKIPPED test was scored as a pass** (gotcha **A44**, the fifth false-green shape). The
89+
verdict was `failures == 0 and tests > 0`, which a skip satisfies without running, so an `@Ignore`d legacy
90+
test reported `outcome: pass` having asserted nothing. `outcome` is now `skipped` when nothing ran and
91+
`partial` when only some tests did, both exiting **5** (new code: ran, but the verdict is incomplete).
92+
`effverify` already scored these correctly, so a disagreement between the two was the tell.
93+
- `efftriage.py` — new rule **T14** for gotcha **A46**: a checked-state assertion that failed because the
94+
node it matched does not carry the state. Compose toggle/radio rows keep `selected` on the MERGED row and
95+
blank the switch with `clearAndSetSemantics`, so `mozVerifyElementIsChecked` can never pass and
96+
`COMPOSE_BY_TEXT` resolves a stateless descendant. This is the shape behind legacy tests that assert
97+
checked state via positional `UiSelector().index(n)`, so it will recur. (MTE-5827)
98+
- `efftriage.py` — new rule **T11** for the above. It reads the per-test statuses rather than `outcome`,
99+
because the field that should raise the alarm was the one lying. On its first run over the existing corpus it
100+
found **2 silently skipped tests** in an earlier 10-test run that nothing had ever reported.
101+
- `efftriage.py` — **a green run was handed a failure diagnosis.** Traces routinely carry non-fatal `[ERR]`
102+
lines (nav-graph polling, tolerated absence checks), and the rules scanned them regardless of outcome, so a
103+
passing run was reported as an absence-assertion failure. Two of the campaign's own labelled runs were
104+
affected. A passing single-attempt run is no longer triaged; its non-fatal error count is reported as a note
105+
instead. Deliberately still triaged: a **retry-pass**, where attempt 1 really did fail and `clean=false`
106+
means flaky-not-done (now labelled `PASSED ONLY ON RETRY`), and a **crash**, because crash mode is where
107+
`outcome: pass` is itself the lie (MTE-5822). (MTE-5827)
108+
- `efftriage.py` — a nonexistent batch dir was diagnosed as gotcha A24 ("run-report.txt is missing"), sending
109+
the reader to check effpretty resolution and pull dumps off a device on account of a mistyped path. It is now
110+
reported as `no-such-batch`, and no longer tracebacks with a `KeyError`.
111+
- `effdoctor.py` — **named the wrong queue, which is the failure it exists to prevent.** effwatch is normally
112+
launched by a relative path, and that path was resolved with `os.path.abspath` — against effdoctor's cwd
113+
rather than the watcher's. Run from a Firefox checkout it reported a live queue under
114+
`firefox/tae-conversion/`, a directory that has never existed, and advised queueing into it; a request
115+
dropped there is never consumed and never errors. The watcher's own cwd is now read (`/proc` first, `lsof`
116+
fallback). A computed queue that is not on disk is a FAIL, and an unreadable cwd is reported as undetermined
117+
with the command to run: declining to answer beats answering wrongly. (MTE-5766)
118+
- `effdoctor.py` — the self-exclusion filter skipped every `ps` line containing the substring `effdoctor`, so
119+
a watcher running from a directory whose name contained it was invisible. Now skips only this process.
120+
- `effverify.py` — **a crashed test was scored as passed**, with `clean=true` and `failed_total=0`. Only
121+
`failed:` markers and the gradle status line were read, and a crash emits neither (`"gradle": null` was the
122+
tell). Now also reads the report's `FAILURES (n of m)` header and `CRASH:` lines, and refuses to report
123+
clean when the declared failure count exceeds what it can attribute. New JSON fields:
124+
`declared_failures`, `unattributed_failures`. Also fixes a `NameError` that crashed the tool while building
125+
a failure excerpt for a batch with no `raw-run.log`. (MTE-5822, gotcha A37)
126+
- `effloop.sh` — resolved `effpretty.py` as `$TOOLS/effpretty.py`, but it lives in mozilla-central, so it only
127+
worked for checkouts that happened to have a copy or symlink beside the script. Everywhere else the renderer
128+
silently never ran: empty `run-report.txt`, and no verdict from effverify. Now resolved under `$REPO`
129+
(override with `EFFPRETTY=`), with a `$TOOLS` fallback and a loud exit-2 guard. (MTE-5764, gotcha A24)
130+
131+
### Changed
132+
- Docs: README's tool table described effverify's pre-2026-08-03 "last run" scoping; lessons **K8** documented
133+
a `NameError` that is now fixed; gotcha **A24** now carries its root cause.
134+
135+
### Known gaps
136+
- `efftriage` rules **T3, T4 and T5 have no labelled example** anywhere in the 53-run corpus they were
137+
hardened against, so they are unvalidated rather than known-good — a rule that has never fired on a real
138+
failure may not match the shape it was written for at all. Listed in `UNVALIDATED_RULES` in the tests, which
139+
fail if the list goes stale in either direction.
140+
- 3 of the 28 failed runs in the corpus get **no diagnosis** (`gap-*` fixtures). They are checked in as
141+
tripwires: add a rule that explains one and the test fails, which is the reminder to record the win.
142+
- Mechanized coverage is not yet reported. `effgates --coverage` plus a `Gate:` line on every doc entry is
143+
MTE-5830; the `effcheck` verb × selector-strategy matrix is MTE-5828; `effparity` (legacy-vs-port assertion
144+
parity, gating lesson G1) is MTE-5829.

‎tae-conversion/README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ Optional environment, all with sane fallbacks:
5151
| `effscaffold.py` | agent | Front-loads a conversion: legacy body, TestRail id, already-converted check, robots + selector lines, existing coverage. |
5252
| `effcheck.py` | agent | Static pre-flight, no device. Resolution, empty nav paths, inline selectors, missing verbs, test-class boilerplate. Exit ≠ 0 = fix something. |
5353
| `effbuild.py` | agent | Gradle log → one-line verdict plus only the compile errors. `--json`. |
54+
| `effdoctor.py` | agent + human | **Preflight — run it FIRST.** Read-only. Prints the toolchain map: canonical tools/queue, which `effwatch` is live and therefore **which queue to drop requests in**, whether the alt checkout has diverged, effpretty resolution, a stale gradle lock, and the device. `--json`. |
55+
| `efftriage.py` | agent + human | **Failure explainer.** Reads a batch's `run-report.txt`/`status.json` and names the likely cause with the HARNESS-GOTCHAS id and the fix. Read-only, so it is safe to run on every failure. Scans the FIRST attempt only. `--json`. |
5456
| `effverify.py` | agent | **Done-gate.** Confirms the named test ran, wasn't skipped, and did not fail in *any* run block. Reads the report's `failed:` markers, its `FAILURES (n of m)` header and `CRASH:` lines, so a test that died from an uncaught exception is not scored as passed. `clean=false` = passed only on retry, i.e. flaky. Refuses to report clean when the declared failure count exceeds what it can attribute (`unattributed_failures`). `--json`. |
5557
| `effloop.sh` | you | One command: build → run on device → write `build-report.txt`, `run-report.txt`, `status.json`. |
5658
| `effwatch.sh` | you | Start once, leave running. Polls `conversion-runs/_queue/`, runs the build and the git/Bugzilla actions, writes results back. |

‎tae-conversion/VERSION‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
2026.08.19

‎tae-conversion/docs/CONVERSION-LESSONS.md‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,3 +483,33 @@ and which assertion to trust.
483483
saving an address Gecko won't match to the US form, vs. a genuine prompt trigger/timing issue. Next step:
484484
one instrumented run dumping Manage Addresses (now that all-layer dumps are in) to confirm the saved values
485485
before deciding between "fix the dropdown" and "fix the trigger". Test + harness are built and compile-clean.
486+
487+
## Autoplay / site-permission screens (2026-08-19, sp01-sp03)
488+
489+
**A radio's text is the label AND its subtext, joined by a newline.** The live dump of the Autoplay screen shows
490+
`res-id="block_radio" text="Block audio and video on cellular data only
Audio and video will play on Wi-Fi"`
491+
and `third_radio text="Block audio only
Recommended"`. So an exact-text selector built from the strings.xml
492+
label alone (`preference_option_autoplay_allowed_wifi_only2`) can never resolve, and the failure surfaces as an
493+
arrival-anchor miss — indistinguishable at first glance from a nav-path problem. Use TEXT_CONTAINS on a fragment,
494+
or the res-id.
495+
496+
**The permission-screen res-ids cannot identify WHICH permission screen you are on.** `ask_to_allow_radio`,
497+
`block_radio`, `third_radio`, `fourth_radio` all live in the layout shared by Camera/Location/Microphone/Autoplay,
498+
where the 3rd and 4th are merely hidden. ESPRESSO_BY_ID resolves `onView(withId(...))` with no visibility
499+
constraint, so an id anchor also resolves on the other screens (gotcha A45, false arrival). Anchor on
500+
autoplay-only label text; keep the ids for the per-option assertions.
501+
502+
**UIAUTOMATOR_WITH_RES_ID restores the legacy visibility semantics for free.** Legacy asserted the three options
503+
with Espresso `withEffectiveVisibility(VISIBLE)`. The UIAutomator tree contains only displayed nodes, so a
504+
res-id group check carries the same guarantee — a better parity match than Espresso ids.
505+
506+
**Leaving a settings screen for a URL needs BOTH halves.** `BrowserPage` has inbound edges only from `HomePage`
507+
and itself, so `on.browserPage.navigateToPage(url)` from a settings page dies with "No navigation path found
508+
from X to BrowserPage". Register a return edge on the settings page —
509+
`NavigationStep.PressBackUntilGone(SettingsSelectors.NAVIGATION_TOOLBAR)`, depth-independent — AND put an
510+
explicit `on.home.navigateToPage()` in the test (the harness equivalent of legacy `exitMenu()`). The edge alone
511+
is not enough: `findPath` only searches from the currently tracked page. This is now **efftriage rule T19**,
512+
with `tests/fixtures/corpus/T19-no-nav-path` as its labelled example.
513+
514+
**Toolchain note:** `effverify <batchdir> <ClassName>` reports `status: not-run` / `clean: false` even for a
515+
fully green run — it keys off METHOD names. Pass the method names, and cross-check `status.json`.

0 commit comments

Comments
 (0)