Tae skills effnext skip and paperwork - #33
Open
jjSDET wants to merge 4 commits into
Open
Conversation
Three rules the loop was getting wrong or leaving implicit, all of which cost real rework on the 2026-08 stack: - Conversion bugs must block the tracking meta (2030727), and must do it at create time rather than as a second round-trip. Tooling, docs and harness bugs stay off the meta — it is specifically about migrating and removing legacy tests. Includes the backfill form, and a warning never to PUT a bare list to a meta's depends_on, since Bugzilla treats that as replace. - @converted goes in the same commit as the conversion, gated on green locally rather than on landing. Annotating after landing needs a second bug and a second review, and in practice gets forgotten; the burndown then reads the conversion as missing. The commit's paths list must include the annotated legacy file. - Jira items go through tools/jiratool.py, which works headless, rather than the Atlassian connector, which is absent in headless and cron runs. Notes that create defaults to Story with no labels and does not self-assign.
…ility effnext gained --skip/--unskip/--skips (testops-tools) so a candidate you decide not to take is recorded with a reason instead of being mentally stepped over — the next caller then gets a different pick. It also now drops candidates whose method already exists in the efficiency tests package, because the done-ledger lags the tree badly: it proposed a test that was already converted and committed, and 11 of its top 30 candidates were already in-tree. Gate 0 and the tool table say so. The assertion gate gains the two traps behind bug 2060405: a disabled Compose button accepts the click gesture and skips onClick, so "clicked" in the report does not mean the app acted; and an enabled-check on a COMPOSE_BY_TEXT selector is a no-op because it resolves the text node inside the button, which reports enabled while the button is disabled. Use COMPOSE_BY_TEXT_MERGED for anything you act on, and prefer a positive assertion over waiting for something to disappear. Also carries the @converted same-commit correction for this skill's step 7.
The tool table said the done-gate is "scoped to the last run". That was the defect, not the contract: effloop emits one run block per test after the class run, so reading only the last block meant passing tests reported as not-run, genuinely failed tests as failed_total 0, and retried false across a run containing eight of them. effverify now aggregates every block and reports a per-test status, including retry-pass. Documenting the broken behaviour as intended is worse than not documenting it — an agent reading this would trust a verdict the tool no longer produces, and would have no reason to question a green that came from the wrong block.
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the tae skill documentation based on an end-to-end run of the ui/efficiency conversion workflow, correcting previously incorrect instructions and documenting new/updated tool behaviors.
Changes:
- Document
effnextskip/unskip flow and in-tree filtering in the authoring gate 0 instructions. - Clarify assertion-gate Compose pitfalls and reinforce
@Convertedtiming/placement requirements. - Update conversion-loop “paperwork” steps to require blocking the tracking meta, using headless Jira tooling, and describing
effverify’s aggregated-run behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| plugins/tae/skills/efficiency-test-authoring/SKILL.md | Documents effnext skip behavior, adds assertion-gate gotchas, and tightens @Converted workflow guidance. |
| plugins/tae/skills/efficiency-conversion-loop/SKILL.md | Corrects conversion-loop steps (meta blocking, @Converted in-commit) and switches Jira instructions toward tools/jiratool.py. |
Suppressed comments (1)
plugins/tae/skills/efficiency-conversion-loop/SKILL.md:138
- This section says to use
tools/jiratool.py, but the command examples invokepython3 jiratool.py, which will fail unless the reader happens to be in thetools/directory. Use an explicit path (or state the required working directory) so the commands are copy/pasteable.
python3 jiratool.py create '<summary>' --file body.txt --parent MTE-5731 --issuetype Sub-task --label conversion
python3 jiratool.py create '<summary>' --file body.txt --parent MTE-5715 --issuetype Sub-task --label enablement
python3 jiratool.py link <enablement-key> Relates <conversion-key>
python3 jiratool.py assign <key> --me # create does NOT self-assign; do this explicitly
</details>
---
💡 <a href="/mozilla/firefox-aidev-plugins/new/main?filename=.github/instructions/*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.
Comment on lines
+18
to
+19
| **convert → file bug (blocking meta 2030727) → annotate `@Converted` + commit (with bug #) → track in Jira | ||
| → submit for review** |
| efficiency tests package. **Never call the Google Sheet to choose** — it's slow and the local pool is the | ||
| working queue. (The Sheet is systems-of-record for status, not the per-test picker.) If the pick isn't one | ||
| to take now — too complex for whoever is picking it up, blocked on a harness gap, deliberately deferred — | ||
| record that rather than stepping over it: `effnext --skip Class.method --reason "…"` parks it (reversible |
Gate 0 now says to fetch main before picking. Both effnext's in-tree filter and effscaffold's already-converted check read the working tree, so a branch that predates someone else's landing cannot see their conversion — which is how bug 2060292 ended up duplicating bug 2060174, surfacing only as a rebase conflict after review and submission. The loop skill gains effbug's dupe_of form for closing the loser, a note that Bugzilla has no API for editing a description, and guidance for rebasing an already-submitted stack: dropping a commit leaves its revision in the dependency graph, so the whole range has to be resubmitted to re-parent it, at the cost of fresh diffs everywhere and accepted revisions resetting to needs-review.
Contributor
|
Might need to bump a point release of the plugin |
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.
tae: correct the conversion loop's paperwork steps and document effnext's skip flow
Three documentation changes to the
taeskills, all from running the conversion loop end to end on a real stack. Two of them correct instructions that were actively wrong; the third documents new tooling.Depends on the companion
testops-toolsPR (tae-conversion-toolchain-fixes) — it adds theeffnextflags described here and theeffverifybehaviour this corrects. Land that one first so these skills don't reference flags that don't exist yet.1. Paperwork the loop was getting wrong (
efficiency-conversion-loop)Three rules, each of which cost rework on the most recent stack:
depends_onlist, so a conversion bug that never gets linked is invisible to anyone reading it for status. Two bugs on the last stack were caught unlinked during a pre-submit audit. Scope matters too: tooling, docs and harness bugs stay off the meta, since it is specifically about migrating and removing legacy tests. Includes the backfill form, and a warning never to PUT a bare list to a meta'sdepends_on— Bugzilla treats that as replace, which would silently drop every other bug it tracks.@Convertedgoes in the same commit as the conversion, gated on green locally rather than on landing. The burndown keys off that marker, so a conversion landing without it reads as unconverted; annotating afterwards needs a second bug and a second review. On the last stack this was missed on two conversions and required a mid-stack rewrite to fix properly. The commit'spathslist must include the annotated legacy file.tools/jiratool.py, which works headless, rather than the Atlassian connector, which is absent in headless and cron runs. Notes thatcreatedefaults toStorywith no labels and does not self-assign, so all three have to be passed explicitly.2. effnext's skip flow (
efficiency-test-authoring, gate 0)Whoever picks up the queue may not be in a position to take the next candidate — too complex for them, blocked on a harness gap, or deliberately deferred by whoever is sequencing the campaign. There was no way to say so, so the decision lived in someone's head and the next caller got the same pick.
Gate 0 and the tool table now describe
--skip Class.method --reason "…",--unskip, and--skips, including the property that makes them safe to use casually: a skip never marks a test converted.The same entry documents the new in-tree filter. The done-ledger lags the tree — it proposed a test that was already converted and committed, and 11 of its top 30 candidates were already in-tree — so
effnextnow drops candidates whose method already exists in the efficiency tests package. Worth knowing thateffscaffold'salready_converteddoes not cover this: it matches files, not methods, which is how the stale pick got past gate 1.3. Two traps at the assertion gate (
efficiency-test-authoring)From bug 2060405, both of which cost a long debugging session:
onClick, so "clicked" in the report means the gesture was delivered, not that the app acted. The failure then surfaces wherever the effect was expected — in that case 25 seconds and several steps later.COMPOSE_BY_TEXTselector is a no-op, because it resolves the text node inside the button, which reports enabled while the button is disabled. UseCOMPOSE_BY_TEXT_MERGEDfor anything you act on.Plus the rule that falls out of both: prefer a positive assertion over waiting for something to disappear, since absence cannot distinguish "it worked" from "the click was dropped".
4. effverify's scope was documented as its own bug
The tool table said the done-gate is "scoped to the last run". That was the defect, not the contract:
effloopemits one run block per test after the class run, so reading only the last block reported passing tests asnot-run, genuinely failed tests asfailed_total: 0, andretried: falseacross a run containing eight of them.effverifynow aggregates every block and reports a per-test status includingretry-pass. Documenting the broken behaviour as intended is worse than not documenting it — an agent would trust a verdict the tool no longer produces, and have no reason to question a green that came from the wrong block.Notes
Documentation only; no plugin code changes. The rules here are not proposals — each one is the corrected version of something that went wrong on the four-commit Fenix stack these skills drove, which shipped two conversions, two harness fixes, and a full-suite sweep of 42 efficiency classes / 137 tests.