refactor extra_arg parsing into QueryReader

blackmad · blackmad · commit 97b747822595 · 2020-07-14T19:09:15.000-04:00
diff --git a/.eslintrc.json b/.eslintrc.json
@@ -3,9 +3,13 @@
     "commonjs": true,
     "es6": true
   },
-  "extends": ["airbnb-base",     "plugin:import/typescript",    "plugin:@typescript-eslint/recommended", "plugin:jsdoc/recommended"],
+  "extends": [
+    "airbnb-base",
+    "plugin:import/typescript",
+    "plugin:@typescript-eslint/recommended",
+    "plugin:jsdoc/recommended"
+  ],
   "parser": "@typescript-eslint/parser",
-  "plugins": ["jsdoc"],
   "rules": {
     "import/extensions": 0,
     "jsdoc/no-undefined-types": 0,
diff --git a/lib/compare/compare.ts b/lib/compare/compare.ts
@@ -1,11 +1,10 @@
 /* eslint-disable camelcase */
-import * as queryString from 'query-string';
 import * as Bluebird from 'bluebird';
 
 import * as jsondiffpatch from 'jsondiffpatch';
 import { AxiosResponse } from 'axios';
 import { ApiEnv, argvToApiEnv } from '../apiEnv';
-import runQuery from '../run-query';
+import runQuery, { AxiosResponseWithDuration } from '../run-query';
 import { Change } from './change';
 import { CompareFormatter } from './formatters/compare-formatter';
 import {
@@ -15,12 +14,16 @@ import getFormatter from './formatters/get-formatter';
 import QueryReader from './query-reader';
 import { Query } from './query';
 
+type CompareArgs = Pick<ParsedArgs, 'concurrency' | 'ignored_fields' | 'extra_params' | 'timeout'>;
+
 /**
- * @param root0
- * @param root0.oldApiEnv
- * @param root0.newApiEnv
- * @param root0.query
- * @param root0.argv
+ * Compare one query against two ApiEnvs, returning a Change object
+ *
+ * @param {ApiEnv} oldApiEnv old env to run query against
+ * @param {ApiEnv} newApiEnv new env to run query against
+ * @param {Query} query query to compare
+ * @param {CompareArgs} argv global command line args that affect running queries
+ * @returns {Promise<Change>} output of running query against both envs
  */
 async function compareQuery({
   oldApiEnv,
@@ -31,23 +34,14 @@ async function compareQuery({
   oldApiEnv: ApiEnv;
   newApiEnv: ApiEnv;
   query: Query;
-  argv: ParsedArgs;
+  argv: CompareArgs;
 }): Promise<Change> {
-  const extraParams = queryString.parse(argv.extra_params.join('&')) as Record<string, string>;
-  delete extraParams.undefined;
-  const params = { ...query.params, ...extraParams };
-  const queryWithExtraParams = {
-    endpoint: query.endpoint,
-    method: query.method,
-    params,
-  };
-
   // if the query has a baseline response (running from a golden json file), use that
   // otherwise run it against the old server
   const oldResponse = query.baselineResponse
-    ? ({ data: query.baselineResponse } as AxiosResponse<any>)
-    : await runQuery(oldApiEnv, queryWithExtraParams, argv.timeout);
-  const newResponse = await runQuery(newApiEnv, queryWithExtraParams, argv.timeout);
+    ? ({ data: query.baselineResponse } as AxiosResponse<unknown>)
+    : await runQuery(oldApiEnv, query, argv.timeout);
+  const newResponse = await runQuery(newApiEnv, query, argv.timeout);
 
   const differ = jsondiffpatch.create({
     propertyFilter(name) {
@@ -58,20 +52,23 @@ async function compareQuery({
   const delta = differ.diff(oldResponse.data, newResponse.data);
 
   return {
-    query: queryWithExtraParams,
+    query,
     delta,
     oldResponse,
     newResponse,
   };
 }
 
 /**
- * @param root0
- * @param root0.oldApiEnv
- * @param root0.newApiEnv
- * @param root0.queries
- * @param root0.argv
- * @param root0.formatter
+ * Compare and output many queries against two ApiEnvs. Passing along
+ * the changes for output to a ChangeFormatter.
+ *
+ * @param {ApiEnv} oldApiEnv old env to run query against
+ * @param {ApiEnv} newApiEnv new env to run query against
+ * @param {Query[]} queries queries to compare
+ * @param {CompareArgs} argv global command line args that affect running queries
+ * @param {CompareFormatter} formatter formatter to use for output
+ * @returns {Promise<void>} side effect only - outputs to output_file or stdout
  */
 async function compareQueries({
   oldApiEnv,
@@ -83,9 +80,9 @@ async function compareQueries({
   oldApiEnv: ApiEnv;
   newApiEnv: ApiEnv;
   queries: Query[];
-  argv: ParsedArgs;
+  argv: CompareArgs;
   formatter: CompareFormatter;
-}) {
+}): Promise<void> {
   const oldResponseTimes: number[] = [];
   const newResponseTimes: number[] = [];
   const oldStatusCodes: Record<number, number> = {};
@@ -100,12 +97,11 @@ async function compareQueries({
         query,
         argv,
       });
-      formatter.queryRan();
-      if (change.delta || argv.unchanged) {
-        formatter.logChange(change);
-      }
-      oldResponseTimes.push((change.oldResponse as any).duration);
-      newResponseTimes.push((change.newResponse as any).duration);
+
+      formatter.queryCompleted(change);
+
+      oldResponseTimes.push((change.oldResponse as AxiosResponseWithDuration).duration);
+      newResponseTimes.push((change.newResponse as AxiosResponseWithDuration).duration);
 
       if (!oldStatusCodes[change.oldResponse.status]) {
         oldStatusCodes[change.oldResponse.status] = 0;
@@ -131,24 +127,33 @@ async function compareQueries({
   });
 }
 
-const argv = parseArgv([OLD_KEY, NEW_KEY]) as ParsedArgs;
+/** Main logic - parses command line, creates a formatter, runs queries and
+ * generates output.
+ *
+ * @returns {Promise<void>} on completion
+ */
+function main(): Promise<void> {
+  const argv = parseArgv([OLD_KEY, NEW_KEY]) as ParsedArgs;
 
-const oldApiEnv = argvToApiEnv(argv[OLD_KEY]);
-const newApiEnv = argvToApiEnv(argv[NEW_KEY]);
+  const oldApiEnv = argvToApiEnv(argv[OLD_KEY]);
+  const newApiEnv = argvToApiEnv(argv[NEW_KEY]);
 
-const queries = QueryReader(argv);
+  const queries = QueryReader(argv);
 
-const formatter = getFormatter(argv.output_mode, {
-  oldApiEnv,
-  newApiEnv,
-  argv,
-  totalQueries: queries.length,
-});
+  const formatter = getFormatter(argv.output_mode, {
+    oldApiEnv,
+    newApiEnv,
+    argv,
+    totalQueries: queries.length,
+  });
 
-compareQueries({
-  oldApiEnv,
-  newApiEnv,
-  queries,
-  argv,
-  formatter,
-});
+  return compareQueries({
+    oldApiEnv,
+    newApiEnv,
+    queries,
+    argv,
+    formatter,
+  });
+}
+
+main();
diff --git a/lib/compare/formatters/compare-formatter.ts b/lib/compare/formatters/compare-formatter.ts
@@ -1,10 +1,11 @@
+/* eslint-disable no-console */
 import * as fs from 'fs';
 import * as stats from 'stats-lite';
 import { Change } from '../change';
 import { ApiEnv } from '../../apiEnv';
 import { ParsedArgs } from '../argv';
 
-export type FormatterArgv = Pick<ParsedArgs, 'output_mode' | 'output_file'>;
+export type FormatterArgv = Pick<ParsedArgs, 'output_mode' | 'output_file' | 'unchanged'>;
 
 export type FormatterConstructorParams = {
   oldApiEnv: ApiEnv;
@@ -19,7 +20,7 @@ export type PerHostFinishedStats = {
 }
 
 export type FinishedStats = {
-  new: PerHostFinishedStats,
+  new?: PerHostFinishedStats,
   old: PerHostFinishedStats
 }
 
@@ -40,14 +41,6 @@ export function makeResponseTimesHistogram(responseTimes: number[]): Record<stri
 }
 
 export abstract class CompareFormatter {
-  totalQueries: number;
-
-  abstract logChange(change: Change): void;
-
-  abstract queryRan(): void;
-
-  abstract finished(stats: FinishedStats): void;
-
   oldApiEnv: ApiEnv;
 
   newApiEnv: ApiEnv;
@@ -56,10 +49,53 @@ export abstract class CompareFormatter {
 
   outputStream: fs.WriteStream;
 
+  showUnchanged: boolean;
+
+  totalQueries = 0;
+
+  numQueriesRun = 0;
+
+  /** Called when a query has actually changed */
+  abstract logChange(change: Change): void;
+
+  /**
+   * Called each time a query completes. Change is a bit of a misnomer.
+   * This is called even when there was no diff. It's up to the formatter
+   * what to do with it. Calls the formatter implementation of logChange if
+   * there is a diff or --unchanged is specified
+   *
+   * @param {Change} change query + old response + new response to log
+   */
+  queryCompleted(change: Change): void {
+    this.numQueriesRun += 1;
+    if (this.numQueriesRun % 10 === 0) {
+      console.error(`IN PROGRESS. ${this.numQueriesRun}/${this.totalQueries} run`);
+    }
+
+    if (!change.delta && this.showUnchanged) {
+      return;
+    }
+
+    this.logChange(change);
+  }
+
+  /** Called when all queries are finished running */
+  abstract finished(stats: FinishedStats): void;
+
+  /**
+   * Helper to deal with output redirection
+   *
+   * @param {string} s string to output with newline
+   */
   writeln(s: string): void {
     this.write(`${s}\n`);
   }
 
+  /**
+   * Helper to deal with output redirection
+   *
+   * @param {string} s string to output
+   */
   write(s: string): void {
     if (this.outputStream) {
       this.outputStream.write(s);
@@ -75,6 +111,7 @@ export abstract class CompareFormatter {
     this.newApiEnv = newApiEnv;
     this.totalQueries = totalQueries;
     this.startDate = new Date();
+    this.showUnchanged = argv.unchanged;
 
     const outputFilename = argv.output_file;
     if (outputFilename && outputFilename !== '-') {
diff --git a/lib/compare/formatters/json-formatter.ts b/lib/compare/formatters/json-formatter.ts
@@ -25,7 +25,15 @@ export default class JsonFormatter extends CompareFormatter {
 
   changes: JsonChange[] = [];
 
-  logChange(change: Change): void {
+  queryCompleted(change: Change): void {
+    this.numQueriesRun += 1;
+    if (this.numQueriesRun % 10 === 0) {
+      console.error(`IN PROGRESS. ${this.numQueriesRun}/${this.totalQueries} run`);
+    }
+
+    if (!change.delta && this.showUnchanged) {
+      return;
+    }
     this.numQueriesChanged += 1;
 
     this.changes.push({
@@ -47,13 +55,6 @@ export default class JsonFormatter extends CompareFormatter {
     });
   }
 
-  queryRan(): void {
-    this.numQueriesRun += 1;
-    if (this.numQueriesRun % 10 === 0) {
-      console.error(`IN PROGRESS. ${this.numQueriesRun}/${this.totalQueries} run`);
-    }
-  }
-
   /**
    * Output the overall json object. Broken out here so html formatter can call it
    *
diff --git a/lib/compare/generate-baseline.ts b/lib/compare/generate-baseline.ts
@@ -1,5 +1,14 @@
+/**
+ * Generate baseline json to be used in future compare runs.
+ *
+ * This file is mostly a copy of compare.ts, which I don't love.
+ * compare.ts is designed to work by comparing two servers, this
+ * script only runs against one server and saves the output to a json file.
+ *
+ * Adding more clauses to compare.ts seemed ugly, but I will probably
+ * attempt it at some point
+ */
 /* eslint-disable camelcase */
-import * as queryString from 'query-string';
 import * as Bluebird from 'bluebird';
 
 import { ApiEnv, argvToApiEnv } from '../apiEnv';
@@ -29,19 +38,10 @@ async function runOneQuery({
   query: Query;
   argv: ParsedArgs;
 }): Promise<Change> {
-  const extraParams = queryString.parse(argv.extra_params.join('&')) as Record<string, string>;
-  delete extraParams.undefined;
-  const params = { ...query.params, ...extraParams };
-  const queryWithExtraParams = {
-    endpoint: query.endpoint,
-    method: query.method,
-    params,
-  };
-
-  const oldResponse = await runQuery(oldApiEnv, queryWithExtraParams, argv.timeout);
+  const oldResponse = await runQuery(oldApiEnv, query, argv.timeout);
 
   return {
-    query: queryWithExtraParams,
+    query,
     delta: undefined,
     oldResponse,
     newResponse: undefined,
@@ -68,6 +68,7 @@ async function runQueries({
   formatter: CompareFormatter
 }) {
   const oldResponseTimes: number[] = [];
+  const oldStatusCodes: Record<number, number> = {};
 
   await Bluebird.map(
     queries,
@@ -77,13 +78,23 @@ async function runQueries({
         query,
         argv,
       });
-      formatter.queryRan();
-      formatter.logChange(change);
+      formatter.queryCompleted(change);
       oldResponseTimes.push((change.oldResponse as any).duration);
+
+      if (!oldStatusCodes[change.oldResponse.status]) {
+        oldStatusCodes[change.oldResponse.status] = 0;
+      }
+
+      oldStatusCodes[change.oldResponse.status] += 1;
     },
     { concurrency: argv.concurrency },
   );
-  formatter.finished({ oldResponseTimes, newResponseTimes: undefined });
+  formatter.finished({
+    old: {
+      responseTimes: oldResponseTimes,
+      statusCodes: oldStatusCodes,
+    },
+  });
 }
 
 const argv = parseArgv([OLD_KEY]) as ParsedArgs;
diff --git a/lib/compare/query-reader.ts b/lib/compare/query-reader.ts
diff --git a/lib/run-query.ts b/lib/run-query.ts
