Skip to content

Commit

Permalink
remove warm up runs
Browse files Browse the repository at this point in the history
  • Loading branch information
csabapalfi committed Jun 12, 2019
1 parent 6f99dfe commit 1493e28
Show file tree
Hide file tree
Showing 11 changed files with 23 additions and 43 deletions.
2 changes: 0 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,6 @@ min 95 0.9 1.0 1.0 3.1 3.7
max 96 0.9 1.0 1.2 3.5 4.0
```

* `--warmup-runs <N>` add warmup runs that are excluded from stats (e.g. to allow CDN or other caches to warm up)

### Force AB tests variants

By making sure we always test the same variants of any AB tests running on the page we can ensure they don’t introduce [Page Nondeterminism](https://docs.google.com/document/d/1BqtL-nG53rxWOI5RO0pItSRPowZVnYJ_gBEQCJ5EeUE/edit#heading=h.js7k0ib0mzzv).
Expand Down
5 changes: 5 additions & 0 deletions TODO.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# TODO

* retry on errors
* show errors to user
* run API calls in parallel (limiting max concurrency)
10 changes: 2 additions & 8 deletions lib/cli-options.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,6 @@ const options = {
group: 'Runs:',
default: 1,
},
'warmup-runs': {
type: 'number',
describe: 'Number of warmup runs',
group: 'Runs:',
default: 0,
},
'usertiming-marks': {
describe: 'User Timing marks',
group: 'Additional metrics:',
Expand Down Expand Up @@ -106,9 +100,9 @@ function parseArgs({argv}) {
.parse();

const url = positionalArgAt(args, 0);
const {runs, warmupRuns, metrics, output, lighthouse} = args;
const {runs, metrics, output, lighthouse} = args;

return {url, runs, warmupRuns, metrics, output, lighthouse};
return {url, runs, metrics, output, lighthouse};
}

module.exports = {parseArgs, check};
12 changes: 5 additions & 7 deletions lib/counter.js
Original file line number Diff line number Diff line change
@@ -1,20 +1,18 @@
class Counter {
constructor(runs, warmupRuns) {
this.warmupRuns = warmupRuns;
this.maxRuns = runs + warmupRuns;
constructor(runs) {
this.runs = runs;
this.run = 0;
}

next() {
const run = ++this.run;
return {
value: {
index: run - this.warmupRuns,
index: run,
first: run === 1,
warmup: run <= this.warmupRuns,
last: run === this.maxRuns,
last: run === this.runs,
},
done: run > this.maxRuns
done: run > this.runs
};
}

Expand Down
4 changes: 2 additions & 2 deletions lib/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ const {Formatter} = require('./formatter');
const {Runner} = require('./runner');

function main({
runs, warmupRuns,
runs,
url, metrics, lighthouse,
output,
}) {
const counter = new Counter(runs, warmupRuns);
const counter = new Counter(runs);
const fetcher = new Fetcher(url, metrics, lighthouse, output);
const formatter = new Formatter(output.jsonl);
return new Runner(counter, fetcher, formatter);
Expand Down
6 changes: 2 additions & 4 deletions lib/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,8 @@ class Runner {

const result = await fetcher.getResult(run.index);

if (!run.warmup) {
samples.push(result);
}

samples.push(result);

const output = [...formatter.format([result], run)];

if (run.last && samples.length > 1) {
Expand Down
1 change: 0 additions & 1 deletion tests/__snapshots__/cli-options.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,5 @@ Object {
},
"runs": 1,
"url": "https://www.google.com",
"warmupRuns": 0,
}
`;
3 changes: 1 addition & 2 deletions tests/__snapshots__/main.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@
exports[`main returns runner 1`] = `
Runner {
"counter": Counter {
"maxRuns": 1,
"run": 0,
"warmupRuns": 0,
"runs": 1,
},
"fetcher": Fetcher {
"getLighthouseResult": [Function],
Expand Down
16 changes: 6 additions & 10 deletions tests/counter.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ describe('Counter', () => {
it('returns a single first and last flag in the correct positions', () => {
fc.assert(
fc.property(
fc.integer(1, 100), fc.integer(0, 100), (r, w) => {
const runs = [...new Counter(r, w)];
fc.integer(1, 100), (r) => {
const runs = [...new Counter(r)];
expect(runs.filter(({first}) => first)).toHaveLength(1);
expect(runs[0]).toHaveProperty('first', true);
expect(runs.filter(({last}) => last)).toHaveLength(1);
Expand All @@ -16,16 +16,12 @@ describe('Counter', () => {
);
});

it('returns the configured number of warmup and normal runs', () => {
it('returns the configured number of runs', () => {
fc.assert(
fc.property(
fc.integer(1, 100), fc.integer(0, 100), (r, w) => {
const runs = [...new Counter(r, w)];
expect(runs.map(({warmup}) => warmup))
.toEqual([
...new Array(w).fill(true),
...new Array(r).fill(false)
])
fc.integer(1, 100), (r) => {
const runs = [...new Counter(r)];
expect(runs.length).toEqual(r)
}
)
);
Expand Down
1 change: 0 additions & 1 deletion tests/main.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ const {main} = require('../lib/main');
describe('main', () => {
const options = {
runs: 1,
warmupRuns: 0,
url: 'https://www.google.com',
metrics: {userTimingMarks: {}},
lighthouse: {enabled: false},
Expand Down
6 changes: 0 additions & 6 deletions tests/runner.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,6 @@ describe('runner', () => {
expect(runner.samples).toContain(result);
});

it('that is not added to samples if warmup result ', async () => {
const runner = new Runner(counter({warmup: true}), fetcher, formatter);
await expect(runner.next()).resolves.toEqual(resolvedResult);
expect(runner.samples).not.toContain(result);
});

it('and no stats if last run but not enough samples (<2)', () => {
const runner = new Runner(counter({last: true}), fetcher, formatter);
return expect(runner.next()).resolves.toEqual(resolvedResult);
Expand Down

0 comments on commit 1493e28

Please sign in to comment.