Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test Sharding #5075

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion lib/cli/collect-files.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ module.exports = ({
file: fileArgs,
recursive,
sort,
spec
spec,
shard
} = {}) => {
const unmatched = [];
const specFiles = spec.reduce((specFiles, arg) => {
Expand Down Expand Up @@ -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;
};

Expand All @@ -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
*/
19 changes: 18 additions & 1 deletion lib/cli/run-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const collectFiles = require('./collect-files');
const {format} = require('util');
const {createInvalidLegacyPluginError} = require('../errors');
const {requireOrImport} = require('../nodejs/esm-utils');
const {parseShardString} = require('../utils');
const PluginLoader = require('../plugin-loader');

/**
Expand Down Expand Up @@ -177,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;
Expand All @@ -190,6 +193,20 @@ exports.runMocha = async (mocha, options) => {
return run(mocha, options, fileCollectParams);
};

/**
* Returns true if the given shard string is valid. Also returns true if no
* shard string is given at all.
* @param shardString
* @returns {boolean}
*/
exports.validateShardString = shardString => {
if (!shardString) {
return true;
}

return Boolean(parseShardString(shardString));
};

/**
* Used for `--reporter` and `--ui`. Ensures there's only one, and asserts that
* it actually exists. This must be run _after_ requires are processed (see
Expand Down
1 change: 1 addition & 0 deletions lib/cli/run-option-metadata.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ const TYPES = (exports.types = {
'package',
'reporter',
'ui',
'shard',
'slow',
'timeout'
]
Expand Down
13 changes: 13 additions & 0 deletions lib/cli/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const {
list,
handleRequires,
validateLegacyPlugin,
validateShardString,
runMocha
} = require('./run-helpers');
const {ONE_AND_DONES, ONE_AND_DONE_ARGS} = require('./one-and-dones');
Expand Down Expand Up @@ -231,6 +232,11 @@ exports.builder = yargs =>
description: 'Retry failed tests this many times',
group: GROUPS.RULES
},
shard: {
description:
'Shard tests and execute only the desired shard, specify in the form "desired/total", starting with 1, for example "2/3".',
group: GROUPS.RULES
},
slow: {
default: defaults.slow,
description: 'Specify "slow" test threshold (in milliseconds)',
Expand Down Expand Up @@ -293,6 +299,12 @@ exports.builder = yargs =>
}

if (argv.parallel) {
if (argv.shard) {
throw createUnsupportedError(
'--parallel runs test files in a non-deterministic order, and is mutually exclusive with --shard'
);
}

// yargs.conflicts() can't deal with `--file foo.js --no-parallel`, either
if (argv.file) {
throw createUnsupportedError(
Expand Down Expand Up @@ -349,6 +361,7 @@ exports.builder = yargs =>
const plugins = await handleRequires(argv.require);
validateLegacyPlugin(argv, 'reporter', Mocha.reporters);
validateLegacyPlugin(argv, 'ui', Mocha.interfaces);
validateShardString(argv.shard);
Object.assign(argv, plugins);
} catch (err) {
// this could be a bad --require, bad reporter, ui, etc.
Expand Down
23 changes: 23 additions & 0 deletions lib/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,13 @@ var constants = {
*/
INVALID_REPORTER: 'ERR_MOCHA_INVALID_REPORTER',

/**
* A a shard value is invalid
* @constant
* @default
*/
INVALID_SHARD: 'ERR_MOCHA_INVALID_SHARD',

/**
* `done()` was called twice in a `Test` or `Hook` callback
* @constant
Expand Down Expand Up @@ -210,6 +217,21 @@ function createInvalidReporterError(message, reporter) {
return err;
}

/**
* Creates an error object to be thrown when the shard string is not properly formatted.
*
* @public
* @param {string} message - Error message to be displayed.
* @param {string} shardString - User-specified shard string.
* @returns {Error} instance detailing the error condition
*/
function createInvalidShardError(message, shardString) {
var err = new TypeError(message);
err.code = constants.INVALID_SHARD;
err.shardString = shardString;
return err;
}

/**
* Creates an error object to be thrown when the interface specified in the options was not found.
*
Expand Down Expand Up @@ -540,6 +562,7 @@ module.exports = {
createInvalidPluginError,
createInvalidPluginImplementationError,
createInvalidReporterError,
createInvalidShardError,
createMissingArgumentError,
createMochaInstanceAlreadyDisposedError,
createMochaInstanceAlreadyRunningError,
Expand Down
28 changes: 27 additions & 1 deletion lib/mocha.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const {
createMochaInstanceAlreadyRunningError,
createUnsupportedError
} = require('./errors');
const {parseShardString} = require('./utils');
const {EVENT_FILE_PRE_REQUIRE, EVENT_FILE_POST_REQUIRE, EVENT_FILE_REQUIRE} =
Suite.constants;
var debug = require('debug')('mocha:mocha');
Expand Down Expand Up @@ -171,6 +172,8 @@ exports.run = function (...args) {
* @param {Object} [options.reporterOption] - Reporter settings object.
* @param {number} [options.retries] - Number of times to retry failed tests.
* @param {number} [options.slow] - Slow threshold value.
* @param {number} [options.slow] - Slow threshold value.
* @param {string} [options.shard] - The shard config represented as a string "desired"/"total".
* @param {number|string} [options.timeout] - Timeout threshold value.
* @param {string} [options.ui] - Interface name.
* @param {boolean} [options.parallel] - Run jobs in parallel.
Expand All @@ -196,7 +199,8 @@ function Mocha(options = {}) {
options.reporterOption || options.reporterOptions // for backwards compatibility
)
.slow(options.slow)
.global(options.global);
.global(options.global)
.shard(options.shard);

// this guard exists because Suite#timeout does not consider `undefined` to be valid input
if (typeof options.timeout !== 'undefined') {
Expand Down Expand Up @@ -785,6 +789,28 @@ Mocha.prototype.slow = function (msecs) {
return this;
};

/**
* Converts and validates the shard string into a shard object.
*
* @public
* @see [CLI option](../#-shard)
* @param {string} shardString - The shard config represented as a string "desired"/"total".
* @return {Mocha} this
* @chainable
*/
Mocha.prototype.shard = function (shardString) {
if (!shardString) {
return this;
}

const [desiredShard, totalShards] = parseShardString(shardString);
this.options.shard = {
desiredShard,
totalShards
};
return this;
};

/**
* Forces all tests to either accept a `done` callback or return a promise.
*
Expand Down
1 change: 1 addition & 0 deletions lib/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -1053,6 +1053,7 @@ Runner.prototype.run = function (fn, opts = {}) {
rootSuite.filterOnly();
debug('run(): filtered exclusive Runnables');
}

this.state = constants.STATE_RUNNING;
if (this._opts.delay) {
this.emit(constants.EVENT_DELAY_END);
Expand Down
22 changes: 22 additions & 0 deletions lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
var path = require('path');
var util = require('util');
var he = require('he');
const {createInvalidShardError} = require('./errors');

const MOCHA_ID_PROP_NAME = '__mocha_id__';

Expand Down Expand Up @@ -647,3 +648,24 @@ exports.assignNewMochaID = obj => {
*/
exports.getMochaID = obj =>
obj && typeof obj === 'object' ? obj[MOCHA_ID_PROP_NAME] : undefined;

/**
* Returns the desired shard and total shards as numbers given a shard string
* @param shardString
* @returns {[number,number]}
*/
exports.parseShardString = shardString => {
const shardParts = shardString.split('/');
const desiredShard = parseInt(shardParts[0]);
const totalShards = parseInt(shardParts[1]);
if (isNaN(desiredShard) || isNaN(totalShards)) {
throw createInvalidShardError('Invalid shard values.', shardString);
}
if (desiredShard <= 0 || totalShards < desiredShard) {
throw createInvalidShardError(
'Desired shard must be greater than 0 and less than total shards.',
shardString
);
}
return [desiredShard, totalShards];
};
6 changes: 6 additions & 0 deletions test/assertions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
5 changes: 5 additions & 0 deletions test/integration/fixtures/options/shard-file-alpha.fixture.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
'use strict';

describe('suite 1', function () {
it('test 1', function () {});
});
17 changes: 17 additions & 0 deletions test/integration/fixtures/options/shard-file-beta.fixture.js
Original file line number Diff line number Diff line change
@@ -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 () {});
});
21 changes: 21 additions & 0 deletions test/integration/fixtures/options/shard-file-theta.fixture.js
Original file line number Diff line number Diff line change
@@ -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 () {});
});
Loading