feat(sharding): switched to file based sharding

lib/cli/collect-files.js

@@ -28,7 +28,8 @@ module.exports = ({
   file: fileArgs,
   recursive,
   sort,
-  spec
+  spec,
+  shard
 } = {}) => {
   const unmatched = [];
   const specFiles = spec.reduce((specFiles, arg) => {
@@ -79,6 +80,15 @@ module.exports = ({
     });
   }
 
+  // Filter out files that don't match the shard
+  if (shard && shard.totalShards > 1) {
+    const desiredShardIndex = shard.desiredShard - 1;
+    return files.filter(
+      (filename, fileNumber) =>
+        fileNumber % shard.totalShards === desiredShardIndex
+    );
+  }
+
   return files;
 };
 
@@ -92,4 +102,7 @@ module.exports = ({
  * @property {string[]} file - List of additional files to include
  * @property {boolean} recursive - Find files recursively
  * @property {boolean} sort - Sort test files
+ * @property {Object} shard - Shard configuration
+ * @property {number} shard.desiredShard - Shard to run
+ * @property {number} shard.totalShards - Total shards
  */

lib/cli/run-helpers.js

@@ -178,7 +178,9 @@ exports.runMocha = async (mocha, options) => {
     file,
     recursive,
     sort,
-    spec
+    spec,
+    // Use the processed --shard argument, not what is directly passed in through the argv.
+    shard: mocha.options.shard
   };
 
   let run;

lib/runner.js

@@ -1054,19 +1054,6 @@ Runner.prototype.run = function (fn, opts = {}) {
       debug('run(): filtered exclusive Runnables');
     }
 
-    if (
-      options.shard &&
-      options.shard.totalShards &&
-      options.shard.desiredShard
-    ) {
-      const shardContext = {
-        testNum: 0,
-        totalShards: options.shard.totalShards
-      };
-
-      rootSuite.filterByShard(options.shard.desiredShard, shardContext);
-    }
-
     this.state = constants.STATE_RUNNING;
     if (this._opts.delay) {
       this.emit(constants.EVENT_DELAY_END);

lib/suite.js

@@ -488,35 +488,6 @@ Suite.prototype.filterOnly = function filterOnly() {
   return this.tests.length > 0 || this.suites.length > 0;
 };
 
-/**
- * Filter suites based on the current shard.
- *
- * @private
- * @returns {Boolean}
- */
-Suite.prototype.filterByShard = function filterByShard(
-  desiredShardNumber,
-  shardContext
-) {
-  this.tests = this.tests.filter(function (childTest) {
-    shardContext.testNum++;
-
-    const shardNumber =
-      ((shardContext.testNum - 1) % shardContext.totalShards) + 1;
-
-    childTest.title = `Shard #${shardNumber} Test #${shardContext.testNum}: ${childTest.title}`;
-
-    return desiredShardNumber === shardNumber;
-  });
-
-  this.suites = this.suites.filter(function (childSuite) {
-    return childSuite.filterByShard(desiredShardNumber, shardContext);
-  });
-
-  // Keep the suite only if there is something to run
-  return this.tests.length > 0 || this.suites.length > 0;
-};
-
 /**
  * Adds a suite to the list of subsuites marked `only`.
  *

test/assertions.js

@@ -146,6 +146,12 @@ module.exports = {
           expect(result.stats.tests, '[not] to be', count);
         }
       )
+      .addAssertion(
+        '<JSONResult> [not] to have suite count <number>',
+        (expect, result, count) => {
+          expect(result.stats.suites, '[not] to be', count);
+        }
+      )
       .addAssertion(
         '<JSONResult> [not] to have failed [test] count <number>',
         (expect, result, count) => {

test/integration/fixtures/options/shard-file-alpha.fixture.js

@@ -0,0 +1,5 @@
+'use strict';
+
+describe('suite 1', function () {
+  it('test 1', function () {});
+});

test/integration/fixtures/options/shard-file-beta.fixture.js

@@ -0,0 +1,17 @@
+'use strict';
+
+describe('suite 2', function () {
+  it('test 1', function () {});
+});
+
+describe('suite 3', function () {
+  it('test 1', function () {});
+});
+
+describe('suite 4', function () {
+  it('test 1', function () {});
+});
+
+describe('suite 5', function () {
+  it('test 1', function () {});
+});

test/integration/fixtures/options/shard-file-theta.fixture.js

@@ -0,0 +1,21 @@
+'use strict';
+
+describe('suite 6', function () {
+  it('test 1', function () {});
+});
+
+describe('suite 7', function () {
+  it('test 1', function () {});
+});
+
+describe('suite 8', function () {
+  it('test 1', function () {});
+});
+
+describe('suite 9', function () {
+  it('test 1', function () {});
+});
+
+describe('suite 10', function () {
+  it('test 1', function () {});
+});

test/integration/options/shard.spec.js

@@ -1,40 +1,80 @@
 'use strict';
 
 var helpers = require('../helpers');
+const {posix: path} = require('path');
 var runMocha = helpers.runMocha;
 var runMochaJSON = helpers.runMochaJSON;
-
-var FIXTURE = 'options/shard';
+var resolvePath = helpers.resolveFixturePath;
 
 describe('--shard', function () {
-  it('should run specs for a given shard 1/2', function (done) {
-    runMochaJSON(FIXTURE, ['--shard', '1/2'], function (err, res) {
-      if (err) {
-        return done(err);
-      }
-      expect(res, 'to have passed')
-        .and('to have passed test count', 2)
-        .and('not to have pending tests');
-      done();
-    });
-  });
+  var fixtures = {
+    alpha: {
+      name: 'alpha',
+      suiteCount: 1,
+      path: path.join('options', 'shard-file-alpha')
+    },
+    beta: {
+      name: 'beta',
+      suiteCount: 4,
+      path: path.join('options', 'shard-file-beta')
+    },
+    theta: {
+      name: 'theta',
+      suiteCount: 5,
+      path: path.join('options', 'shard-file-theta')
+    }
+  };
+
+  const combinations = [
+    // Each combination with distinct files
+    [fixtures.alpha, fixtures.beta, fixtures.theta],
+    [fixtures.alpha, fixtures.theta, fixtures.beta],
+    [fixtures.beta, fixtures.alpha, fixtures.theta],
+    [fixtures.beta, fixtures.theta, fixtures.alpha],
+    [fixtures.theta, fixtures.alpha, fixtures.beta],
+    [fixtures.theta, fixtures.beta, fixtures.alpha]
+  ];
 
-  it('should run specs for a given shard 2/2', function (done) {
-    runMochaJSON(FIXTURE, ['--shard', '2/2'], function (err, res) {
-      if (err) {
-        return done(err);
+  const shards = ['1/2', '2/2'];
+
+  for (const [fixture1, fixture2, fixture3] of combinations) {
+    for (const shard of shards) {
+      const testName = `should run specs for combination of ${fixture1.name}, ${fixture2.name}, and ${fixture3.name} on shard ${shard}`;
+      let expectedCount;
+
+      if (shard === shards[0]) {
+        expectedCount = fixture1.suiteCount + fixture3.suiteCount;
+      } else {
+        expectedCount = fixture2.suiteCount;
       }
-      expect(res, 'to have passed')
-        .and('to have passed test count', 1)
-        .and('not to have pending tests');
-      done();
-    });
-  });
+
+      it(testName, function (done) {
+        const args = [
+          '--file',
+          resolvePath(fixture1.path),
+          '--file',
+          resolvePath(fixture2.path),
+          '--shard',
+          shard
+        ];
+        // Test that come in through --file are always run first
+        runMochaJSON(fixture3.path, args, function (err, res) {
+          if (err) {
+            return done(err);
+          }
+          expect(res, 'to have passed')
+            .and('to have suite count', expectedCount)
+            .and('not to have pending tests');
+          done();
+        });
+      });
+    }
+  }
 
   it('should fail if the parameter has non numeric value', function (done) {
     var spawnOpts = {stdio: 'pipe'};
     runMocha(
-      FIXTURE,
+      fixtures.alpha.path,
       ['--shard', '1/a'],
       function (err, res) {
         if (err) {
@@ -50,7 +90,7 @@ describe('--shard', function () {
   it('should fail if the parameter has invalid numeric value', function (done) {
     var spawnOpts = {stdio: 'pipe'};
     runMocha(
-      FIXTURE,
+      fixtures.alpha.path,
       ['--shard', '2/1'],
       function (err, res) {
         if (err) {

test/unit/suite.spec.js

@@ -647,28 +647,6 @@ describe('Suite', function () {
       });
     });
 
-    describe('filterByShard()', function () {
-      it('should filter out all tests that are not in the current shard', function () {
-        const suite = new Suite('a');
-        const nested = new Suite('b');
-        const test = new Test('c');
-        const test2 = new Test('d');
-
-        suite.suites.push(nested);
-        suite.tests.push(test);
-        suite.tests.push(test2);
-
-        suite.filterByShard(1, {totalShards: 2, testNum: 0});
-
-        expect(suite, 'to satisfy', {
-          suites: expect.it('to be empty'),
-          tests: expect
-            .it('to have length', 1)
-            .and('to have an item satisfying', {title: 'Shard #1 Test #1: c'})
-        });
-      });
-    });
-
     describe('markOnly()', function () {
       it('should call appendOnlySuite on parent', function () {
         const suite = new Suite('foo');