|
| 1 | +// Unit tests for the editor-exit classifier (issue #1153). |
| 2 | +// Run with: npm run test:units |
| 3 | +// |
| 4 | +// What rides on this module: when the upstream exit deadlock fires, the line |
| 5 | +// this produces is the entire product of the failure in a daily's report. Before |
| 6 | +// #1153 the same event surfaced as a bare `home-dropdown-menu` visibility |
| 7 | +// timeout, which #1005's triage spent a full 24-run burst re-deriving into "the |
| 8 | +// blocker dialog never cleared". |
| 9 | +// |
| 10 | +// Four distinctions these tests exist to protect: |
| 11 | +// |
| 12 | +// 1. **`stuck` is not `blocked-deadlocked`.** A swallowed chevron click and a |
| 13 | +// blocked navigation both end with the editor still on screen, and they send |
| 14 | +// a reader to opposite places (a click that never fired vs. the upstream |
| 15 | +// defect). Collapsing them re-creates the unattributed timeout. |
| 16 | +// 2. **`pending` is a verdict, not an absence.** The polling loop terminates on |
| 17 | +// "anything but pending", so a classifier that returned `left` for "nothing |
| 18 | +// has happened yet" would make the loop exit on its first tick and assert |
| 19 | +// against a page that has not navigated. |
| 20 | +// 3. **Home wins over a painted dialog.** The dialog animates out, so it is |
| 21 | +// routinely still in the DOM on the tick where home first renders. Reporting |
| 22 | +// that as a deadlock would warn — and force a page load — on healthy exits. |
| 23 | +// 4. **The two deadlines are not one deadline.** An empty screen is an ordinary |
| 24 | +// in-flight navigation and gets the full home budget; only a dialog already |
| 25 | +// on screen is judged on the shorter blocker grace. Charging the grace window |
| 26 | +// to a slow-but-healthy exit reports a dead click that never happened — the |
| 27 | +// same mis-attribution as a bare timeout, only with a confident label. |
| 28 | +import { test } from "node:test"; |
| 29 | +import assert from "node:assert/strict"; |
| 30 | +import { |
| 31 | + BLOCKER_GRACE_MS, |
| 32 | + HOME_TIMEOUT_MS, |
| 33 | + classifyEditorExit, |
| 34 | + formatEditorExitStuckFailure, |
| 35 | + formatEditorExitWarning, |
| 36 | + type EditorExitVerdict, |
| 37 | +} from "./leave-flow-editor"; |
| 38 | + |
| 39 | +/** |
| 40 | + * Named rather than positional, and every deadline defaults to NOT expired: a |
| 41 | + * test that forgets to expire one gets `pending` — which fails loudly — instead |
| 42 | + * of silently agreeing with whichever flag happened to be passed. |
| 43 | + */ |
| 44 | +const verdict = (observed: { |
| 45 | + home?: boolean; |
| 46 | + blocker?: boolean; |
| 47 | + /** `BLOCKER_GRACE_MS` elapsed. Only ever promotes a dialog already on screen. */ |
| 48 | + grace?: boolean; |
| 49 | + /** `HOME_TIMEOUT_MS` elapsed. The only thing that may call an empty screen stuck. */ |
| 50 | + homeBudget?: boolean; |
| 51 | +}): EditorExitVerdict => |
| 52 | + classifyEditorExit({ |
| 53 | + homeVisible: observed.home ?? false, |
| 54 | + blockerVisible: observed.blocker ?? false, |
| 55 | + graceExpired: observed.grace ?? false, |
| 56 | + homeBudgetExpired: observed.homeBudget ?? false, |
| 57 | + }); |
| 58 | + |
| 59 | +test("home rendered with no dialog is a clean exit", () => { |
| 60 | + assert.equal(verdict({ home: true }), "left"); |
| 61 | + assert.equal(verdict({ home: true, grace: true, homeBudget: true }), "left"); |
| 62 | +}); |
| 63 | + |
| 64 | +test("home rendered while the dialog is still painted is NOT a deadlock", () => { |
| 65 | + // The dialog animates out; it is routinely still in the DOM on the tick where |
| 66 | + // home first renders. Calling that a deadlock would warn and force a page load |
| 67 | + // on every healthy exit that happened to pass through the blocker. |
| 68 | + assert.equal(verdict({ home: true, blocker: true }), "blocked-settled"); |
| 69 | + assert.equal( |
| 70 | + verdict({ home: true, blocker: true, grace: true, homeBudget: true }), |
| 71 | + "blocked-settled", |
| 72 | + ); |
| 73 | +}); |
| 74 | + |
| 75 | +test("the dialog inside the grace window is pending, not a verdict", () => { |
| 76 | + // `handleSave`'s own timeout is 1200ms, so a dialog that is up for a moment is |
| 77 | + // the normal save-then-proceed path. Returning a terminal verdict here would |
| 78 | + // make the caller give up before the exit had a chance to complete. |
| 79 | + assert.equal(verdict({ blocker: true }), "pending"); |
| 80 | +}); |
| 81 | + |
| 82 | +test("the dialog past the grace window is the #1153 deadlock", () => { |
| 83 | + assert.equal(verdict({ blocker: true, grace: true }), "blocked-deadlocked"); |
| 84 | +}); |
| 85 | + |
| 86 | +test("an empty screen past the grace window is NOT yet stuck", () => { |
| 87 | + // The regression this pins: `stuck` used to share the blocker's deadline, so a |
| 88 | + // navigation that was merely slow — a client-side route change plus the |
| 89 | + // listing's own GET, the thing that runs long on a saturated daily — failed at |
| 90 | + // 15s claiming the click never registered. The call sites this helper replaced |
| 91 | + // allowed 30s for exactly this window. |
| 92 | + assert.equal(verdict({ grace: true }), "pending"); |
| 93 | +}); |
| 94 | + |
| 95 | +test("an empty screen past the HOME budget is a swallowed click, not a deadlock", () => { |
| 96 | + // Distinct from `blocked-deadlocked` on purpose: this is the editor still on |
| 97 | + // screen with nothing blocking it, i.e. the chevron click never registered. |
| 98 | + assert.equal(verdict({ grace: true, homeBudget: true }), "stuck"); |
| 99 | +}); |
| 100 | + |
| 101 | +test("neither home nor dialog inside both windows is still pending", () => { |
| 102 | + assert.equal(verdict({}), "pending"); |
| 103 | +}); |
| 104 | + |
| 105 | +test("`pending` is never terminal, so the poll loop cannot exit early", () => { |
| 106 | + // The loop's only exit condition is `verdict !== "pending"`. Every combination |
| 107 | + // that has not resolved yet must therefore classify as pending, or the caller |
| 108 | + // asserts against a page that has not navigated. |
| 109 | + // |
| 110 | + // Includes the deadline states, because they expire at different times: with |
| 111 | + // the blocker grace elapsed but the home budget still open, an empty screen is |
| 112 | + // the one state where a shared deadline used to produce a wrong verdict. |
| 113 | + const unresolved = [ |
| 114 | + { blocker: false, grace: false }, |
| 115 | + { blocker: true, grace: false }, |
| 116 | + { blocker: false, grace: true }, |
| 117 | + ]; |
| 118 | + for (const { blocker, grace } of unresolved) { |
| 119 | + assert.equal( |
| 120 | + verdict({ blocker, grace }), |
| 121 | + "pending", |
| 122 | + `blocker=${blocker} grace=${grace} must stay pending while the home budget is open`, |
| 123 | + ); |
| 124 | + } |
| 125 | +}); |
| 126 | + |
| 127 | +test("the deadlock message names the upstream defect, not just the symptom", () => { |
| 128 | + // Called with NO argument, so this pins the pairing the helper actually ships |
| 129 | + // — not just the formatting. Feeding the constant in would have passed a |
| 130 | + // regression that quoted a budget this verdict is not gated on. |
| 131 | + const message = formatEditorExitWarning(); |
| 132 | + // A triager reading only this line has to reach the issue and the mechanism |
| 133 | + // without opening the screenshot. |
| 134 | + assert.match(message, /#1153/); |
| 135 | + assert.match(message, /SaveChangesModal/); |
| 136 | + assert.match(message, /no \.catch\(\)/); |
| 137 | + assert.match(message, new RegExp(`${BLOCKER_GRACE_MS}ms`)); |
| 138 | + // Must not quote the home budget: this verdict is gated on the grace window, |
| 139 | + // and swapping the two is precisely how the deadlines get collapsed again. |
| 140 | + assert.doesNotMatch(message, new RegExp(`${HOME_TIMEOUT_MS}ms`)); |
| 141 | +}); |
| 142 | + |
| 143 | +test("the stuck message rules the deadlock OUT rather than staying vague", () => { |
| 144 | + // The two failures send a reader to opposite places, so the message for one |
| 145 | + // must not read as the other. A triager who sees this line must not go |
| 146 | + // looking at SaveChangesModal. |
| 147 | + // Also called with no argument — see the deadlock message test above. |
| 148 | + const message = formatEditorExitStuckFailure(); |
| 149 | + assert.match(message, /did not navigate/); |
| 150 | + assert.match(message, /NOT the #1153/); |
| 151 | + assert.match(message, /LE-2019/); |
| 152 | + assert.doesNotMatch( |
| 153 | + message, |
| 154 | + /deadlocked/, |
| 155 | + "the swallowed-click message must not describe itself as the deadlock", |
| 156 | + ); |
| 157 | + // It must quote the window it actually waited out. Quoting the blocker's |
| 158 | + // shorter grace would understate the evidence behind "the click never |
| 159 | + // registered" — and would be the visible symptom of the two deadlines having |
| 160 | + // been collapsed back into one. |
| 161 | + assert.match(message, new RegExp(`${HOME_TIMEOUT_MS}ms`)); |
| 162 | + assert.doesNotMatch(message, new RegExp(`${BLOCKER_GRACE_MS}ms`)); |
| 163 | +}); |
| 164 | + |
| 165 | +test("the grace budget is not below the repo's save budgets", () => { |
| 166 | + // `renameFlow` allows 15s per modal step and a single save click has needed |
| 167 | + // longer than that under CI saturation (#790). A budget below that would |
| 168 | + // classify a slow-but-working save as a deadlock — and, where recovery is |
| 169 | + // enabled, discard editor state over it. |
| 170 | + assert.ok( |
| 171 | + BLOCKER_GRACE_MS >= 15000, |
| 172 | + `grace window ${BLOCKER_GRACE_MS}ms is below renameFlow's 15000ms per-step budget`, |
| 173 | + ); |
| 174 | +}); |
| 175 | + |
| 176 | +test("the home budget stays above the blocker grace, and above what the call sites had", () => { |
| 177 | + // Ordering is the whole point of splitting the deadlines: if the home budget |
| 178 | + // ever fell to or below the grace window, `stuck` would start firing on slow |
| 179 | + // navigations again and this helper would be back to mis-attributing them. |
| 180 | + assert.ok( |
| 181 | + HOME_TIMEOUT_MS > BLOCKER_GRACE_MS, |
| 182 | + `home budget ${HOME_TIMEOUT_MS}ms must exceed the blocker grace ${BLOCKER_GRACE_MS}ms`, |
| 183 | + ); |
| 184 | + // 30s is inherited, not invented: it is what `duplicate-flow` (`toBeVisible`) |
| 185 | + // and `export-import-flow` (`waitForSelector`) allowed this assertion before |
| 186 | + // the helper existed. The helper must not silently shorten it. |
| 187 | + assert.ok( |
| 188 | + HOME_TIMEOUT_MS >= 30000, |
| 189 | + `home budget ${HOME_TIMEOUT_MS}ms is below the 30000ms the call sites already allowed`, |
| 190 | + ); |
| 191 | +}); |
0 commit comments