From 88427e9a4362b82f6fe07587a8618daa48a51534 Mon Sep 17 00:00:00 2001 From: spalger Date: Thu, 1 Sep 2016 15:02:43 -0700 Subject: [PATCH] add test sharding 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. --- package.json | 1 + src/ui/public/test_harness/test_harness.js | 12 +-- .../test_sharding/find_test_bundle_url.js | 20 ++++ .../test_sharding/get_shard_num.js | 28 ++++++ .../get_sharding_params_from_url.js | 26 +++++ .../test_harness/test_sharding/index.js | 1 + .../test_sharding/setup_test_sharding.js | 48 +++++++++ .../setup_top_level_describe_filter.js | 97 +++++++++++++++++++ tasks/config/karma.js | 74 +++++++++++++- tasks/jenkins.js | 2 +- tasks/test.js | 16 ++- 11 files changed, 313 insertions(+), 12 deletions(-) create mode 100644 src/ui/public/test_harness/test_sharding/find_test_bundle_url.js create mode 100644 src/ui/public/test_harness/test_sharding/get_shard_num.js create mode 100644 src/ui/public/test_harness/test_sharding/get_sharding_params_from_url.js create mode 100644 src/ui/public/test_harness/test_sharding/index.js create mode 100644 src/ui/public/test_harness/test_sharding/setup_test_sharding.js create mode 100644 src/ui/public/test_harness/test_sharding/setup_top_level_describe_filter.js diff --git a/package.json b/package.json index 683d4fed98ec9..7b13bce70daf1 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/src/ui/public/test_harness/test_harness.js b/src/ui/public/test_harness/test_harness.js index 9b68ffea17a13..3c1cba2ea0327 100644 --- a/src/ui/public/test_harness/test_harness.js +++ b/src/ui/public/test_harness/test_harness.js @@ -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 @@ -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 () { diff --git a/src/ui/public/test_harness/test_sharding/find_test_bundle_url.js b/src/ui/public/test_harness/test_sharding/find_test_bundle_url.js new file mode 100644 index 0000000000000..26d76a67d5743 --- /dev/null +++ b/src/ui/public/test_harness/test_sharding/find_test_bundle_url.js @@ -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; +} diff --git a/src/ui/public/test_harness/test_sharding/get_shard_num.js b/src/ui/public/test_harness/test_sharding/get_shard_num.js new file mode 100644 index 0000000000000..434efc2d8eac3 --- /dev/null +++ b/src/ui/public/test_harness/test_sharding/get_shard_num.js @@ -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)); +} diff --git a/src/ui/public/test_harness/test_sharding/get_sharding_params_from_url.js b/src/ui/public/test_harness/test_sharding/get_sharding_params_from_url.js new file mode 100644 index 0000000000000..f06d337fdca23 --- /dev/null +++ b/src/ui/public/test_harness/test_sharding/get_sharding_params_from_url.js @@ -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; +} diff --git a/src/ui/public/test_harness/test_sharding/index.js b/src/ui/public/test_harness/test_sharding/index.js new file mode 100644 index 0000000000000..cdd6de476dfc2 --- /dev/null +++ b/src/ui/public/test_harness/test_sharding/index.js @@ -0,0 +1 @@ +export { setupTestSharding } from './setup_test_sharding'; diff --git a/src/ui/public/test_harness/test_sharding/setup_test_sharding.js b/src/ui/public/test_harness/test_sharding/setup_test_sharding.js new file mode 100644 index 0000000000000..ab19a9ec1f6f8 --- /dev/null +++ b/src/ui/public/test_harness/test_sharding/setup_test_sharding.js @@ -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}`); +} diff --git a/src/ui/public/test_harness/test_sharding/setup_top_level_describe_filter.js b/src/ui/public/test_harness/test_sharding/setup_top_level_describe_filter.js new file mode 100644 index 0000000000000..ef712a752dbf8 --- /dev/null +++ b/src/ui/public/test_harness/test_sharding/setup_top_level_describe_filter.js @@ -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; + } + } + }); +} diff --git a/tasks/config/karma.js b/tasks/config/karma.js index af707e9de07e5..e19ea7449f789 100644 --- a/tasks/config/karma.js +++ b/tasks/config/karma.js @@ -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: '', @@ -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; }; diff --git a/tasks/jenkins.js b/tasks/jenkins.js index 5becf1341a5d3..800467dc4b9a5 100644 --- a/tasks/jenkins.js +++ b/tasks/jenkins.js @@ -28,7 +28,7 @@ module.exports = function (grunt) { 'eslint:source', 'licenses', 'test:server', - 'test:browser', + 'test:browser-ci', 'test:api', ]); diff --git a/tasks/test.js b/tasks/test.js index 07ab10d225ef0..8c67078ec2e78 100644 --- a/tasks/test.js +++ b/tasks/test.js @@ -1,4 +1,4 @@ -const _ = require('lodash'); +import _, { keys } from 'lodash'; const visualRegression = require('../utilities/visual_regression'); module.exports = function (grunt) { @@ -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', [