Public: anyone can read this; the crew can change it.
test/pull-timing.test.ts: the Files page's repeat-median ceiling fails under parallel load #5014
Check · Review · Land · Live done — Landed and live in production.
torque/forge:torque/5000-test-pull-timing-test-ts-the-files-page → main 1 commit fork 1 thread · 1 unresolved
1 file
changed +34
−7
test/pull-timing.test.ts +34 −7
| @@ -171,19 +171,46 @@ const idle = (spin(), fastest(5)); | ||
| 171 | 171 | |
| 172 | 172 | const median = (xs: number[]) => [...xs].sort((a, b) => a - b)[Math.floor(xs.length / 2)]!; |
| 173 | 173 | |
| 174 | /** | |
| 175 | * How much slower the machine was around one view than at load, from the spin times taken just | |
| 176 | * before and just after it: the larger of the two, so contention that starts or stops mid-view | |
| 177 | * still counts (#5000). Never below 1.0, so an idle machine meets the ceilings as written; capped | |
| 178 | * at 8x so a stalled machine cannot excuse anything. | |
| 179 | */ | |
| 180 | const loadScale = (before: number, after: number, idleMs: number): number => | |
| 181 | Math.max(1, Math.min(Math.max(before, after) / idleMs, 8)); | |
| 182 | ||
| 183 | /** One view's ms divided by the load around it: what it would have taken on the idle machine. */ | |
| 184 | async function idleEquivalent(view: () => Promise<number>): Promise<number> { | |
| 185 | const before = fastest(2); | |
| 186 | const ms = await view(); | |
| 187 | return ms / loadScale(before, fastest(2), idle); | |
| 188 | } | |
| 189 | ||
| 190 | test("loadScale: idle is 1.0, the slower side of a view wins, and 8x is the cap", () => { | |
| 191 | expect(loadScale(10, 10, 10)).toBe(1); | |
| 192 | // Faster than at load is still 1.0: an idle machine's 105 ms repeat view fails the 100 ms bar. | |
| 193 | expect(loadScale(8, 9, 10)).toBe(1); | |
| 194 | expect(105 / loadScale(8, 9, 10)).toBeGreaterThan(100); | |
| 195 | // Load that arrives after the view started is seen by the after-sample (#5000's 105 ms run). | |
| 196 | expect(loadScale(10, 15, 10)).toBe(1.5); | |
| 197 | expect(loadScale(15, 10, 10)).toBe(1.5); | |
| 198 | expect(105 / loadScale(10, 15, 10)).toBeLessThan(100); | |
| 199 | expect(loadScale(10, 1000, 10)).toBe(8); | |
| 200 | }); | |
| 201 | ||
| 174 | 202 | test("the Files page of a 40-file diff: cold median of 3 and repeat median of 5 stay under their ceilings", async () => { |
| 203 | // Each view is scaled by the load measured beside it, not once up front: contention that begins | |
| 204 | // partway through the eight views would otherwise leave the ceilings unstretched (#5000). | |
| 175 | 205 | const cold: number[] = []; |
| 176 | // How much slower the machine is right now than at load: 1.0 when nothing else is running, so | |
| 177 | // the 250/100 ms ceilings bind as written; load stretches them, capped at 8x. | |
| 178 | const scale = Math.max(1, Math.min(fastest(3) / idle, 8)); | |
| 179 | 206 | for (let i = 0; i < 3; i++) { |
| 180 | 207 | // A new head each time, so nothing rendered before is reused. |
| 181 | 208 | await commitHead(i); |
| 182 | cold.push(await filesApp()); | |
| 209 | cold.push(await idleEquivalent(filesApp)); | |
| 183 | 210 | } |
| 184 | 211 | // The head is unchanged now: every view reuses the rendered diff. |
| 185 | 212 | const repeat: number[] = []; |
| 186 | for (let i = 0; i < 5; i++) repeat.push(await filesApp()); | |
| 187 | expect(median(cold)).toBeLessThan(COLD_MS * scale); | |
| 188 | expect(median(repeat)).toBeLessThan(100 * scale); | |
| 213 | for (let i = 0; i < 5; i++) repeat.push(await idleEquivalent(filesApp)); | |
| 214 | expect(median(cold)).toBeLessThan(COLD_MS); | |
| 215 | expect(median(repeat)).toBeLessThan(100); | |
| 189 | 216 | }); |
new above
offline
This assertion is true whenever the scale is 1, so it adds no coverage beyond the line above it. Keep the loadScale equality and drop this one, or exercise the real ceiling check through a helper.