Add sugar to make tests suite-er (#9)

* Add some sugar for simpler, easier to read test files * Cleanup * Cleanup import type * Fix ANSI codes making it into JSON * Improve test coverage of console reporter * Handle ansi output for junit too
LastCallMedia · Mar 17, 2020 · 2ed9bb1 · 2ed9bb1
1 parent b06dce3
commit 2ed9bb1
Show file tree

Hide file tree

Showing 19 changed files with 505 additions and 298 deletions.
diff --git a/package.json b/package.json
@@ -7,6 +7,7 @@
     "chalk": "^2.0.0",
     "debug": "^3.0.0",
     "indent-string": "^4.0.0",
+    "strip-ansi": "^6.0.0",
     "wrap-ansi": "^6.0.0",
     "xml": "^1.0.0",
     "yargs": "^15.0.0"
@@ -37,7 +38,7 @@
     "prettier": "^1.19.1",
     "tmp-promise": "^2.0.2",
     "ts-jest": "^25.2.1",
-    "typescript": "^3.7.5"
+    "typescript": "^3.8.3"
   },
   "scripts": {
     "prettier": "prettier --single-quote --write './src/**/*.ts'",

diff --git a/src/crawler.ts → src/Crawler.ts b/src/crawler.ts → src/Crawler.ts
@@ -1,15 +1,11 @@
 import debug from 'debug';
 import NativeDriver from './driver/native';
-import { toAsyncIterable } from './cli/util';
+import { isCrawlerRequest, toAsyncIterable } from './util';
 
 const log = debug('nightcrawler:info');
 const error = debug('nightcrawler:error');
 
-import { Driver, CrawlerRequest, CrawlerUnit } from './types';
-
-type RequestIterable<T extends CrawlerRequest = CrawlerRequest> =
-  | Iterable<T>
-  | AsyncIterable<T>;
+import { RequestIterable, Driver, CrawlerRequest, CrawlerUnit } from './types';
 
 export default class Crawler {
   driver: Driver;
@@ -39,6 +35,11 @@ export default class Crawler {
     const queueOne = async (): Promise<void> => {
       const next = await iterator.next();
       if (next.value) {
+        if (!isCrawlerRequest(next.value)) {
+          throw new Error(
+            `This item does not look like a crawler request: ${next.value.toString()}`
+          );
+        }
         const prom = this._fetch(next.value)
           .then(collectToBuffer, collectToBuffer)
           .finally(() => pool.delete(prom));
@@ -62,11 +63,6 @@ export default class Crawler {
         yield buffer.pop() as CrawlerUnit;
       }
     }
-
-    // Yield any leftover buffered results.
-    while (buffer.length > 0) {
-      yield buffer.pop() as CrawlerUnit;
-    }
   }
 
   /**

diff --git a/src/__tests__/crawler.ts b/src/__tests__/crawler.ts
@@ -1,4 +1,4 @@
-import Crawler from '../crawler';
+import Crawler from '../Crawler';
 import DummyDriver from '../driver/dummy';
 import { performance } from 'perf_hooks';
 import { CrawlerRequest } from '../types';
@@ -37,6 +37,11 @@ describe('Crawler', () => {
     const c = new Crawler(requests, new DummyDriver());
     await expect(all(c.crawl())).resolves.toHaveLength(1);
   });
+  it('Should throw a meaningful error when an invalid iterator is given', function() {
+    expect(() => {
+      new Crawler((1 as unknown) as AsyncIterable<CrawlerRequest>);
+    }).toThrow('Unable to create an async iterator from the request iterable.');
+  });
 
   it('Should respect concurrency in crawling', async () => {
     /**
@@ -105,4 +110,12 @@ describe('Crawler', () => {
       }
     ]);
   });
+
+  it('Should fail if any item in the iterator is not in the shape of a request', async function() {
+    const items = ['foo'];
+    const crawler = new Crawler(items as Iterable<CrawlerRequest>);
+    await expect(all(crawler.crawl(1))).rejects.toThrow(
+      'This item does not look like a crawler request: foo'
+    );
+  });
 });
diff --git a/src/cli/commands/__tests__/crawl.ts b/src/cli/commands/__tests__/crawl.ts
@@ -1,20 +1,19 @@
 import { CrawlCommandArgs, handler } from '../crawl';
 import stream from 'stream';
-import Crawler from '../../../crawler';
-import DummyDriver from '../../../driver/dummy';
 import { FailedAnalysisError } from '../../errors';
 import yargs from 'yargs';
 
 import * as crawlModule from '../crawl';
 import TestContext from '../../../testing/TestContext';
-import { CrawlerRequest } from '../../../types';
 
 import ConsoleReporter from '../../formatters/ConsoleReporter';
 import JUnitReporter from '../../formatters/JUnitReporter';
 import JSONReporter from '../../formatters/JSONReporter';
 
-import { mocked } from 'ts-jest';
+import { mocked } from 'ts-jest/utils';
+import { makeResult } from '../../util';
 
+jest.mock('../../../testing/TestContext');
 jest.mock('../../formatters/JUnitReporter');
 jest.mock('../../formatters/ConsoleReporter');
 jest.mock('../../formatters/JSONReporter');
@@ -96,112 +95,114 @@ describe('Crawl Handler', function() {
 
   it('Executes the crawl', async function() {
     // eslint-disable-next-line
-    const crawl = jest.fn(async function*() {});
-    const crawler = new Crawler([]);
-    const tests = new TestContext();
-    crawler.crawl = crawl;
-    await handler({ stdout, crawler, tests, concurrency: 1 });
-    expect(crawl).toHaveBeenCalledWith(1);
+    const context = new TestContext('');
+    context.crawl = jest.fn(async function*(): ReturnType<
+      TestContext['crawl']
+    > {
+      // No-op.
+    });
+    const mockedContext = mocked(context);
+    await handler({ stdout, context, concurrency: 1 });
+    expect(mockedContext.crawl).toHaveBeenCalledWith(1);
   });
 
   it('Displays console output.', async function() {
-    const crawler = new Crawler(
-      [{ url: 'http://example.com' }],
-      new DummyDriver()
-    );
-    const tests = new TestContext();
-    tests.each('Testing', () => {
-      throw new Error('Test');
-    });
-    tests.all('Testing', () => {
-      throw new Error('Test');
-    });
+    const context = new TestContext('');
+    context.crawl = async function*(): ReturnType<TestContext['crawl']> {
+      yield [
+        'http://example.com',
+        makeResult({ Testing: { pass: false, message: 'Test' } })
+      ];
+    };
     try {
-      await handler({ stdout, crawler, tests });
+      await handler({ stdout, context });
     } catch (e) {
       // no-op - we don't care.
     }
     expect(MockedConsoleReporter).toHaveBeenCalled();
     const reporter = MockedConsoleReporter.mock.instances[0];
     expect(reporter.start).toHaveBeenCalledTimes(1);
-    expect(reporter.report).toHaveBeenCalledTimes(2);
+    expect(reporter.report).toHaveBeenCalledTimes(1);
     expect(reporter.stop).toHaveBeenCalledTimes(1);
   });
 
+  it('Does not display console output if passed --silent', async function() {
+    const context = new TestContext('');
+    context.crawl = async function*(): ReturnType<TestContext['crawl']> {
+      yield [
+        'http://example.com',
+        makeResult({ Testing: { pass: false, message: 'Test' } })
+      ];
+    };
+    try {
+      await handler({ stdout, context, silent: true });
+    } catch (e) {
+      // no-op - we don't care.
+    }
+    expect(MockedConsoleReporter).not.toHaveBeenCalled();
+  });
+
   it('Writes a JUnit report', async function() {
-    const crawler = new Crawler(
-      [{ url: 'http://example.com' }],
-      new DummyDriver()
-    );
-    const tests = new TestContext();
-    tests.each('Testing', () => {
-      throw new Error('Test');
-    });
-    tests.all('Testing', () => {
-      throw new Error('Test');
-    });
+    const context = new TestContext('');
+    context.crawl = async function*(): ReturnType<TestContext['crawl']> {
+      yield [
+        'http://example.com',
+        makeResult({ Testing: { pass: false, message: 'Test' } })
+      ];
+    };
     try {
-      await handler({ stdout, crawler, tests, junit: 'test' });
+      await handler({ stdout, context, junit: 'test.xml' });
     } catch (e) {
       // no-op - we don't care.
     }
-    expect(MockedJUnitReporter).toHaveBeenCalled();
+    expect(MockedJUnitReporter).toHaveBeenCalledWith('test.xml');
     const reporter = MockedJUnitReporter.mock.instances[0];
     expect(reporter.start).toHaveBeenCalledTimes(1);
-    expect(reporter.report).toHaveBeenCalledTimes(2);
+    expect(reporter.report).toHaveBeenCalledTimes(1);
     expect(reporter.stop).toHaveBeenCalledTimes(1);
   });
 
   it('Writes a JSON report', async function() {
-    const crawler = new Crawler(
-      [{ url: 'http://example.com' }],
-      new DummyDriver()
-    );
-    const tests = new TestContext();
-    tests.each('Testing', () => {
-      throw new Error('Test');
-    });
-    tests.all('Testing', () => {
-      throw new Error('Test');
-    });
+    const context = new TestContext('');
+    context.crawl = async function*(): ReturnType<TestContext['crawl']> {
+      yield [
+        'http://example.com',
+        makeResult({ Testing: { pass: false, message: 'Test' } })
+      ];
+    };
     try {
-      await handler({ stdout, crawler, tests, json: 'test' });
+      await handler({ stdout, context, json: 'test.json' });
     } catch (e) {
       // no-op - we don't care.
     }
-    expect(MockedJSONReporter).toHaveBeenCalled();
+    expect(MockedJSONReporter).toHaveBeenCalledWith('test.json');
     const reporter = MockedJSONReporter.mock.instances[0];
     expect(reporter.start).toHaveBeenCalledTimes(1);
-    expect(reporter.report).toHaveBeenCalledTimes(2);
+    expect(reporter.report).toHaveBeenCalledTimes(1);
     expect(reporter.stop).toHaveBeenCalledTimes(1);
   });
 
   it('Throws an error if the analysis contains failures', async function() {
-    const crawler = new Crawler(
-      [{ url: 'https://example.com' }],
-      new DummyDriver()
-    );
-    const tests = new TestContext();
-    tests.each('testing', () => {
-      throw new Error('test');
-    });
-    await expect(handler({ stdout, crawler, tests })).rejects.toBeInstanceOf(
+    const context = new TestContext('');
+    context.crawl = async function*(): ReturnType<TestContext['crawl']> {
+      yield [
+        'http://example.com',
+        makeResult({ Testing: { pass: false, message: 'Test' } })
+      ];
+    };
+    await expect(handler({ stdout, context })).rejects.toBeInstanceOf(
       FailedAnalysisError
     );
   });
 
   it('Should stop the crawl if setup fails', async function() {
-    const crawler = new Crawler(
-      // eslint-disable-next-line require-yield
-      (async function*(): AsyncIterable<CrawlerRequest> {
-        throw new Error('Oh no!');
-      })(),
-      new DummyDriver()
-    );
+    const context = new TestContext('');
+    context.crawl = (): ReturnType<TestContext['crawl']> => {
+      throw new Error('Oh no!');
+    };
     const p = handler({
       stdout,
-      crawler,
-      tests: new TestContext()
+      context
     });
     return expect(p).rejects.toThrow('Oh no!');
   });

diff --git a/src/cli/commands/crawl.ts b/src/cli/commands/crawl.ts
@@ -62,30 +62,20 @@ function getReporters(argv: CrawlCommandArgs): Reporter[] {
   return reporters;
 }
 export const handler = async function(argv: CrawlCommandArgs): Promise<void> {
-  const { crawler, tests, concurrency = 3 } = argv;
+  const { context, concurrency = 3 } = argv;
+  let hasAnyFailure = false;
 
   const reporters: Reporter[] = getReporters(argv);
   await Promise.all(reporters.map(reporter => reporter.start()));
 
-  let hasError = false;
-  const allUnits = [];
-  for await (const unit of crawler.crawl(concurrency)) {
-    const result = tests.testUnit(unit);
-    reporters.forEach(r => r.report(unit.request.url, result));
-    hasError = hasError || hasFailure(result);
-    allUnits.push(unit);
+  for await (const [url, result] of context.crawl(concurrency)) {
+    reporters.forEach(r => r.report(url, result));
+    hasAnyFailure = hasAnyFailure || hasFailure(result);
   }
-  const allResults = tests.testUnits(allUnits);
-  await Promise.all(
-    reporters.map(reporter => {
-      if (allResults.size > 0) {
-        reporter.report('All Requests', allResults);
-      }
-      return reporter.stop();
-    })
-  );
 
-  if (hasFailure(allResults) || hasError) {
+  await Promise.all(reporters.map(r => r.stop()));
+
+  if (hasAnyFailure) {
     throw new FailedAnalysisError('Testing reported an error.');
   }
 };
diff --git a/src/cli/formatters/JSONReporter.ts b/src/cli/formatters/JSONReporter.ts
@@ -2,13 +2,24 @@ import Reporter from './Reporter';
 import { TestResult, TestResultMap } from '../../testing/TestContext';
 import { writeFile } from 'fs';
 import { promisify } from 'util';
+import strip from 'strip-ansi';
 
 const writeFileP = promisify(writeFile);
 
-function mapToObj<T = unknown>(map: Map<string, T>): { [k: string]: T } {
+function stripResult(object: {
+  [k: string]: string | boolean;
+}): { [k: string]: string | boolean } {
+  const ret = Object.create(null);
+  for (const [k, v] of Object.entries(object)) {
+    ret[k] = typeof v === 'string' ? strip(v) : v;
+  }
+  return ret;
+}
+
+function mapToObj(map: Map<string, TestResult>): { [k: string]: TestResult } {
   const obj = Object.create(null);
   for (const [k, v] of map) {
-    obj[k] = v;
+    obj[strip(k)] = stripResult(v);
   }
   return obj;
 }
@@ -24,7 +35,7 @@ export default class JSONReporter implements Reporter {
     // no-op.
   }
   report(url: string, result: TestResultMap): void {
-    this.collected.push({ url, result: mapToObj<TestResult>(result) });
+    this.collected.push({ url, result: mapToObj(result) });
   }
   async stop(): Promise<void> {
     await writeFileP(this.path, JSON.stringify(this.collected, null, 4));

diff --git a/src/cli/formatters/JUnitReporter.ts b/src/cli/formatters/JUnitReporter.ts
@@ -3,6 +3,7 @@ import { TestResultMap } from '../../testing/TestContext';
 import Reporter from './Reporter';
 import { writeFile } from 'fs';
 import { promisify } from 'util';
+import strip from 'strip-ansi';
 
 const writeFileP = promisify(writeFile);
 
@@ -99,7 +100,7 @@ function formatResultSet(name: string, results: TestResultMap): XMLElement {
     const thisCase = new TestCase(url);
     if (!result.pass) {
       thisCase.failure = true;
-      thisCase.output = result.message;
+      thisCase.output = strip(result.message);
     }
     suite.addCase(thisCase);
   });