Skip to content

Commit a852f2f

Browse files
committed
Skip costly tests
1. Add a script to generate a sorted list of most costly tests. A tests' cost is roughly `runtime% / number of edits`. A slow test that's only been updated once is much less valuable than a slow test that has been updated 20 times: the latter test is catching more changes in the type system. 2. Check in the results of running this script. I want to make the skipping behaviour deterministic and the same for everybody, even though you may get slightly better performance by examining only *your* test changes. 3. Add code to skip tests until it reaches a 5% chance of missing an edit. Right now this provides a 38% speedup. Still not done: 4. Make this value configurable. 5. Make the CI configuration specify a 0% chance of missing an edit.
1 parent 81f7153 commit a852f2f

File tree

4 files changed

+139
-4
lines changed

4 files changed

+139
-4
lines changed

.test-cost.json

Lines changed: 1 addition & 0 deletions
Large diffs are not rendered by default.

package.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@
8282
"prex": "^0.4.3",
8383
"q": "latest",
8484
"remove-internal": "^2.9.2",
85+
"simple-git": "^1.113.0",
8586
"source-map-support": "latest",
8687
"through2": "latest",
8788
"travis-fold": "latest",
@@ -102,7 +103,8 @@
102103
"gulp": "gulp",
103104
"jake": "gulp",
104105
"lint": "gulp lint",
105-
"setup-hooks": "node scripts/link-hooks.js"
106+
"setup-hooks": "node scripts/link-hooks.js",
107+
"update-costly-tests": "node scripts/costly-tests.js"
106108
},
107109
"browser": {
108110
"fs": false,

scripts/costly-tests.js

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
// @ts-check
2+
const fs = require("fs");
3+
const git = require('simple-git/promise')('.')
4+
const readline = require('readline')
5+
6+
/** @typedef {{ [s: string]: number}} Histogram */
7+
8+
async function main() {
9+
/** @type {Histogram} */
10+
const edits = Object.create(null)
11+
/** @type {Histogram} */
12+
const perf = JSON.parse(fs.readFileSync('.parallelperf.json', 'utf8'))
13+
14+
await collectCommits(git, "release-2.3", "master", /*author*/ undefined, files => fillMap(files, edits))
15+
16+
const totalTime = Object.values(perf).reduce((n,m) => n + m, 0)
17+
const untouched = Object.values(perf).length - Object.values(edits).length
18+
const totalEdits = Object.values(edits).reduce((n,m) => n + m, 0) + untouched + Object.values(edits).length
19+
20+
let i = 0
21+
/** @type {{ name: string, time: number, edits: number, cost: number }[]} */
22+
let data = []
23+
for (const k in perf) {
24+
const otherk = k.replace(/tsrunner-[a-z-]+?:\/\//, '')
25+
const percentTime = perf[k] / totalTime
26+
const percentHits = (1 + (edits[otherk] || 0)) / totalEdits
27+
const cost = 5 + Math.log(percentTime / percentHits)
28+
// TODO: Write counts instead of numbers to make JSON file smaller
29+
data.push({ name: otherk, time: perf[k], edits: 1 + (edits[otherk] || 0), cost})
30+
if (edits[otherk])
31+
i++
32+
}
33+
const output = {
34+
totalTime,
35+
totalEdits,
36+
data: data.sort((x,y) => y.cost - x.cost).map(x => ({ ...x, cost: x.cost.toFixed(2) }))
37+
}
38+
39+
fs.writeFileSync('.test-cost.json', JSON.stringify(output), 'utf8')
40+
}
41+
42+
main().catch(e => {
43+
console.log(e);
44+
process.exit(1);
45+
})
46+
47+
/**
48+
* @param {string[]} files
49+
* @param {Histogram} histogram
50+
*/
51+
function fillMap(files, histogram) {
52+
// keep edits to test cases (but not /users), and not file moves
53+
const tests = files.filter(f => f.startsWith('tests/cases/') && !f.startsWith('tests/cases/user') && !/=>/.test(f))
54+
for (const test of tests) {
55+
histogram[test] = (histogram[test] || 0) + 1
56+
}
57+
}
58+
59+
/**
60+
* @param {string} s
61+
*/
62+
function isSquashMergeMessage(s) {
63+
return /\(#[0-9]+\)$/.test(s)
64+
}
65+
66+
/**
67+
* @param {string} s
68+
*/
69+
function isMergeCommit(s) {
70+
return /Merge pull request #[0-9]+/.test(s)
71+
}
72+
73+
/**
74+
* @param {string} s
75+
*/
76+
function parseFiles(s) {
77+
const lines = s.split('\n')
78+
// Note that slice(2) only works for merge commits, which have an empty newline after the title
79+
return lines.slice(2, lines.length - 2).map(line => line.split("|")[0].trim())
80+
}
81+
82+
/**
83+
* @param {import('simple-git/promise').SimpleGit} git
84+
* @param {string} from
85+
* @param {string} to
86+
* @param {string | undefined} author - only include commits from this author
87+
* @param {(files: string[]) => void} update
88+
*/
89+
async function collectCommits(git, from, to, author, update) {
90+
let i = 0
91+
for (const commit of (await git.log({ from, to })).all) {
92+
i++
93+
if ((!author || commit.author_name === author) && isMergeCommit(commit.message) || isSquashMergeMessage(commit.message)) {
94+
readline.clearLine(process.stdout, /*left*/ -1)
95+
readline.cursorTo(process.stdout, 0)
96+
process.stdout.write(i + ": " + commit.date)
97+
const files = parseFiles(await git.show([commit.hash, "--stat=1000,960,40", "--pretty=oneline"]))
98+
update(files)
99+
}
100+
}
101+
}
102+
103+
104+

src/testRunner/parallel/host.ts

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ namespace Harness.Parallel.Host {
1414
const isatty = tty.isatty(1) && tty.isatty(2);
1515
const path = require("path") as typeof import("path");
1616
const { fork } = require("child_process") as typeof import("child_process");
17-
const { statSync } = require("fs") as typeof import("fs");
17+
const { statSync, readFileSync } = require("fs") as typeof import("fs");
18+
19+
const editSkipRate = 0.05
1820

1921
// NOTE: paths for module and types for FailedTestReporter _do not_ line up due to our use of --outFile for run.js
2022
// tslint:disable-next-line:variable-name
@@ -192,7 +194,32 @@ namespace Harness.Parallel.Host {
192194
return `tsrunner-${runner}://${test}`;
193195
}
194196

195-
function startDelayed(perfData: { [testHash: string]: number } | undefined, totalCost: number) {
197+
function skipCostlyTests(tasks: Task[], editSkipRate: number) {
198+
if (statSync('.test-cost.json')) {
199+
const costs = JSON.parse(readFileSync('.test-cost.json', 'utf8')) as {
200+
totalTime: number,
201+
totalEdits: number,
202+
data: Array<{ name: string, time: number, edits: number, costs: number }>
203+
}
204+
let skippedEdits = 0;
205+
let skippedTests = new Set<string>();
206+
let skippedTime = 0;
207+
let i = 0;
208+
for (; i < costs.data.length && (skippedEdits / costs.totalEdits) < editSkipRate; i++) {
209+
skippedEdits += costs.data[i].edits;
210+
skippedTime += costs.data[i].time;
211+
skippedTests.add(costs.data[i].name);
212+
}
213+
console.log(`Skipped ${i} expensive tests; estimated time savings of ${(skippedTime / costs.totalTime * 100).toFixed(2)}% with ${(editSkipRate * 100).toFixed(2)}% chance of missing a test.`)
214+
return tasks.filter(t => !skippedTests.has(t.file));
215+
}
216+
else {
217+
console.log('No cost analysis discovered.');
218+
return tasks;
219+
}
220+
}
221+
222+
function startDelayed(perfData: { [testHash: string]: number } | undefined, totalCost: number, editSkipRate: number) {
196223
console.log(`Discovered ${tasks.length} unittest suites` + (newTasks.length ? ` and ${newTasks.length} new suites.` : "."));
197224
console.log("Discovering runner-based tests...");
198225
const discoverStart = +(new Date());
@@ -231,6 +258,7 @@ namespace Harness.Parallel.Host {
231258
}
232259
tasks.sort((a, b) => a.size - b.size);
233260
tasks = tasks.concat(newTasks);
261+
tasks = skipCostlyTests(tasks, editSkipRate);
234262
const batchCount = workerCount;
235263
const packfraction = 0.9;
236264
const chunkSize = 1000; // ~1KB or 1s for sending batches near the end of a test
@@ -625,6 +653,6 @@ namespace Harness.Parallel.Host {
625653
}
626654

627655
// tslint:disable-next-line:ban
628-
setTimeout(() => startDelayed(perfData, totalCost), 0); // Do real startup on next tick, so all unit tests have been collected
656+
setTimeout(() => startDelayed(perfData, totalCost, editSkipRate), 0); // Do real startup on next tick, so all unit tests have been collected
629657
}
630658
}

0 commit comments

Comments
 (0)