make invalid event data throw a fatal error

the only time this should happen in a non-unit-test context is when a mocha developer introduces a bug also fix a `rewiremock` call in `ParallelBuffered` reporter tests
mochajs · Oct 7, 2020 · 1288205 · 1288205
1 parent 2e214f3
commit 1288205
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 17 deletions.
diff --git a/lib/nodejs/parallel-buffered-runner.js b/lib/nodejs/parallel-buffered-runner.js
@@ -14,6 +14,7 @@ const {BufferedWorkerPool} = require('./buffered-worker-pool');
 const {setInterval, clearInterval} = global;
 const {createMap, constants} = require('../utils');
 const {MOCHA_ID_PROP_NAME} = constants;
+const {createFatalError} = require('../errors');
 
 const DEFAULT_WORKER_REPORTER = require.resolve(
   './reporters/parallel-buffered'
@@ -140,15 +141,20 @@ class ParallelBufferedRunner extends Runner {
         const {parent, prop} = stack.pop();
         const obj = parent[prop];
         let newObj;
-        if (obj && typeof obj === 'object' && obj[MOCHA_ID_PROP_NAME]) {
-          const id = obj[MOCHA_ID_PROP_NAME];
-          newObj = this._linkedObjectMap.has(id)
-            ? Object.assign(this._linkedObjectMap.get(id), obj)
-            : obj;
-          this._linkedObjectMap.set(id, newObj);
-          parent[prop] = newObj;
-        } else {
-          newObj = obj;
+        if (obj && typeof obj === 'object') {
+          if (obj[MOCHA_ID_PROP_NAME]) {
+            const id = obj[MOCHA_ID_PROP_NAME];
+            newObj = this._linkedObjectMap.has(id)
+              ? Object.assign(this._linkedObjectMap.get(id), obj)
+              : obj;
+            this._linkedObjectMap.set(id, newObj);
+            parent[prop] = newObj;
+          } else {
+            throw createFatalError(
+              'Object missing ID received in event data',
+              obj
+            );
+          }
         }
         Object.keys(newObj).forEach(key => {
           const value = obj[key];

diff --git a/test/node-unit/parallel-buffered-runner.spec.js b/test/node-unit/parallel-buffered-runner.spec.js
@@ -22,13 +22,14 @@ describe('parallel-buffered-runner', function() {
     let ParallelBufferedRunner;
     let suite;
     let warn;
-    let cpuCount;
+    let fatalError;
 
     beforeEach(function() {
-      cpuCount = 1;
       suite = new Suite('a root suite', {}, true);
       warn = sinon.stub();
 
+      fatalError = new Error();
+
       // tests will want to further define the behavior of these.
       run = sinon.stub();
       terminate = sinon.stub();
@@ -48,10 +49,12 @@ describe('parallel-buffered-runner', function() {
           '../../lib/nodejs/buffered-worker-pool': {
             BufferedWorkerPool
           },
-          os: {
-            cpus: sinon.stub().callsFake(() => new Array(cpuCount))
-          },
-          '../../lib/utils': r.with({warn}).callThrough()
+          '../../lib/utils': r.with({warn}).callThrough(),
+          '../../lib/errors': r
+            .with({
+              createFatalError: sinon.stub().returns(fatalError)
+            })
+            .callThrough()
         })
       );
     });
@@ -131,7 +134,7 @@ describe('parallel-buffered-runner', function() {
 
         describe('when instructed to link objects', function() {
           beforeEach(function() {
-            runner.linkPartialObjects(true);
+            runner._linkPartialObjects = true;
           });
 
           it('should create object references', function() {
@@ -185,6 +188,54 @@ describe('parallel-buffered-runner', function() {
               }
             );
           });
+
+          describe('when event data object is missing an ID', function() {
+            it('should result in an uncaught exception', function(done) {
+              const options = {reporter: runner._workerReporter};
+              sinon.spy(runner, 'uncaught');
+              const someSuite = {
+                title: 'some suite',
+                [MOCHA_ID_PROP_NAME]: 'bar'
+              };
+
+              run.withArgs('some-file.js', options).resolves({
+                failureCount: 0,
+                events: [
+                  {
+                    eventName: EVENT_SUITE_END,
+                    data: someSuite
+                  },
+                  {
+                    eventName: EVENT_TEST_PASS,
+                    data: {
+                      title: 'some test',
+                      // note missing ID right here
+                      parent: {
+                        // this stub object points to someSuite with id 'bar'
+                        [MOCHA_ID_PROP_NAME]: 'bar'
+                      }
+                    }
+                  },
+                  {
+                    eventName: EVENT_SUITE_END,
+                    // ensure we are not passing the _same_ someSuite,
+                    // because we won't get the same one from the subprocess
+                    data: {...someSuite}
+                  }
+                ]
+              });
+
+              runner.run(
+                () => {
+                  expect(runner.uncaught, 'to have a call satisfying', [
+                    fatalError
+                  ]);
+                  done();
+                },
+                {files: ['some-file.js'], options: {}}
+              );
+            });
+          });
         });
 
         describe('when a worker fails', function() {
@@ -352,6 +403,7 @@ describe('parallel-buffered-runner', function() {
                 );
               });
             });
+
             describe('when subsequent files already started running', function() {
               it('should cleanly terminate the thread pool', function(done) {
                 const options = {reporter: runner._workerReporter};
@@ -463,6 +515,7 @@ describe('parallel-buffered-runner', function() {
               );
             });
           });
+
           describe('when an event contains an error and has positive failures', function() {
             describe('when subsequent files have not yet been run', function() {
               it('should cleanly terminate the thread pool', function(done) {

diff --git a/test/node-unit/reporters/parallel-buffered.spec.js b/test/node-unit/reporters/parallel-buffered.spec.js
@@ -29,7 +29,7 @@ describe('ParallelBuffered', function() {
   beforeEach(function() {
     runner = new EventEmitter();
     ParallelBuffered = rewiremock.proxy(
-      require.resolve('../../../lib/nodejs/reporters/parallel-buffered'),
+      () => require('../../../lib/nodejs/reporters/parallel-buffered'),
       {
         '../../../lib/nodejs/serializer': {
           SerializableEvent: {