Skip to content

Commit 2b1faef

Browse files
committed
fix(smoke): a start that drew nothing is the same event as a frame of one colour
The release's next run failed on camera-follow — "reported ready, then drew no frame at all" — where the run before it drew 240 frames, on the same emulator from the same build. The retry did not cover it: it knew the flat-frame face of this and not the empty-record one. Which face shows depends only on whether the boot record caught a frame count before the capture, so both are relaunched now. The border is `ready`, and it needs no code: a launch that never reported ready is given that verdict and no other, so both faces already imply a start that succeeded. A guard for it went in first and its sabotage came back GREEN — the criterion that was supposed to hold the border passes on the verdict text alone, which is the guard being unreachable rather than the criterion being weak. A branch nothing can reach cannot be wrong, so it is gone and the comment says why. Sabotage-verified in the form that does bite: dropping the empty-record face reds the criterion that names it.
1 parent 1e894bb commit 2b1faef

2 files changed

Lines changed: 33 additions & 13 deletions

File tree

tools/lib/smokeRetry.mjs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,14 @@
2626

2727
/** @typedef {{ok?: boolean, undetermined?: boolean, why?: string, unjudged?: string|null, offScreen?: string|null}} SmokeResult */
2828

29-
/** The verdict for a frame that has no colour in it but one. */
30-
const FLAT_FRAME = / flat color after /;
29+
/**
30+
* The verdicts that follow a successful start and say the app did not draw.
31+
*
32+
* Two faces of one event — the frame never reaching the surface — telling apart
33+
* only by whether the record caught a frame count (camera-follow: 240 frames in
34+
* one run of a build, none in the next).
35+
*/
36+
const NOT_DRAWN_AFTER_READY = [/ flat color after /, /drew no frame at all/];
3137

3238
/**
3339
* Worth launching a second time, on the same installed APK.
@@ -40,7 +46,10 @@ export function worthAnotherLaunch(r) {
4046
// spent, and the frame under a dialog is not this check's to judge.
4147
if (r.offScreen) return false;
4248
if (r.undetermined) return true;
43-
return !r.ok && FLAT_FRAME.test(r.why ?? '');
49+
// No `ready` check guarding these: a run that never reported ready is given
50+
// that verdict and no other, so both faces below already imply a start that
51+
// succeeded. The border is real, and the caller draws it.
52+
return !r.ok && NOT_DRAWN_AFTER_READY.some((face) => face.test(r.why ?? ''));
4453
}
4554

4655
/**

tools/tests/smoke-retry.test.ts

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,30 +17,41 @@ import { worthAnotherLaunch, verdictOf, exitCodeFor, unjudgedReason } from '../l
1717
const run = (o: Partial<{ countColors: boolean, frames: number, wanted: number, colors: number, minColors: number }>) =>
1818
({ countColors: true, frames: 30, wanted: 30, colors: 8, minColors: 2, ...o });
1919

20-
const flat = { ok: false, why: 'the frame is 1 flat color after 120 frame(s)' };
21-
const drew = { ok: true, colors: 8 };
22-
const slow = { ok: false, undetermined: true, unjudged: 'too slow to judge here — 10 frame(s) before the capture' };
20+
const flat = { ok: false, ready: true, why: 'the frame is 1 flat color after 120 frame(s)' };
21+
const noFrame = { ok: false, ready: true, why: 'reported ready, then drew no frame at all' };
22+
const drew = { ok: true, ready: true, colors: 8 };
23+
const slow = { ok: false, ready: true, undetermined: true, unjudged: 'too slow to judge here — 10 frame(s) before the capture' };
2324

2425
describe('what earns a second launch', () => {
2526
it('repeats a frame of one flat colour', () => {
2627
expect(worthAnotherLaunch(flat)).toBe(true);
2728
});
2829

30+
// The same event as a flat frame, seen when the record caught no frame count
31+
// at all: camera-follow drew 240 frames in one run of a build and none in the
32+
// next, on the same emulator.
33+
it('repeats a start that reported ready and then drew nothing', () => {
34+
expect(worthAnotherLaunch(noFrame)).toBe(true);
35+
});
36+
2937
it('repeats a run that never reached the frame it is judged at', () => {
3038
expect(worthAnotherLaunch(slow)).toBe(true);
3139
});
3240

3341
// The whole point of the policy: these are the ones that must not be given a
34-
// second chance, or a broken build becomes a slow green one.
35-
it('does not repeat a crash, a launch that never reported ready, or a boot error', () => {
36-
expect(worthAnotherLaunch({ ok: false, why: 'never reported ready' })).toBe(false);
37-
expect(worthAnotherLaunch({ ok: false, why: 'reported ready, then drew no frame at all' })).toBe(false);
38-
expect(worthAnotherLaunch({ ok: false, why: 'ERROR [asset] texture 4 failed to upload' })).toBe(false);
39-
expect(worthAnotherLaunch({ ok: false, why: 'after an activity recreate: ERROR [js] undefined' })).toBe(false);
42+
// second chance, or a broken build becomes a slow green one. `ready` is the
43+
// border — a launch that never got there is judged once.
44+
it('does not repeat a launch that never reported ready', () => {
45+
expect(worthAnotherLaunch({ ok: false, ready: false, why: 'never reported ready' })).toBe(false);
46+
});
47+
48+
it('does not repeat an engine error, even after a successful start', () => {
49+
expect(worthAnotherLaunch({ ok: false, ready: true, why: 'ERROR [asset] texture 4 failed to upload' })).toBe(false);
50+
expect(worthAnotherLaunch({ ok: false, ready: true, why: 'after an activity recreate: ERROR [js] undefined' })).toBe(false);
4051
});
4152

4253
it('does not repeat a game a dialog was covering — that retry is already spent', () => {
43-
expect(worthAnotherLaunch({ ok: false, why: 'the game was not on screen — a dialog has focus', offScreen: 'a dialog has focus' })).toBe(false);
54+
expect(worthAnotherLaunch({ ok: false, ready: true, why: 'the game was not on screen — a dialog has focus', offScreen: 'a dialog has focus' })).toBe(false);
4455
});
4556

4657
it('leaves a run that drew alone', () => {

0 commit comments

Comments
 (0)