Repository: mochajs/mocha

Problem statement:
Add output option to JSON reporter
**Describe the solution you'd like**
I'd like to use test report for other applications (e.g. Slack)
So, JSON file is useful.

`package.json`:

```json
{
  "scripts": {
    "test": "mocha ./test/*-test.js --reporter json --reporter-options output=report.json",
    "posttest": "node slack.js"
  }
}
```

`slack.js`:

```js
const readJSONFile = require('./readJSONFile')
const createMessage = require('./createMessage')
const postMessage = require('./postMessage')

const report = await readJSONFile('report.json')
const message = createMessage(report)
await postMessage(message)
```

**Describe alternatives you've considered**

- https://github.com/i3oges/mocha-json-file-reporter



Hints:
@munierujp In your shell you can pipe the report output to a file. You don't need a `reporter-option` for this. In Windows it is the ">" sign.
The standard output may occasionally contain noise.
And, xunit reporter supports `output` option already.

> By default, it will output to the console. To write directly to a file, use --reporter-options output=filename.xml.

https://mochajs.org/#xunit

Is there a difference between these reporters?
any update here? if you need another use case to add momentum to this ticket: i'm trying to add `mocha-multi-reporters` to a project so that our custom `stdout` reporter still shows up on our bitbucket pipeline, but also have a json output blob so we can do more programmatically interesting things with the results outside of the execution of the tests themselves.

e: ~~@munierujp [json](https://github.com/mochajs/mocha/blob/master/lib/reporters/json.js) and [xunit](https://github.com/mochajs/mocha/blob/master/lib/reporters/xunit.js) reporters are actually implemented rather differently. i'm currently taking a stab at introducing rudimentary output file specification similar to how it's done in the `xunit` reporter.~~

e2: @munierujp's solution below does what i need. thanks!
I've created a third-party reporter for outputting test result as JSON file.

- [@mochajs/json-file-reporter](https://www.npmjs.com/package/@mochajs/json-file-reporter)

Having both human readable console output  and json or xunit file output at same time would be very useful.
It's quite ridiculous you need one 3rd party package to support multiple reporters and another 3rd party package to write JSON report to file. Redirecting whole output to file wouldn't work in this case.

I run into issue because [json-file-reporter](https://github.com/mocha-community/json-file-reporter) doesn't include `file` property like built in `json` reporter. This is exactly kind of issue that would not happen if built in `json` reporter had `output` option.
@dorny yes, you should definitively terminate this ridiculous situation.
What about a PR from yourside? I will be back, to check on your reply.
@juergba sure I will do my best :)

And don't get me wrong. My intension was not to be offensive. I was simply pointing out current situation is not ideal for the user and your suggestion to pipe the output does not cover all use cases.

By the way, I actually doesn't need this for myself.
I'm working on GitHub action [test-reporter](https://github.com/dorny/test-reporter).
I've originally implemented support for JEST which I needed. But thought it could be useful for others to support mocha too as it's the second most used testing framework.

Meanwhile I've also discovered another option: [mochawesome](https://www.npmjs.com/package/mochawesome) reporter.
It produces JSON and HTML output while you can still specify another reporter for console output. Unfortunately it uses it's own format of JSON output so I would still prefer solution using [mocha-multi-reporters](https://www.npmjs.com/package/mocha-multi-reporters) and mocha built-in JSON reporter.

@juergba 
I took a first look at the code. Changes to `json.js` reporter are trivial.  Basically it would follow the same pattern as is already implemented in `xunit` reporter.

However I'm not so sure about unit test.
The [json.spec.js](https://github.com/mochajs/mocha/blob/v8.3.2/test/reporters/json.spec.js) looks it would benefit from some refactoring. Existing tests are not actually testing the stdout of reporter, instead they look at `testResults` property set on `runner`. This property is set [here](https://github.com/mochajs/mocha/blob/v8.3.2/lib/reporters/json.js#L67).
Is `runner.testResults` actually part of API or it's some old hack just for testing purposes? It doesn't look it's used anywhere else.

All other reporter tests are using different pattern - example [here](https://github.com/mochajs/mocha/blob/v8.3.2/test/reporters/spec.spec.js#L45).

My idea would be to remove `testResults` property and rework `spec.json.js` so stdout is captured, parsed as JSON and then checked if expectations are met.
Another test then would check if the stdout is empty when `output` option is provided and if correct content would be created on file system (mocked of course).

So should I remove setting of the `testResults` property in `json` reporter and refactor the tests as I proposed?
As I'm completely new to mocha codebase I would appreciate guidance. Thanks.
@dorny thank you for taking over.

> [...] Is `runner.testResults` actually part of API [...]?

I blamed _lib/reporters/json.js_ and found #1294, which states:
`It also saves the json result back into the runner object to allow a programmatically mocha run to query the output.`

So I propose to leave this property as is, but refactor the tests as per your suggestion. Following the `xunit` implementation seems  a good approach to me.

I don't know this part of our codebase well either, and mocks aren't really my friends. So you shouldn't trust my comments completely.

Patch:
diff --git a/example/config/.mocharc.js b/example/config/.mocharc.js
--- a/example/config/.mocharc.js
+++ b/example/config/.mocharc.js
@@ -33,7 +33,7 @@ module.exports = {
   parallel: false,
   recursive: false,
   reporter: 'spec',
-  'reporter-option': ['foo=bar', 'baz=quux'],
+  'reporter-option': ['foo=bar', 'baz=quux'], // array, not object
   require: '@babel/register',
   retries: 1,
   slow: '75',
diff --git a/lib/reporters/base.js b/lib/reporters/base.js
--- a/lib/reporters/base.js
+++ b/lib/reporters/base.js
@@ -90,7 +90,7 @@ exports.colors = {
 
 exports.symbols = {
   ok: symbols.success,
-  err: symbols.err,
+  err: symbols.error,
   dot: '.',
   comma: ',',
   bang: '!'
diff --git a/lib/reporters/json.js b/lib/reporters/json.js
--- a/lib/reporters/json.js
+++ b/lib/reporters/json.js
@@ -7,12 +7,16 @@
  */
 
 var Base = require('./base');
+var fs = require('fs');
+var path = require('path');
+const createUnsupportedError = require('../errors').createUnsupportedError;
+const utils = require('../utils');
 var constants = require('../runner').constants;
 var EVENT_TEST_PASS = constants.EVENT_TEST_PASS;
+var EVENT_TEST_PENDING = constants.EVENT_TEST_PENDING;
 var EVENT_TEST_FAIL = constants.EVENT_TEST_FAIL;
 var EVENT_TEST_END = constants.EVENT_TEST_END;
 var EVENT_RUN_END = constants.EVENT_RUN_END;
-var EVENT_TEST_PENDING = constants.EVENT_TEST_PENDING;
 
 /**
  * Expose `JSON`.
@@ -30,7 +34,7 @@ exports = module.exports = JSONReporter;
  * @param {Runner} runner - Instance triggers reporter actions.
  * @param {Object} [options] - runner options
  */
-function JSONReporter(runner, options) {
+function JSONReporter(runner, options = {}) {
   Base.call(this, runner, options);
 
   var self = this;
@@ -38,6 +42,14 @@ function JSONReporter(runner, options) {
   var pending = [];
   var failures = [];
   var passes = [];
+  var output;
+
+  if (options.reporterOption && options.reporterOption.output) {
+    if (utils.isBrowser()) {
+      throw createUnsupportedError('file output not supported in browser');
+    }
+    output = options.reporterOption.output;
+  }
 
   runner.on(EVENT_TEST_END, function(test) {
     tests.push(test);
@@ -66,7 +78,20 @@ function JSONReporter(runner, options) {
 
     runner.testResults = obj;
 
-    process.stdout.write(JSON.stringify(obj, null, 2));
+    var json = JSON.stringify(obj, null, 2);
+    if (output) {
+      try {
+        fs.mkdirSync(path.dirname(output), {recursive: true});
+        fs.writeFileSync(output, json);
+      } catch (err) {
+        console.error(
+          `${Base.symbols.err} [mocha] writing output to "${output}" failed: ${err.message}\n`
+        );
+        process.stdout.write(json);
+      }
+    } else {
+      process.stdout.write(json);
+    }
   });
 }
 


Test patch:
diff --git a/test/reporters/json.spec.js b/test/reporters/json.spec.js
--- a/test/reporters/json.spec.js
+++ b/test/reporters/json.spec.js
@@ -1,12 +1,16 @@
 'use strict';
 
+var fs = require('fs');
 var sinon = require('sinon');
+var JSONReporter = require('../../lib/reporters/json');
+var utils = require('../../lib/utils');
 var Mocha = require('../../');
 var Suite = Mocha.Suite;
 var Runner = Mocha.Runner;
 var Test = Mocha.Test;
 
 describe('JSON reporter', function() {
+  var mocha;
   var suite;
   var runner;
   var testTitle = 'json test 1';
@@ -14,131 +18,216 @@ describe('JSON reporter', function() {
   var noop = function() {};
 
   beforeEach(function() {
-    var mocha = new Mocha({
+    mocha = new Mocha({
       reporter: 'json'
     });
     suite = new Suite('JSON suite', 'root');
     runner = new Runner(suite);
-    var options = {};
-    /* eslint no-unused-vars: off */
-    var mochaReporter = new mocha._reporter(runner, options);
-  });
-
-  beforeEach(function() {
-    sinon.stub(process.stdout, 'write').callsFake(noop);
   });
 
   afterEach(function() {
     sinon.restore();
   });
 
-  it('should have 1 test failure', function(done) {
-    var error = {message: 'oh shit'};
+  describe('test results', function() {
+    beforeEach(function() {
+      var options = {};
+      /* eslint no-unused-vars: off */
+      var mochaReporter = new mocha._reporter(runner, options);
+    });
+
+    beforeEach(function() {
+      sinon.stub(process.stdout, 'write').callsFake(noop);
+    });
+
+    it('should have 1 test failure', function(done) {
+      var error = {message: 'oh shit'};
 
-    var test = new Test(testTitle, function(done) {
-      done(new Error(error.message));
+      var test = new Test(testTitle, function(done) {
+        done(new Error(error.message));
+      });
+
+      test.file = testFile;
+      suite.addTest(test);
+
+      runner.run(function(failureCount) {
+        sinon.restore();
+        expect(runner, 'to satisfy', {
+          testResults: {
+            failures: [
+              {
+                title: testTitle,
+                file: testFile,
+                err: {
+                  message: error.message
+                }
+              }
+            ]
+          }
+        });
+        expect(failureCount, 'to be', 1);
+        done();
+      });
     });
 
-    test.file = testFile;
-    suite.addTest(test);
-
-    runner.run(function(failureCount) {
-      sinon.restore();
-      expect(runner, 'to satisfy', {
-        testResults: {
-          failures: [
-            {
-              title: testTitle,
-              file: testFile,
-              err: {
-                message: error.message
+    it('should have 1 test pending', function(done) {
+      var test = new Test(testTitle);
+      test.file = testFile;
+      suite.addTest(test);
+
+      runner.run(function(failureCount) {
+        sinon.restore();
+        expect(runner, 'to satisfy', {
+          testResults: {
+            pending: [
+              {
+                title: testTitle,
+                file: testFile
               }
-            }
-          ]
-        }
+            ]
+          }
+        });
+        expect(failureCount, 'to be', 0);
+        done();
       });
-      expect(failureCount, 'to be', 1);
-      done();
     });
-  });
 
-  it('should have 1 test pending', function(done) {
-    var test = new Test(testTitle);
-    test.file = testFile;
-    suite.addTest(test);
-
-    runner.run(function(failureCount) {
-      sinon.restore();
-      expect(runner, 'to satisfy', {
-        testResults: {
-          pending: [
-            {
-              title: testTitle,
-              file: testFile
-            }
-          ]
-        }
+    it('should have 1 test pass', function(done) {
+      const test = new Test(testTitle, () => {});
+
+      test.file = testFile;
+      suite.addTest(test);
+
+      runner.run(function(failureCount) {
+        sinon.restore();
+        expect(runner, 'to satisfy', {
+          testResults: {
+            passes: [
+              {
+                title: testTitle,
+                file: testFile,
+                speed: /(slow|medium|fast)/
+              }
+            ]
+          }
+        });
+        expect(failureCount, 'to be', 0);
+        done();
       });
-      expect(failureCount, 'to be', 0);
-      done();
     });
-  });
 
-  it('should have 1 test pass', function(done) {
-    const test = new Test(testTitle, () => {});
-
-    test.file = testFile;
-    suite.addTest(test);
-
-    runner.run(function(failureCount) {
-      expect(runner, 'to satisfy', {
-        testResults: {
-          passes: [
-            {
-              title: testTitle,
-              file: testFile,
-              speed: /(slow|medium|fast)/
-            }
-          ]
-        }
+    it('should handle circular objects in errors', function(done) {
+      var testTitle = 'json test 1';
+      function CircleError() {
+        this.message = 'oh shit';
+        this.circular = this;
+      }
+      var error = new CircleError();
+
+      var test = new Test(testTitle, function(done) {
+        throw error;
+      });
+
+      test.file = testFile;
+      suite.addTest(test);
+
+      runner.run(function(failureCount) {
+        sinon.restore();
+        expect(runner, 'to satisfy', {
+          testResults: {
+            failures: [
+              {
+                title: testTitle,
+                file: testFile,
+                err: {
+                  message: error.message
+                }
+              }
+            ]
+          }
+        });
+        expect(failureCount, 'to be', 1);
+        done();
       });
-      expect(failureCount, 'to be', 0);
-      done();
     });
   });
 
-  it('should handle circular objects in errors', function(done) {
-    var testTitle = 'json test 1';
-    function CircleError() {
-      this.message = 'oh shit';
-      this.circular = this;
-    }
-    var error = new CircleError();
+  describe('when "reporterOption.output" is provided', function() {
+    var expectedDirName = 'reports';
+    var expectedFileName = 'reports/test-results.json';
+    var options = {
+      reporterOption: {
+        output: expectedFileName
+      }
+    };
+
+    beforeEach(function() {
+      /* eslint no-unused-vars: off */
+      var mochaReporter = new mocha._reporter(runner, options);
+    });
 
-    var test = new Test(testTitle, function(done) {
-      throw error;
+    beforeEach(function() {
+      // Add one test to suite to avoid assertions against empty test results
+      var test = new Test(testTitle, () => {});
+      test.file = testFile;
+      suite.addTest(test);
     });
 
-    test.file = testFile;
-    suite.addTest(test);
-
-    runner.run(function(failureCount) {
-      sinon.restore();
-      expect(runner, 'to satisfy', {
-        testResults: {
-          failures: [
-            {
-              title: testTitle,
-              file: testFile,
-              err: {
-                message: error.message
-              }
-            }
-          ]
-        }
+    it('should write test results to file', function(done) {
+      const fsMkdirSync = sinon.stub(fs, 'mkdirSync');
+      const fsWriteFileSync = sinon.stub(fs, 'writeFileSync');
+
+      fsWriteFileSync.callsFake(function(filename, content) {
+        const expectedJson = JSON.stringify(runner.testResults, null, 2);
+        expect(expectedFileName, 'to be', filename);
+        expect(content, 'to be', expectedJson);
       });
-      expect(failureCount, 'to be', 1);
-      done();
+
+      runner.run(function() {
+        expect(
+          fsMkdirSync.calledWith(expectedDirName, {recursive: true}),
+          'to be true'
+        );
+        expect(fsWriteFileSync.calledOnce, 'to be true');
+        done();
+      });
+    });
+
+    it('should warn and write test results to console', function(done) {
+      const fsMkdirSync = sinon.stub(fs, 'mkdirSync');
+      const fsWriteFileSync = sinon.stub(fs, 'writeFileSync');
+
+      fsWriteFileSync.throws('unable to write file');
+
+      const outLog = [];
+      const fake = chunk => outLog.push(chunk);
+      sinon.stub(process.stderr, 'write').callsFake(fake);
+      sinon.stub(process.stdout, 'write').callsFake(fake);
+
+      runner.run(function() {
+        sinon.restore();
+        expect(
+          fsMkdirSync.calledWith(expectedDirName, {recursive: true}),
+          'to be true'
+        );
+        expect(fsWriteFileSync.calledOnce, 'to be true');
+        expect(
+          outLog[0],
+          'to contain',
+          `[mocha] writing output to "${expectedFileName}" failed:`
+        );
+        expect(outLog[1], 'to match', /"fullTitle": "JSON suite json test 1"/);
+        done();
+      });
+    });
+
+    it('should throw "unsupported error" in browser', function() {
+      sinon.stub(utils, 'isBrowser').callsFake(() => true);
+      expect(
+        () => new JSONReporter(runner, options),
+        'to throw',
+        'file output not supported in browser'
+      );
     });
   });
 });