Added directory/filename support to JUnit

intern.json

@@ -31,7 +31,9 @@
       "_tests/tests/unit/bin/intern.js",
       "_tests/tests/unit/lib/executors/Node.js",
       "_tests/tests/unit/lib/node/**/*.js",
-      "_tests/tests/unit/tasks/**/*.js"
+      "_tests/tests/unit/tasks/**/*.js",
+      "_tests/tests/unit/lib/reporters/util/*.js",
+      "_tests/tests/unit/lib/reporters/JUnit.js"
     ],
     "plugins": [
       "_tests/tests/support/nodeMocking.js",

src/lib/reporters/Coverage.ts

@@ -10,6 +10,19 @@ import Node, { NodeEvents } from '../executors/Node';
 
 export { ReportType };
 
+export interface CoverageProperties extends ReporterProperties {
+  /** A filename provided to the coverage reporter */
+  filename?: string;
+
+  /** A directory provided to the coverage reporter */
+  directory?: string;
+
+  /** Watermarks used to check coverage */
+  watermarks?: Watermarks;
+}
+
+export type CoverageOptions = Partial<CoverageProperties>;
+
 const eventHandler = createEventHandler<NodeEvents>();
 
 export default abstract class Coverage extends Reporter
@@ -35,6 +48,10 @@ export default abstract class Coverage extends Reporter
     }
   }
 
+  /**
+   * This is a bag of data provided to the selected istanbul reporter (defined by `reportType`).
+   * by default this provides a filename (if present), though not all reporters use a filename.
+   */
   getReporterOptions(): { [key: string]: any } {
     return {
       file: this.filename
@@ -52,6 +69,7 @@ export default abstract class Coverage extends Reporter
 
     const transformed = this.executor.sourceMapStore.transformCoverage(map);
 
+    // context is a bag of data used by the report. Values will not necessarily be used (it depends on the specific reporter)
     const context = createContext({
       dir: this.directory,
       sourceFinder: transformed.sourceFinder,
@@ -71,19 +89,6 @@ export default abstract class Coverage extends Reporter
   }
 }
 
-export interface CoverageProperties extends ReporterProperties {
-  /** A filename to write coverage data to */
-  filename?: string;
-
-  /** A direcotry to write coverage data to */
-  directory?: string;
-
-  /** Watermarks used to check coverage */
-  watermarks?: Watermarks;
-}
-
-export type CoverageOptions = Partial<CoverageProperties>;
-
 function isCoverageMap(value: any): value is CoverageMap {
   return value != null && typeof value.files === 'function';
 }

src/lib/reporters/JUnit.ts

@@ -1,11 +1,17 @@
 import { createWriteStream } from 'fs';
 import { dirname } from 'path';
 
+import { Executor } from '../executors/Executor';
+import { mkdirp } from '../node/util';
 import Suite, { isSuite } from '../Suite';
 import Test from '../Test';
 import Reporter, { eventHandler, ReporterProperties } from './Reporter';
-import { Executor } from '../executors/Executor';
-import { mkdirp } from '../node/util';
+import { getPath } from './util/getPath';
+
+export interface JUnitProperties extends ReporterProperties {
+  directory: string;
+  filename?: string;
+}
 
 /**
  * There is no formal spec for this format and everyone does it differently, so
@@ -17,8 +23,9 @@ export default class JUnit extends Reporter {
 
   constructor(executor: Executor, options: Partial<JUnitProperties> = {}) {
     super(executor, options);
-    if (options.filename) {
-      this.filename = options.filename;
+    const path = getPath(options.directory, options.filename, 'junit.xml');
+    if (path) {
+      this.filename = path;
       if (dirname(this.filename) !== '.') {
         mkdirp(dirname(this.filename));
       }
@@ -38,10 +45,6 @@ export default class JUnit extends Reporter {
   }
 }
 
-export interface JUnitProperties extends ReporterProperties {
-  filename?: string;
-}
-
 /**
  * Simple XML generator.
  * @constructor

src/lib/reporters/util/getPath.ts

@@ -0,0 +1,37 @@
+import { join } from 'path';
+
+/**
+ * This method helps to normalize the rules surrounding LEGACY reporters.
+ *
+ * DO NOT use this method when writing new Reporters. New reporters should ALWAYS require a directory
+ * and if needed an optional filename. If the filename is not provided, but necessary, then a default
+ * should be assigned by the reporter.
+ *
+ * getPath() normalizes legacy Reporters following these rules:
+ *
+ * 1. If a directory exists:
+ * 1a. and a filename exists; return the joined directory and filename
+ * 1b. and a default filename exists; return the joined directory and default filename
+ * 1c. return just the directory
+ * 2. If a filename exists; return just the filename as a full path
+ * 3. If no conditions are met, return undefined
+ *
+ * The defaultFilename exists as a separate parameter so it is only used when a directory exists.
+ */
+export function getPath(
+  directory?: string,
+  filename?: string,
+  defaultFilename?: string
+) {
+  if (directory) {
+    if (filename) {
+      return join(directory, filename);
+    } else if (defaultFilename) {
+      return join(directory, defaultFilename);
+    }
+
+    return directory;
+  } else if (filename) {
+    return filename;
+  }
+}

tests/tsconfig.json

@@ -40,6 +40,7 @@
     "./unit/lib/reporters/Simple.ts",
     "./unit/lib/reporters/TeamCity.ts",
     "./unit/lib/reporters/TextCoverage.ts",
+    "./unit/lib/reporters/util/getPath.ts",
     "./unit/lib/*.ts",
     "./unit/loaders/*.ts",
     "./unit/tasks/*.ts",

tests/unit/lib/reporters/JUnit.ts

@@ -1,6 +1,5 @@
 import { spy, stub } from 'sinon';
 
-import _JUnit from 'src/lib/reporters/JUnit';
 import Test from 'src/lib/Test';
 import Suite from 'src/lib/Suite';
 
@@ -16,12 +15,10 @@ registerSuite('lib/reporters/JUnit', function() {
     formatError: spy((error: Error) => error.message)
   };
 
-  const mockFs = {
-    createWriteStream: spy(),
-    mkdirSync: spy()
-  };
+  const createWriteStreamMock = stub();
+  const mkdirpMock = stub();
 
-  let JUnit: typeof _JUnit;
+  let JUnit: typeof import('src/lib/reporters/JUnit').default;
   let removeMocks: () => void;
 
   const getReportOutput = () => {
@@ -44,7 +41,12 @@ registerSuite('lib/reporters/JUnit', function() {
   return {
     before() {
       return mockRequire(require, 'src/lib/reporters/JUnit', {
-        fs: mockFs
+        'src/lib/node/util': {
+          mkdirp: mkdirpMock
+        },
+        fs: {
+          createWriteStream: createWriteStreamMock
+        }
       }).then(handle => {
         removeMocks = handle.remove;
         JUnit = handle.module.default;
@@ -58,24 +60,16 @@ registerSuite('lib/reporters/JUnit', function() {
     beforeEach() {
       mockExecutor.suites = [];
       mockExecutor.on.reset();
-      mockFs.createWriteStream.resetHistory();
-      mockFs.mkdirSync.resetHistory();
+      createWriteStreamMock.reset();
+      mkdirpMock.reset();
     },
 
     tests: {
       construct() {
-        let callCount = 0;
-        (mockFs as any).existsSync = (dir: string) => {
-          if (dir === 'somewhere' && callCount === 0) {
-            callCount++;
-            return false;
-          }
-          return true;
-        };
         new JUnit(mockExecutor, { filename: 'somewhere/foo.js' });
-        assert.equal(mockFs.mkdirSync.callCount, 1);
-        assert.equal(mockFs.mkdirSync.getCall(0).args[0], 'somewhere');
-        assert.equal(mockFs.createWriteStream.callCount, 1);
+        assert.equal(mkdirpMock.callCount, 1);
+        assert.equal(mkdirpMock.getCall(0).args[0], 'somewhere');
+        assert.equal(createWriteStreamMock.callCount, 1);
       },
 
       '#runEnd': {

tests/unit/lib/reporters/util/getPath.ts

@@ -0,0 +1,35 @@
+import { getPath } from 'src/lib/reporters/util/getPath';
+import { join } from 'path';
+
+const DIR = 'directory';
+const FILE = 'file.name';
+
+registerSuite('lib/reporters/util/getPath', {
+  tests: {
+    'getPath()': {
+      'no parameters; returns undefined'() {
+        assert.isUndefined(getPath());
+      },
+
+      'directory only; returns directory'() {
+        assert.strictEqual(getPath(DIR), DIR);
+      },
+
+      'directory and filename; returns joined path'() {
+        assert.strictEqual(getPath(DIR, FILE), join(DIR, FILE));
+      },
+
+      'directory and defaultFilename; returns joined path'() {
+        assert.strictEqual(getPath(DIR, undefined, FILE), join(DIR, FILE));
+      },
+
+      'filename only; returns filename'() {
+        assert.strictEqual(getPath(undefined, FILE), FILE);
+      },
+
+      'defaultFilename only; returns undefined'() {
+        assert.isUndefined(getPath(undefined, undefined, FILE));
+      }
+    }
+  }
+});