Skip to content

Commit

Permalink
add test sharding
Browse files Browse the repository at this point in the history
The tests in master are currently failing regularly because our current browser tests are serious memory hogs. Investigation reveals that nearly every test is retaining all of the memory it causes to be allocated. We have made some progress to being able to diagnose the problems, but we expect that problem to take some serious work to fix. We need a short-term solution though, and this is it.

Rather than modify the bundling process, we will shard the top-level test suites by name. For now, we've created 4 shards, but adding new shards is trivial if we need to.

Sharding is accomplished by creating a murmur3 hash of the top level suite names, then bucketing based on the hash output. If a test suite resolves to shard2, but we are running shard1, we simply never pass the function to `mocha.describe()`. Rather than redefine every describe statement, we have shimmed the global `window.describe()` function to accomplish this.
  • Loading branch information
spalger committed Sep 2, 2016
1 parent 9bff067 commit 88427e9
Show file tree
Hide file tree
Showing 11 changed files with 313 additions and 12 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@
"makelogs": "3.0.2",
"marked-text-renderer": "0.1.0",
"mocha": "2.5.3",
"murmurhash3js": "3.0.1",
"ncp": "2.0.0",
"nock": "2.10.0",
"npm": "3.10.3",
Expand Down
12 changes: 5 additions & 7 deletions src/ui/public/test_harness/test_harness.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ import _ from 'lodash';
import StackTraceMapper from 'ui/stack_trace_mapper';
import { parse } from 'url';
import $ from 'jquery';
import { setupAutoRelease } from 'auto-release-sinon';
import './test_harness.less';
import 'ng_mock';
import { setupTestSharding } from './test_sharding';

/*** the vislib tests have certain style requirements, so lets make sure they are met ***/
$('body').attr('id', 'test-harness-body'); // so we can make high priority selectors
Expand All @@ -24,13 +26,9 @@ Math.random = _.bindKey(new Nonsense(seed), 'frac');
Math.random.nonsense = new Nonsense(seed);
console.log('Random-ness seed: ' + seed);


/*** Setup auto releasing stubs and spys ***/
require('auto-release-sinon').setupAutoRelease(sinon, window.afterEach);


/*** Make sure that angular-mocks gets setup in the global suite **/

// Setup auto releasing stubs and spys
setupAutoRelease(sinon, window.afterEach);
setupTestSharding();

/*** manually map error stack traces using the sourcemap ***/
before(function () {
Expand Down
20 changes: 20 additions & 0 deletions src/ui/public/test_harness/test_sharding/find_test_bundle_url.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/**
* We don't have a lot of options for passing arguments to the page that karma
* creates, so we tack some query string params onto the test bundle script url.
*
* This function finds that url by looking for a script tag that has
* the "/tests.bundle.js" segment
*
* @return {string} url
*/
export function findTestBundleUrl() {
const scriptTags = document.querySelectorAll('script[src]');
const scriptUrls = [].map.call(scriptTags, el => el.getAttribute('src'));
const testBundleUrl = scriptUrls.find(url => url.includes('/tests.bundle.js'));

if (!testBundleUrl) {
throw new Error('test bundle url couldn\'t be found');
}

return testBundleUrl;
}
28 changes: 28 additions & 0 deletions src/ui/public/test_harness/test_sharding/get_shard_num.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import murmurHash3 from 'murmurhash3js';

// murmur hashes are 32bit unsigned integers
const MAX_HASH = Math.pow(2, 32);

/**
* Determine the shard number for a suite by hashing
* its name and placing it based on the hash
*
* @param {number} shardTotal - the total number of shards
* @param {string} suiteName - the suite name to hash
* @return {number} shardNum - 1-based shard number
*/
export function getShardNum(shardTotal, suiteName) {
const hashIntsPerShard = MAX_HASH / shardTotal;

const hashInt = murmurHash3.x86.hash32(suiteName);

// murmur3 produces 32bit integers, so we devide it by the number of chunks
// to determine which chunk the suite should fall in. +1 because the current
// chunk is 1-based
const shardNum = Math.floor(hashInt / hashIntsPerShard) + 1;

// It's not clear if hash32 can produce the MAX_HASH or not,
// but this just ensures that shard numbers don't go out of bounds
// and cause tests to be ignored unnecessarily
return Math.max(1, Math.min(shardNum, shardTotal));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { parse as parseUrl } from 'url';

/**
* This function extracts the relevant "shards" and "shard_num"
* params from the url.
*
* @param {string} testBundleUrl
* @return {object} params
* @property {number} params.shards - the total number of shards
* @property {number} params.shard_num - the current shard number, 1 based
*/
export function getShardingParamsFromUrl(url) {
const parsedUrl = parseUrl(url, true);
const parsedQuery = parsedUrl.query || {};

const params = {};
if (parsedQuery.shards) {
params.shards = parseInt(parsedQuery.shards, 10);
}

if (parsedQuery.shard_num) {
params.shard_num = parseInt(parsedQuery.shard_num, 10);
}

return params;
}
1 change: 1 addition & 0 deletions src/ui/public/test_harness/test_sharding/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { setupTestSharding } from './setup_test_sharding';
48 changes: 48 additions & 0 deletions src/ui/public/test_harness/test_sharding/setup_test_sharding.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { uniq, defaults } from 'lodash';

import { findTestBundleUrl } from './find_test_bundle_url';
import { getShardingParamsFromUrl } from './get_sharding_params_from_url';
import { setupTopLevelDescribeFilter } from './setup_top_level_describe_filter';
import { getShardNum } from './get_shard_num';

const DEFAULT_PARAMS = {
shards: 1,
shard_num: 1
};

export function setupTestSharding() {
const pageUrl = window.location.href;
const bundleUrl = findTestBundleUrl();

// supports overriding params via the debug page
// url in dev mode
const params = defaults(
{},
getShardingParamsFromUrl(pageUrl),
getShardingParamsFromUrl(bundleUrl),
DEFAULT_PARAMS
);

const { shards: shardTotal, shard_num: shardNum } = params;
if (shardNum < 1 || shardNum > shardTotal) {
throw new TypeError(`shard_num param of ${shardNum} must be greater 0 and less than the total, ${shardTotal}`);
}

// track and log the number of ignored describe calls
const ignoredDescribeShards = [];
before(() => {
const ignoredCount = ignoredDescribeShards.length;
const ignoredFrom = uniq(ignoredDescribeShards).join(', ');
console.log(`Ignored ${ignoredCount} top-level suites from ${ignoredFrom}`);
});

// Filter top-level describe statements as they come
setupTopLevelDescribeFilter(describeName => {
const describeShardNum = getShardNum(shardTotal, describeName);
if (describeShardNum === shardNum) return true;
// track shard numbers that we ignore
ignoredDescribeShards.push(describeShardNum);
});

console.log(`ready to load tests for shard ${shardNum} of ${shardTotal}`);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/**
* Intercept all calls to mocha.describe() and determine
* which calls make it through using a filter function.
*
* The filter function is also only called for top-level
* describe() calls; all describe calls nested within another
* are allowed based on the filter value for the parent
* describe
*
* ## example
*
* assume tests that look like this:
*
* ```js
* describe('section 1', () => {
* describe('item 1', () => {
*
* })
* })
* ```
*
* If the filter function returned true for "section 1" then "item 1"
* would automatically be defined. If it returned false for "section 1"
* then "section 1" would be ignored and "item 1" would never be defined
*
* @param {function} test - a function that takes the first argument
* passed to describe, the sections name, and
* returns true if the describe call should
* be delegated to mocha, any other value causes
* the describe call to be ignored
* @return {undefined}
*/
export function setupTopLevelDescribeFilter(test) {
const originalDescribe = window.describe;

if (!originalDescribe) {
throw new TypeError('window.describe must be defined by mocha before test sharding can be setup');
}

/**
* When describe is called it is likely to make additional, nested,
* calls to describe. We track how deeply nested we are at any time
* with a depth counter, `describeCallDepth`.
*
* Before delegating a describe call to mocha we increment
* that counter, and once mocha is done we decrement it.
*
* This way, we can check if `describeCallDepth > 0` at any time
* to know if we are already within a describe call.
*
* ```js
* // +1
* describe('section 1', () => {
* // describeCallDepth = 1
* // +1
* describe('item 1', () => {
* // describeCallDepth = 2
* })
* // -1
* })
* // -1
* // describeCallDepth = 0
* ```
*
* @type {Number}
*/
let describeCallDepth = 0;
const ignoredCalls = [];

// ensure that window.describe isn't messed with by other code
Object.defineProperty(window, 'describe', {
configurable: false,
enumerable: true,
value: function describeInterceptor(describeName, describeBody) {
const context = this;

const isTopLevelCall = describeCallDepth === 0;
const shouldIgnore = isTopLevelCall && Boolean(test(describeName)) === false;
if (shouldIgnore) return;

/**
* we wrap the delegation to mocha in a try/finally block
* to ensure that our describeCallDepth counter stays up
* to date even if the call throws an error.
*
* note that try/finally won't actually catch the error, it
* will continue to propogate up the call stack
*/
try {
describeCallDepth += 1;
originalDescribe.call(context, describeName, describeBody);
} finally {
describeCallDepth -= 1;
}
}
});
}
74 changes: 72 additions & 2 deletions tasks/config/karma.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { times, zipObject } from 'lodash';

const TOTAL_CI_SHARDS = 4;

module.exports = function (grunt) {
return {
const config = {
options: {
// base path that will be used to resolve all patterns (eg. files, exclude)
basePath: '',
Expand Down Expand Up @@ -49,6 +53,72 @@ module.exports = function (grunt) {
{ type: 'text-summary' },
]
}
}
},
};

/**
* ------------------------------------------------------------
* CI sharding
* ------------------------------------------------------------
*
* Every test retains nearly all of the memory it causes to be allocated,
* which has started to kill the test browser as the size of the test suite
* increases. This is a deep-rooted problem that will take some serious
* work to fix.
*
* CI sharding is a short-term solution that splits the top-level describe
* calls into different "shards" and instructs karma to only run one shard
* at a time, reloading the browser in between each shard and forcing the
* memory from the previous shard to be released.
*
* ## how
*
* Rather than modify the bundling process to produce multiple testing
* bundles, top-level describe calls are sharded by their first argument,
* the suite name.
*
* The number of shards to create is controlled with the TOTAL_CI_SHARDS
* constant defined at the top of this file.
*
* ## controlling sharding
*
* To control sharding in a specific karma configuration, the total number
* of shards to create (?shards=X), and the current shard number
* (&shard_num=Y), are added to the testing bundle url and read by the
* test_harness/setup_test_sharding[1] module. This allows us to use a
* different number of shards in different scenarios (ie. running
* `npm run test:browser` runs the tests in a single shard, effectively
* disabling sharding)
*
* These same parameters can also be defined in the URL/query string of the
* karma debug page (started when you run `npm run test:dev`).
*
* ## debugging
*
* It is *possible* that some tests will only pass if run after/with certain
* other suites. To debug this, make sure that your tests pass in isolation
* (by clicking the suite name on the karma debug page) and that it runs
* correctly in it's given shard (using the `?shards=X&shard_num=Y` query
* string params on the karma debug page). You can spot the shard number
* a test is running in by searching for the "ready to load tests for shard X"
* log message.
*
* [1]: src/ui/public/test_harness/test_sharding/setup_test_sharding.js
*/
times(TOTAL_CI_SHARDS, i => {
const n = i + 1;
config[`ciShard-${n}`] = {
singleRun: true,
options: {
files: [
'http://localhost:5610/bundles/commons.bundle.js',
`http://localhost:5610/bundles/tests.bundle.js?shards=${TOTAL_CI_SHARDS}&shard_num=${n}`,
'http://localhost:5610/bundles/commons.style.css',
'http://localhost:5610/bundles/tests.style.css'
]
}
};
});

return config;
};
2 changes: 1 addition & 1 deletion tasks/jenkins.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ module.exports = function (grunt) {
'eslint:source',
'licenses',
'test:server',
'test:browser',
'test:browser-ci',
'test:api',
]);

Expand Down
16 changes: 14 additions & 2 deletions tasks/test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const _ = require('lodash');
import _, { keys } from 'lodash';
const visualRegression = require('../utilities/visual_regression');

module.exports = function (grunt) {
Expand All @@ -22,7 +22,19 @@ module.exports = function (grunt) {
);

grunt.registerTask('test:server', [ 'esvm:test', 'simplemocha:all', 'esvm_shutdown:test' ]);
grunt.registerTask('test:browser', [ 'run:testServer', 'karma:unit' ]);
grunt.registerTask('test:browser', ['run:testServer', 'karma:unit']);
grunt.registerTask('test:browser-ci', () => {
const ciShardTasks = keys(grunt.config.get('karma'))
.filter(key => key.startsWith('ciShard-'))
.map(key => `karma:${key}`);

grunt.log.ok(`Running UI tests in ${ciShardTasks.length} shards`);

grunt.task.run([
'run:testServer',
...ciShardTasks
]);
});
grunt.registerTask('test:coverage', [ 'run:testCoverageServer', 'karma:coverage' ]);

grunt.registerTask('test:quick', [
Expand Down

0 comments on commit 88427e9

Please sign in to comment.