Merge branch 'master' into measure-refactor

README.md

@@ -117,9 +117,11 @@ And in your config file:
 The following performance entry types are supported:
 
 - [`measure`](https://developer.mozilla.org/en-US/docs/Web/API/PerformanceMeasure):
-  Retrieve the `duration` of a user-defined interval between two marks.
+  Retrieve the `duration` of a user-defined interval between two marks. Use for
+  measuring the timing of a specific chunk of your code.
 - [`mark`](https://developer.mozilla.org/en-US/docs/Web/API/User_Timing_API/Using_the_User_Timing_API#Performance_measures):
-  Retrieve the `startTime` of a user-defined instant.
+  Retrieve the `startTime` of a user-defined instant. Use for measuring the time
+  between initial page navigation and a specific point in your code.
 - [`paint`](https://developer.mozilla.org/en-US/docs/Web/API/PerformancePaintTiming):
   Retrieve the `startTime` of a built-in paint measurement (e.g.
   `first-contentful-paint`).

src/cli.ts

@@ -24,7 +24,7 @@ import {Server} from './server';
 import {ResultStatsWithDifferences} from './stats';
 import {prepareVersionDirectory, makeServerPlans} from './versions';
 import {manualMode} from './manual';
-import {AutomaticMode} from './automatic';
+import {Runner} from './runner';
 import {runNpm} from './util';
 
 const installedVersion = (): string =>
@@ -150,7 +150,7 @@ $ tach http://example.com
     await manualMode(config, servers);
 
   } else {
-    const runner = new AutomaticMode(config, servers);
+    const runner = new Runner(config, servers);
     try {
       return await runner.run();
     } finally {

src/configfile.ts

@@ -15,7 +15,7 @@ import * as jsonschema from 'jsonschema';
 import {BrowserConfig, BrowserName, parseBrowserConfigString, validateBrowserConfig} from './browser';
 import {Config, parseHorizons, urlFromLocalPath} from './config';
 import * as defaults from './defaults';
-import {BenchmarkSpec, Measurement, PackageDependencyMap} from './types';
+import {BenchmarkSpec, Measurement, measurements, PackageDependencyMap} from './types';
 import {isHttpUrl} from './util';
 
 /**
@@ -278,7 +278,8 @@ export async function parseConfigFile(parsedJson: unknown):
   const result =
       jsonschema.validate(parsedJson, schema, {propertyName: 'config'});
   if (result.errors.length > 0) {
-    throw new Error(result.errors[0].toString());
+    throw new Error(
+        [...new Set(result.errors.map(customizeJsonSchemaError))].join('\n'));
   }
   const validated = parsedJson as ConfigFile;
   const root = validated.root || '.';
@@ -301,6 +302,19 @@ export async function parseConfigFile(parsedJson: unknown):
   };
 }
 
+/**
+ * Some of the automatically generated jsonschema errors are unclear, e.g. when
+ * there is a union of complex types they are reported as "[schema1],
+ * [schema2]" etc.
+ */
+function customizeJsonSchemaError(error: jsonschema.ValidationError): string {
+  if (error.property.match(/^config\.benchmarks\[\d+\]\.measurement$/)) {
+    return `${error.property} is not one of: ${[...measurements].join(', ')}` +
+        ' or an object like `performanceEntry: string`';
+  }
+  return error.toString();
+}
+
 async function parseBenchmark(benchmark: ConfigFileBenchmark, root: string):
     Promise<Partial<BenchmarkSpec>> {
   const spec: Partial<BenchmarkSpec> = {};

src/format.ts

@@ -275,36 +275,43 @@ const runtimeConfidenceIntervalDimension: Dimension = {
   tableConfig: {
     alignment: 'right',
   },
-  format: (r: ResultStats) =>
-      formatConfidenceInterval(r.stats.meanCI, (n) => n.toFixed(2) + 'ms'),
+  format: (r: ResultStats) => formatConfidenceInterval(r.stats.meanCI, milli),
 };
 
 function formatDifference({absolute, relative}: Difference): string {
   let word, rel, abs;
   if (absolute.low > 0 && relative.low > 0) {
     word = `[bold red]{slower}`;
-    rel = `${percent(relative.low)}% [gray]{-} ${percent(relative.high)}%`;
-    abs =
-        `${absolute.low.toFixed(2)}ms [gray]{-} ${absolute.high.toFixed(2)}ms`;
+    rel = formatConfidenceInterval(relative, percent);
+    abs = formatConfidenceInterval(absolute, milli);
 
   } else if (absolute.high < 0 && relative.high < 0) {
     word = `[bold green]{faster}`;
-    rel = `${percent(-relative.high)}% [gray]{-} ${percent(-relative.low)}%`;
-    abs = `${- absolute.high.toFixed(2)}ms [gray]{-} ${
-        - absolute.low.toFixed(2)}ms`;
+    rel = formatConfidenceInterval(negate(relative), percent);
+    abs = formatConfidenceInterval(negate(absolute), milli);
 
   } else {
     word = `[bold blue]{unsure}`;
-    rel = `${colorizeSign(relative.low, (n) => percent(n))}% [gray]{-} ${
-        colorizeSign(relative.high, (n) => percent(n))}%`;
-    abs = `${colorizeSign(absolute.low, (n) => n.toFixed(2))}ms [gray]{-} ${
-        colorizeSign(absolute.high, (n) => n.toFixed(2))}ms`;
+    rel = formatConfidenceInterval(relative, (n) => colorizeSign(n, percent));
+    abs = formatConfidenceInterval(absolute, (n) => colorizeSign(n, milli));
   }
+
   return ansi.format(`${word}\n${rel}\n${abs}`);
 }
 
 function percent(n: number): string {
-  return (n * 100).toFixed(0);
+  return (n * 100).toFixed(0) + '%';
+}
+
+function milli(n: number): string {
+  return n.toFixed(2) + 'ms';
+}
+
+function negate(ci: ConfidenceInterval): ConfidenceInterval {
+  return {
+    low: -ci.high,
+    high: -ci.low,
+  };
 }
 
 /**

src/measure.ts

@@ -13,7 +13,7 @@ import * as webdriver from 'selenium-webdriver';
 
 import {Server} from './server';
 import {Measurement, PerformanceEntryMeasurement} from './types';
-import {escapeStringLiteral, throwUnreachable} from './util';
+import {throwUnreachable} from './util';
 
 /**
  * Try to take a measurement in milliseconds from the given browser. Returns
@@ -108,4 +108,14 @@ async function queryForExpression(
     }
     return result;
   }
-}
+}
+
+/**
+ * Escape a string such that it can be safely embedded in a JavaScript template
+ * literal (backtick string).
+ */
+function escapeStringLiteral(unescaped: string): string {
+  return unescaped.replace(/\\/g, '\\\\')
+      .replace(/`/g, '\\`')
+      .replace(/\$/g, '\\$');
+}

src/automatic.ts → src/runner.ts

@@ -34,14 +34,14 @@ interface Browser {
   initialTabHandle: string;
 }
 
-export class AutomaticMode {
-  private config: Config;
-  private specs: BenchmarkSpec[];
-  private servers: Map<BenchmarkSpec, Server>;
-  private browsers = new Map<string, Browser>();
-  private bar: ProgressBar;
+export class Runner {
+  private readonly config: Config;
+  private readonly specs: BenchmarkSpec[];
+  private readonly servers: Map<BenchmarkSpec, Server>;
+  private readonly browsers = new Map<string, Browser>();
+  private readonly bar: ProgressBar;
+  private readonly specResults = new Map<BenchmarkSpec, BenchmarkResult[]>();
   private completeGithubCheck?: (markdown: string) => void;
-  private specResults = new Map<BenchmarkSpec, BenchmarkResult[]>();
   private hitTimeout = false;
 
   constructor(config: Config, servers: Map<BenchmarkSpec, Server>) {
@@ -140,38 +140,39 @@ export class AutomaticMode {
 
   private async takeAdditionalSamples() {
     const {config, specs, specResults} = this;
-    if (config.timeout > 0) {
-      console.log();
-      const timeoutMs = config.timeout * 60 * 1000;  // minutes -> millis
-      const startMs = Date.now();
-      let run = 0;
-      let sample = 0;
-      let elapsed = 0;
-      while (true) {
-        if (horizonsResolved(this.makeResults(), config.horizons)) {
-          console.log();
-          break;
-        }
-        if (elapsed >= timeoutMs) {
-          this.hitTimeout = true;
-          break;
-        }
-        // Run batches of 10 additional samples at a time for more presentable
-        // sample sizes, and to nudge sample sizes up a little.
-        for (let i = 0; i < 10; i++) {
-          sample++;
-          for (const spec of specs) {
-            run++;
-            elapsed = Date.now() - startMs;
-            const remainingSecs =
-                Math.max(0, Math.round((timeoutMs - elapsed) / 1000));
-            const mins = Math.floor(remainingSecs / 60);
-            const secs = remainingSecs % 60;
-            process.stdout.write(
-                `\r${spinner[run % spinner.length]} Auto-sample ${sample} ` +
-                `(timeout in ${mins}m${secs}s)` + ansi.erase.inLine(0));
-            specResults.get(spec)!.push(await this.takeSample(spec));
-          }
+    if (config.timeout <= 0) {
+      return;
+    }
+    console.log();
+    const timeoutMs = config.timeout * 60 * 1000;  // minutes -> millis
+    const startMs = Date.now();
+    let run = 0;
+    let sample = 0;
+    let elapsed = 0;
+    while (true) {
+      if (horizonsResolved(this.makeResults(), config.horizons)) {
+        console.log();
+        break;
+      }
+      if (elapsed >= timeoutMs) {
+        this.hitTimeout = true;
+        break;
+      }
+      // Run batches of 10 additional samples at a time for more presentable
+      // sample sizes, and to nudge sample sizes up a little.
+      for (let i = 0; i < 10; i++) {
+        sample++;
+        for (const spec of specs) {
+          run++;
+          elapsed = Date.now() - startMs;
+          const remainingSecs =
+              Math.max(0, Math.round((timeoutMs - elapsed) / 1000));
+          const mins = Math.floor(remainingSecs / 60);
+          const secs = remainingSecs % 60;
+          process.stdout.write(
+              `\r${spinner[run % spinner.length]} Auto-sample ${sample} ` +
+              `(timeout in ${mins}m${secs}s)` + ansi.erase.inLine(0));
+          specResults.get(spec)!.push(await this.takeSample(spec));
         }
       }
     }

src/test/configfile_test.ts

@@ -519,7 +519,8 @@ suite('config', () => {
           }],
         };
         await assert.isRejected(
-            parseConfigFile(config), 'config.benchmarks[0].measurement');
+            parseConfigFile(config),
+            'config.benchmarks[0].measurement is not one of: callback, fcp');
       });
 
       test('sampleSize too small', async () => {

src/util.ts

@@ -49,16 +49,6 @@ export async function runNpm(
   return promisify(execFile)(npmCmd, args, options).then(({stdout}) => stdout);
 }
 
-/**
- * Escape a string such that it can be safely embedded in a JavaScript template
- * literal (backtick string).
- */
-export function escapeStringLiteral(unescaped: string): string {
-  return unescaped.replace(/\\/g, '\\\\')
-      .replace(/`/g, '\\`')
-      .replace(/\$/g, '\\$');
-}
-
 /**
  * Promisified version of setTimeout.
  */