Skip to content

Commit b26b436

Browse files
authored
ci: balance macOS Electron smoke shards by measured duration (#9315)
* ci: balance desktop smoke shards by observed file durations * test: verify balancing guards and document duration evidence * style: use the script alias for shard planning * fix: select Playwright file suites rather than test declaration locations * test: prove desktop discovery preserves imported and unmeasured cases
1 parent 9f73c4a commit b26b436

7 files changed

Lines changed: 228 additions & 4 deletions

File tree

‎.github/CI.md‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,8 @@ The long-import and lease-expiration checks remain: they prove distinct survival
4343

4444
Local planner validation is `bun run test:scripts scripts/ci-changes.test.ts scripts/ci-gate.test.ts`. It executes real Turbo queries in small Git fixtures; no application build or database is required. Full builds, services, browser suites, and Electron checks run on hosted CI.
4545

46-
The separate `desktop-e2e` workflow retains its desktop-specific path filters. Its macOS shell smoke suite is partitioned by Playwright across three isolated runners; serial test-file groups stay intact. Packaging smoke and the weekly Electron canary remain separate. Each shard uploads its small JSON coverage/timing report.
46+
The separate `desktop-e2e` workflow retains its desktop-specific path filters. Its macOS shell smoke suite runs whole files across three isolated runners, with one worker per runner. `scripts/desktop-e2e.ts` asks Playwright to discover the complete suite, then assigns longer files first to the currently lightest shard using `apps/desktop/e2e-durations.json`. Exact escaped file filters preserve serial fixtures. Unmeasured files receive the median measured weight, deleted files cannot select tests, and invalid or empty selections fail. Normal local runs, packaging smoke and the weekly Electron canary keep their existing commands.
47+
48+
The initial weights are median test durations in seconds from runs [39734295953](https://github.com/simstudioai/sim/actions/runs/39734295953), [39833640524](https://github.com/simstudioai/sim/actions/runs/39833640524), and [39836596108](https://github.com/simstudioai/sim/actions/runs/39836596108). Refresh them from complete `desktop-smoke-reports-*` artifacts when the workload changes: sum `tests[].results[].duration` by `spec.file` in each run, take the median across runs, and divide milliseconds by 1,000. The weights steer scheduling only; Playwright discovery owns coverage. Before adopting new weights, compare repeated paired runs and the complete case/status multiset, including skipped cases and retries.
49+
50+
Each shard uploads its coverage/timing report, the complete discovery report, and its deterministic shard plan. `bun run test:scripts scripts/duration-shards.test.ts scripts/desktop-e2e.test.ts` checks partition coverage, unknown files, deterministic ordering, and balancing. The CLI regression executes tiny browser-free Playwright fixtures to prove that imported test factories and paths containing regex characters are selected correctly; it launches no Electron or browser.

‎.github/workflows/desktop-e2e.yml‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
name: desktop-e2e
22

33
# Smoke coverage of the real Electron shell on desktop changes. Three isolated runners
4-
# partition the complete suite by Playwright test-file groups; workers remain serial per runner.
4+
# partition complete test files by measured duration; workers remain serial per runner.
55
# The weekly
66
# advisory canary against electron@latest so Chromium-cadence breakage surfaces
77
# before an upgrade is attempted.
@@ -47,6 +47,8 @@ on:
4747
- 'packages/utils/**'
4848
- 'bun.lock'
4949
- 'package.json'
50+
- 'scripts/desktop-e2e.ts'
51+
- 'scripts/duration-shards.ts'
5052
schedule:
5153
- cron: '23 9 * * 1'
5254
workflow_dispatch:
@@ -108,7 +110,7 @@ jobs:
108110

109111
- name: Run Playwright _electron smoke suite
110112
working-directory: apps/desktop
111-
run: bunx playwright test --shard=${{ matrix.shard }}/3 --reporter=line,json
113+
run: bun run ../../scripts/desktop-e2e.ts ${{ matrix.shard }}/3
112114
env:
113115
PLAYWRIGHT_JSON_OUTPUT_FILE: test-results/desktop-smoke.json
114116
BACKGROUND_EXECUTOR_REPORT_PATH: test-results/background-executor-report.json
@@ -127,7 +129,10 @@ jobs:
127129
uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2
128130
with:
129131
name: desktop-smoke-reports-${{ matrix.shard }}
130-
path: apps/desktop/test-results/desktop-smoke.json
132+
path: |
133+
apps/desktop/test-results/desktop-smoke.json
134+
apps/desktop/test-results/desktop-shard-plan.json
135+
apps/desktop/test-results/desktop-discovery.json
131136
if-no-files-found: ignore
132137
retention-days: 7
133138

‎apps/desktop/e2e-durations.json‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
{
2+
"background-executor.spec.ts": 131.12,
3+
"browser-chrome.spec.ts": 15.28,
4+
"browser-focus.spec.ts": 6.53,
5+
"browser-page-dialogs.spec.ts": 11.81,
6+
"browser-tools.spec.ts": 94.66,
7+
"chat-panel.spec.ts": 31.08,
8+
"check-report.spec.ts": 3.78,
9+
"computer-use.spec.ts": 2.07,
10+
"desktop-tools-live-sim.spec.ts": 0.0,
11+
"local-files.spec.ts": 27.68,
12+
"packaged-smoke.spec.ts": 0.0,
13+
"password-autofill.spec.ts": 46.56,
14+
"session-cookies.spec.ts": 11.45,
15+
"smoke.spec.ts": 25.56,
16+
"source-connect.spec.ts": 41.49,
17+
"terminal-cancel.spec.ts": 199.76,
18+
"update-notification.spec.ts": 19.55,
19+
"updater.spec.ts": 2.48
20+
}

‎scripts/desktop-e2e.test.ts‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import { spawnSync } from 'node:child_process'
2+
import { mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'
3+
import { tmpdir } from 'node:os'
4+
import { join, resolve } from 'node:path'
5+
import { expect, it } from 'vitest'
6+
7+
it('executes imported test factories and unmeasured files exactly once across desktop shards', () => {
8+
const root = resolve(import.meta.dirname, '..')
9+
const fixture = mkdtempSync(join(tmpdir(), 'desktop-shard-'))
10+
try {
11+
symlinkSync(join(root, 'node_modules'), join(fixture, 'node_modules'), 'dir')
12+
writeFileSync(join(fixture, 'package.json'), '{"type":"module"}')
13+
writeFileSync(
14+
join(fixture, 'playwright.config.ts'),
15+
"export default { testDir: './tests', workers: 1, retries: 0 }"
16+
)
17+
mkdirSync(join(fixture, 'tests/nested'), { recursive: true })
18+
mkdirSync(join(fixture, 'tests/support'), { recursive: true })
19+
writeFileSync(
20+
join(fixture, 'tests/support/factory.ts'),
21+
"import { test } from '@playwright/test'; export function register() { test('factory', () => {}); }"
22+
)
23+
writeFileSync(
24+
join(fixture, 'tests/nested/a[1].spec.ts'),
25+
"import { register } from '../support/factory'; register();"
26+
)
27+
for (const file of ['brand-new.test.ts', 'ordinary.spec.ts']) {
28+
writeFileSync(
29+
join(fixture, 'tests', file),
30+
`import { test } from '@playwright/test'; test('${file}', () => {});`
31+
)
32+
}
33+
const cases: { title: string; status: string }[] = []
34+
for (const index of [1, 2, 3]) {
35+
const reportPath = join(fixture, `${index}.json`)
36+
const result = spawnSync('bun', ['run', join(root, 'scripts/desktop-e2e.ts'), `${index}/3`], {
37+
cwd: fixture,
38+
env: { ...process.env, PLAYWRIGHT_JSON_OUTPUT_FILE: reportPath },
39+
encoding: 'utf8',
40+
timeout: 10_000,
41+
})
42+
expect(result.status, result.stdout + result.stderr).toBe(0)
43+
const report = JSON.parse(readFileSync(reportPath, 'utf8')) as {
44+
suites: { specs: { title: string; tests: { results: { status: string }[] }[] }[] }[]
45+
}
46+
for (const suite of report.suites) {
47+
for (const spec of suite.specs) {
48+
for (const test of spec.tests) {
49+
for (const run of test.results) cases.push({ title: spec.title, status: run.status })
50+
}
51+
}
52+
}
53+
}
54+
expect(cases).toEqual(
55+
expect.arrayContaining([
56+
{ title: 'factory', status: 'passed' },
57+
{ title: 'brand-new.test.ts', status: 'passed' },
58+
{ title: 'ordinary.spec.ts', status: 'passed' },
59+
])
60+
)
61+
expect(cases).toHaveLength(3)
62+
} finally {
63+
rmSync(fixture, { recursive: true, force: true })
64+
}
65+
})

‎scripts/desktop-e2e.ts‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
import { spawnSync } from 'node:child_process'
2+
import { mkdirSync, readFileSync, writeFileSync } from 'node:fs'
3+
import { resolve } from 'node:path'
4+
import { partitionFiles } from '@scripts/duration-shards'
5+
import { createLogger } from '@sim/logger'
6+
import { escapeRegExp } from '@sim/utils/string'
7+
8+
interface DiscoveryReport {
9+
config: { rootDir: string }
10+
suites: { file: string }[]
11+
errors: unknown[]
12+
}
13+
14+
const logger = createLogger('DesktopE2E')
15+
const match = /^(\d+)\/(\d+)$/.exec(process.argv[2] ?? '')
16+
if (!match) throw new Error('Usage: bun run ../../scripts/desktop-e2e.ts <shard>/<total>')
17+
const index = Number(match[1])
18+
const count = Number(match[2])
19+
if (!Number.isSafeInteger(index) || !Number.isSafeInteger(count) || index < 1 || index > count) {
20+
throw new Error('Invalid desktop shard index or count')
21+
}
22+
23+
mkdirSync('test-results', { recursive: true })
24+
const discoveryPath = resolve('test-results/desktop-discovery.json')
25+
const discovery = spawnSync('bunx', ['playwright', 'test', '--list', '--reporter=json'], {
26+
stdio: 'inherit',
27+
env: { ...process.env, PLAYWRIGHT_JSON_OUTPUT_FILE: discoveryPath },
28+
})
29+
if (discovery.error) throw discovery.error
30+
if (discovery.status !== 0) process.exit(discovery.status ?? 1)
31+
const discoveryJson = readFileSync(discoveryPath, 'utf8')
32+
const report = JSON.parse(discoveryJson) as DiscoveryReport
33+
if (report.errors.length > 0) throw new Error('Playwright discovery reported errors')
34+
// A test factory can declare tests in a helper; the outer suite owns the discoverable test file.
35+
const files = new Set(report.suites.map((suite) => suite.file))
36+
if (count > files.size) throw new Error('More desktop shards than discovered test files')
37+
const durations = JSON.parse(
38+
readFileSync(new URL('../apps/desktop/e2e-durations.json', import.meta.url), 'utf8')
39+
) as Record<string, number>
40+
const shards = partitionFiles([...files], durations, count)
41+
const selected = shards[index - 1]
42+
if (selected.length === 0) throw new Error('Desktop shard selected no test files')
43+
logger.info(`Running desktop shard ${index}/${count}: ${selected.join(', ')}`)
44+
const run = spawnSync(
45+
'bunx',
46+
[
47+
'playwright',
48+
'test',
49+
...selected.map((file) => `^${escapeRegExp(resolve(report.config.rootDir, file))}$`),
50+
'--reporter=line,json',
51+
],
52+
{ stdio: 'inherit', env: process.env }
53+
)
54+
mkdirSync('test-results', { recursive: true })
55+
writeFileSync(discoveryPath, discoveryJson)
56+
writeFileSync(
57+
'test-results/desktop-shard-plan.json',
58+
`${JSON.stringify({ index, count, discoveredFiles: files.size, shards }, null, 2)}\n`
59+
)
60+
if (run.error) throw run.error
61+
process.exit(run.status ?? 1)

‎scripts/duration-shards.test.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import { partitionFiles } from '@scripts/duration-shards'
2+
import { describe, expect, it } from 'vitest'
3+
4+
describe('duration-balanced file partitions', () => {
5+
it('keeps whole files and assigns every discovered file exactly once, including unmeasured files', () => {
6+
const files = ['a.spec.ts', 'b.spec.ts', 'new/nested.test.ts', 'unmeasured.spec.ts']
7+
const shards = partitionFiles(
8+
files,
9+
{ 'a.spec.ts': 100, 'b.spec.ts': 1, 'deleted.spec.ts': 999 },
10+
3
11+
)
12+
expect(shards.flat().sort()).toEqual([...files].sort())
13+
expect(shards.every((shard) => shard.length > 0)).toBe(true)
14+
})
15+
16+
it('separates long files instead of packing them into the same shard', () => {
17+
const weights = { a: 90, b: 20, c: 10, d: 100, e: 80 }
18+
const shards = partitionFiles(Object.keys(weights), weights, 3)
19+
const loads = shards.map((files) =>
20+
files.reduce((sum, file) => sum + weights[file as keyof typeof weights], 0)
21+
)
22+
expect(Math.max(...loads)).toBe(100)
23+
})
24+
25+
it('gives all runners the same partition regardless of discovery order and equal weights', () => {
26+
const files = ['z', 'b', 'a', 'nested/c', 'x']
27+
expect(partitionFiles(files, {}, 3)).toEqual(partitionFiles([...files].reverse(), {}, 3))
28+
})
29+
30+
it('preserves zero-duration files without placing every skipped file in one empty shard', () => {
31+
const shards = partitionFiles(['a', 'b', 'c'], { a: 0, b: 0, c: 0 }, 3)
32+
expect(shards.map((files) => files.length)).toEqual([1, 1, 1])
33+
})
34+
35+
it.each([0, -1, 1.5, Number.NaN, Number.POSITIVE_INFINITY])(
36+
'rejects invalid shard count %s',
37+
(count) => {
38+
expect(() => partitionFiles(['a'], {}, count)).toThrow('shard count')
39+
}
40+
)
41+
})

‎scripts/duration-shards.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import { compareStrings } from '@sim/utils/string'
2+
3+
/** Partitions discovered files without splitting their serial fixtures or omitting unmeasured files. */
4+
export function partitionFiles(
5+
files: readonly string[],
6+
durations: Record<string, number>,
7+
count: number
8+
): string[][] {
9+
if (!Number.isSafeInteger(count) || count < 1) throw new Error('Invalid shard count')
10+
const measured = files
11+
.map((file) => durations[file])
12+
.filter((duration) => Number.isFinite(duration) && duration > 0)
13+
.sort((a, b) => a - b)
14+
const fallback = measured[Math.floor(measured.length / 2)] ?? 1
15+
const weight = (file: string) =>
16+
Number.isFinite(durations[file]) && durations[file] >= 0
17+
? Math.max(1, durations[file])
18+
: fallback
19+
const shards = Array.from({ length: count }, () => ({ files: [] as string[], seconds: 0 }))
20+
for (const file of [...files].sort((a, b) => weight(b) - weight(a) || compareStrings(a, b))) {
21+
const target = shards.reduce((smallest, shard) =>
22+
shard.seconds < smallest.seconds ? shard : smallest
23+
)
24+
target.files.push(file)
25+
target.seconds += weight(file)
26+
}
27+
return shards.map((shard) => shard.files.sort(compareStrings))
28+
}

0 commit comments

Comments
 (0)