From 4a6ff3aa474bda2ee5fc7ee480c4e2b52b132840 Mon Sep 17 00:00:00 2001 From: Andrew Halberstadt Date: Tue, 10 Mar 2015 09:55:30 -0400 Subject: [PATCH] Bug 1131098 - Make mochitest use manifestparser's chunking algorithms and remove JS based ones, r=jmaher --HG-- extra : rebase_source : 1374462488e20cbd6e2048d8d5c63aff49a87911 --- .../source/test/addons/jetpack-addon.ini | 1 - testing/mochitest/bisection.py | 45 -------- testing/mochitest/browser-harness.xul | 5 - testing/mochitest/chunkifyTests.js | 72 ------------ testing/mochitest/jetpack-addon-harness.js | 5 - testing/mochitest/jetpack-package-harness.js | 5 - testing/mochitest/manifestLibrary.js | 1 + testing/mochitest/runtests.py | 103 +++++++++++------- testing/mochitest/runtestsremote.py | 27 ++--- testing/mochitest/tests/SimpleTest/setup.js | 5 +- .../manifestparser/manifestparser/filters.py | 18 ++- 11 files changed, 87 insertions(+), 200 deletions(-) diff --git a/addon-sdk/source/test/addons/jetpack-addon.ini b/addon-sdk/source/test/addons/jetpack-addon.ini index fa0a69517369..5b0ef7757486 100644 --- a/addon-sdk/source/test/addons/jetpack-addon.ini +++ b/addon-sdk/source/test/addons/jetpack-addon.ini @@ -13,7 +13,6 @@ skip-if = true [e10s-tabs.xpi] skip-if = true [l10n.xpi] -[l10n-manifest.xpi] [l10n-properties.xpi] [layout-change.xpi] [main.xpi] diff --git a/testing/mochitest/bisection.py b/testing/mochitest/bisection.py index 4ba3eaf27968..5ffd27719229 100644 --- a/testing/mochitest/bisection.py +++ b/testing/mochitest/bisection.py @@ -31,53 +31,8 @@ class Bisect(object): self.expectedError = expectedError self.result = result - def get_test_chunk(self, options, tests): - "This method is used to return the chunk of test that is to be run" - if not options.totalChunks or not options.thisChunk: - return tests - - # The logic here is same as chunkifyTests.js, we need this for - # bisecting tests. - if options.chunkByDir: - tests_by_dir = {} - test_dirs = [] - for test in tests: - directory = test.split("/") - directory = directory[ - 0:min( - options.chunkByDir, - len(directory) - - 1)] - directory = "/".join(directory) - - if directory not in tests_by_dir: - tests_by_dir[directory] = [test] - test_dirs.append(directory) - else: - tests_by_dir[directory].append(test) - - tests_per_chunk = float(len(test_dirs)) / options.totalChunks - start = int(round((options.thisChunk - 1) * tests_per_chunk)) - end = int(round((options.thisChunk) * tests_per_chunk)) - test_dirs = test_dirs[start:end] - return_tests = [] - for directory in test_dirs: - return_tests += tests_by_dir[directory] - - else: - tests_per_chunk = float(len(tests)) / options.totalChunks - start = int(round((options.thisChunk - 1) * tests_per_chunk)) - end = int(round(options.thisChunk * tests_per_chunk)) - return_tests = tests[start:end] - - options.totalChunks = None - options.thisChunk = None - options.chunkByDir = None - return return_tests - def get_tests_for_bisection(self, options, tests): "Make a list of tests for bisection from a given list of tests" - tests = self.get_test_chunk(options, tests) bisectlist = [] for test in tests: bisectlist.append(test) diff --git a/testing/mochitest/browser-harness.xul b/testing/mochitest/browser-harness.xul index 42443fb51a76..99d9166621fd 100644 --- a/testing/mochitest/browser-harness.xul +++ b/testing/mochitest/browser-harness.xul @@ -229,11 +229,6 @@ fileNames = skipTests(fileNames, gConfig.startAt, gConfig.endAt); } - if (gConfig.totalChunks && gConfig.thisChunk) { - fileNames = chunkifyTests(fileNames, gConfig.totalChunks, - gConfig.thisChunk, gConfig.chunkByDir); - } - createTester(fileNames.map(function (f) { return new browserTest(f); })); } diff --git a/testing/mochitest/chunkifyTests.js b/testing/mochitest/chunkifyTests.js index d09e29e8ed19..02f972e8dfa6 100644 --- a/testing/mochitest/chunkifyTests.js +++ b/testing/mochitest/chunkifyTests.js @@ -2,78 +2,6 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -function chunkifyTests(tests, totalChunks, thisChunk, chunkByDir, logger) { - var total_chunks = parseInt(totalChunks); - // this_chunk is in the range [1,total_chunks] - var this_chunk = parseInt(thisChunk); - var returnTests; - - // We want to split the tests up into chunks according to which directory - // they're in - if (chunkByDir) { - chunkByDir = parseInt(chunkByDir); - var tests_by_dir = {}; - var test_dirs = [] - for (var i = 0; i < tests.length; ++i) { - if ((tests[i] instanceof Object) && ('test' in tests[i])) { - var test_path = tests[i]['test']['url']; - } - else if ((tests[i] instanceof Object) && ('url' in tests[i])) { - // This condition is needed to run --chunk-by-dir on mochitest bc and dt. - var test_path = tests[i]['url']; - } - else { - // This condition is needed to run --chunk-by-dir on android chunks. - var test_path = tests[i]; - } - if (test_path[0] == '/') { - test_path = test_path.substr(1); - } - // mochitest-chrome and mochitest-browser-chrome pass an array of chrome:// - // URIs - var protocolRegexp = /^[a-zA-Z]+:\/\//; - if (protocolRegexp.test(test_path)) { - test_path = test_path.replace(protocolRegexp, ""); - } - var dir = test_path.split("/"); - // We want the first chunkByDir+1 components, or everything but the - // last component, whichever is less. - // we add 1 to chunkByDir since 'tests' is always part of the path, and - // want to ignore the last component since it's the test filename. - dir = dir.slice(0, Math.min(chunkByDir+1, dir.length-1)); - // reconstruct a directory name - dir = dir.join("/"); - if (!(dir in tests_by_dir)) { - tests_by_dir[dir] = [tests[i]]; - test_dirs.push(dir); - } else { - tests_by_dir[dir].push(tests[i]); - } - } - var tests_per_chunk = test_dirs.length / total_chunks; - var start = Math.round((this_chunk-1) * tests_per_chunk); - var end = Math.round(this_chunk * tests_per_chunk); - returnTests = []; - var dirs = [] - for (var i = start; i < end; ++i) { - var dir = test_dirs[i]; - dirs.push(dir); - returnTests = returnTests.concat(tests_by_dir[dir]); - } - if (logger) - logger.log("Running tests in " + dirs.join(", ")); - } else { - var tests_per_chunk = tests.length / total_chunks; - var start = Math.round((this_chunk-1) * tests_per_chunk); - var end = Math.round(this_chunk * tests_per_chunk); - returnTests = tests.slice(start, end); - if (logger) - logger.log("Running tests " + (start+1) + "-" + end + "/" + tests.length); - } - - return returnTests; -} - function skipTests(tests, startTestPattern, endTestPattern) { var startIndex = 0, endIndex = tests.length - 1; for (var i = 0; i < tests.length; ++i) { diff --git a/testing/mochitest/jetpack-addon-harness.js b/testing/mochitest/jetpack-addon-harness.js index dd116a474370..79fe61d6a1b3 100644 --- a/testing/mochitest/jetpack-addon-harness.js +++ b/testing/mochitest/jetpack-addon-harness.js @@ -153,11 +153,6 @@ function testInit() { fileNames = skipTests(fileNames, config.startAt, config.endAt); } - if (config.totalChunks && config.thisChunk) { - fileNames = chunkifyTests(fileNames, config.totalChunks, - config.thisChunk, config.chunkByDir); - } - // Override the SDK modules if necessary try { let sdklibs = Services.prefs.getCharPref("extensions.sdk.path"); diff --git a/testing/mochitest/jetpack-package-harness.js b/testing/mochitest/jetpack-package-harness.js index fff9f8537d04..eb1b89f3d5cd 100644 --- a/testing/mochitest/jetpack-package-harness.js +++ b/testing/mochitest/jetpack-package-harness.js @@ -134,11 +134,6 @@ function testInit() { fileNames = skipTests(fileNames, config.startAt, config.endAt); } - if (config.totalChunks && config.thisChunk) { - fileNames = chunkifyTests(fileNames, config.totalChunks, - config.thisChunk, config.chunkByDir); - } - // The SDK assumes it is being run from resource URIs let chromeReg = Cc["@mozilla.org/chrome/chrome-registry;1"].getService(Ci.nsIChromeRegistry); let realPath = chromeReg.convertChromeURL(Services.io.newURI(TEST_PACKAGE, null, null)); diff --git a/testing/mochitest/manifestLibrary.js b/testing/mochitest/manifestLibrary.js index 6858500d39ca..ea282094e49b 100644 --- a/testing/mochitest/manifestLibrary.js +++ b/testing/mochitest/manifestLibrary.js @@ -61,6 +61,7 @@ function getTestManifest(url, params, callback) { } // Test Filtering Code +// TODO Only used by ipc tests, remove once those are implemented sanely /* Open the file referenced by runOnly|exclude and use that to compare against diff --git a/testing/mochitest/runtests.py b/testing/mochitest/runtests.py index e48318f951c8..af0b541464f5 100644 --- a/testing/mochitest/runtests.py +++ b/testing/mochitest/runtests.py @@ -46,7 +46,11 @@ from automationutils import ( from datetime import datetime from manifestparser import TestManifest -from manifestparser.filters import subsuite +from manifestparser.filters import ( + chunk_by_dir, + chunk_by_slice, + subsuite, +) from mochitest_options import MochitestOptions from mozprofile import Profile, Preferences from mozprofile.permissions import ServerLocations @@ -54,9 +58,6 @@ from urllib import quote_plus as encodeURIComponent from mozlog.structured.formatters import TbplFormatter from mozlog.structured import commandline -# This should use the `which` module already in tree, but it is -# not yet present in the mozharness environment -from mozrunner.utils import findInPath as which ########################### # Option for NSPR logging # @@ -564,8 +565,6 @@ class MochitestUtilsMixin(object): closeWhenDone -- closes the browser after the tests hideResultsTable -- hides the table of individual test results logFile -- logs test run to an absolute path - totalChunks -- how many chunks to split tests into - thisChunk -- which chunk to run startAt -- name of test to start at endAt -- name of test to end at timeout -- per-test timeout in seconds @@ -612,11 +611,6 @@ class MochitestUtilsMixin(object): "consoleLevel=" + encodeURIComponent( options.consoleLevel)) - if options.totalChunks: - self.urlOpts.append("totalChunks=%d" % options.totalChunks) - self.urlOpts.append("thisChunk=%d" % options.thisChunk) - if options.chunkByDir: - self.urlOpts.append("chunkByDir=%d" % options.chunkByDir) if options.startAt: self.urlOpts.append("startAt=%s" % options.startAt) if options.endAt: @@ -638,12 +632,6 @@ class MochitestUtilsMixin(object): options.testPath)) and options.repeat > 0: self.urlOpts.append("testname=%s" % ("/").join([self.TEST_PATH, options.testPath])) - if options.testManifest: - self.urlOpts.append("testManifest=%s" % options.testManifest) - if hasattr(options, 'runOnly') and options.runOnly: - self.urlOpts.append("runOnly=true") - else: - self.urlOpts.append("runOnly=false") if options.manifestFile: self.urlOpts.append("manifestFile=%s" % options.manifestFile) if options.failureFile: @@ -1863,14 +1851,7 @@ class Mochitest(MochitestUtilsMixin): self.setTestRoot(options) manifest = self.getTestManifest(options) if manifest: - # Python 2.6 doesn't allow unicode keys to be used for keyword - # arguments. This gross hack works around the problem until we - # rid ourselves of 2.6. - info = {} - for k, v in mozinfo.info.items(): - if isinstance(k, unicode): - k = k.encode('ascii') - info[k] = v + info = mozinfo.info # Bug 883858 - return all tests including disabled tests testPath = self.getTestPath(options) @@ -1879,19 +1860,70 @@ class Mochitest(MochitestUtilsMixin): testPath.endswith('.xhtml') or \ testPath.endswith('.xul') or \ testPath.endswith('.js'): - # In the case where we have a single file, we don't want to - # filter based on options such as subsuite. - tests = manifest.active_tests(disabled=disabled, **info) + # In the case where we have a single file, we don't want to + # filter based on options such as subsuite. + tests = manifest.active_tests( + exists=False, disabled=disabled, **info) for test in tests: if 'disabled' in test: del test['disabled'] else: - filters = [subsuite(options.subsuite)] + # Bug 1089034 - imptest failure expectations are encoded as + # test manifests, even though they aren't tests. This gross + # hack causes several problems in automation including + # throwing off the chunking numbers. Remove them manually + # until bug 1089034 is fixed. + def remove_imptest_failure_expectations(tests, values): + return (t for t in tests + if 'imptests/failures' not in t['path']) + + # filter that implements old-style JSON manifests, remove + # once everything is using .ini + def apply_json_manifest(tests, values): + m = os.path.join(SCRIPT_DIR, options.testManifest) + with open(m, 'r') as f: + m = json.loads(f.read()) + + runtests = m.get('runtests') + exctests = m.get('excludetests') + if runtests is None and exctests is None: + if options.runOnly: + runtests = m + else: + exctests = m + + disabled = 'disabled by {}'.format(options.testManifest) + for t in tests: + if runtests and not any(t['relpath'].startswith(r) + for r in runtests): + t['disabled'] = disabled + if exctests and any(t['relpath'].startswith(r) + for r in exctests): + t['disabled'] = disabled + yield t + + filters = [ + remove_imptest_failure_expectations, + subsuite(options.subsuite), + ] + + if options.testManifest: + filters.append(apply_json_manifest) + + # Add chunking filters if specified + if options.chunkByDir: + filters.append(chunk_by_dir(options.thisChunk, + options.totalChunks, + options.chunkByDir)) + elif options.totalChunks: + filters.append(chunk_by_slice(options.thisChunk, + options.totalChunks)) tests = manifest.active_tests( - disabled=disabled, filters=filters, **info) + exists=False, disabled=disabled, filters=filters, **info) if len(tests) == 0: - tests = manifest.active_tests(disabled=True, **info) + tests = manifest.active_tests( + exists=False, disabled=True, **info) paths = [] @@ -2011,15 +2043,6 @@ class Mochitest(MochitestUtilsMixin): # code for --run-by-dir dirs = self.getDirectories(options) - if options.totalChunks > 1: - chunkSize = int(len(dirs) / options.totalChunks) + 1 - start = chunkSize * (options.thisChunk - 1) - end = chunkSize * (options.thisChunk) - dirs = dirs[start:end] - - options.totalChunks = None - options.thisChunk = None - options.chunkByDir = 0 result = 1 # default value, if no tests are run. inputTestPath = self.getTestPath(options) for dir in dirs: diff --git a/testing/mochitest/runtestsremote.py b/testing/mochitest/runtestsremote.py index 9ddcd7b1ea7b..747594d312d4 100644 --- a/testing/mochitest/runtestsremote.py +++ b/testing/mochitest/runtestsremote.py @@ -5,9 +5,7 @@ import base64 import json import logging -import math import os -import re import shutil import sys import tempfile @@ -24,9 +22,10 @@ from runtests import Mochitest, MessageLogger from mochitest_options import MochitestOptions from mozlog import structured +from manifestparser import TestManifest +from manifestparser.filters import chunk_by_slice import devicemanager import droid -import manifestparser import mozinfo import moznetwork @@ -799,24 +798,15 @@ def main(args): # sut may wait up to 300 s for a robocop am process before returning dm.default_timeout = 320 - mp = manifestparser.TestManifest(strict=False) + mp = TestManifest(strict=False) # TODO: pull this in dynamically mp.read(options.robocopIni) - robocop_tests = mp.active_tests(exists=False, **mozinfo.info) - tests = [] - my_tests = tests - for test in robocop_tests: - tests.append(test['name']) + filters = [] if options.totalChunks: - tests_per_chunk = math.ceil( - len(tests) / (options.totalChunks * 1.0)) - start = int(round((options.thisChunk - 1) * tests_per_chunk)) - end = int(round(options.thisChunk * tests_per_chunk)) - if end > len(tests): - end = len(tests) - my_tests = tests[start:end] - log.info("Running tests %d-%d/%d" % (start + 1, end, len(tests))) + filters.append( + chunk_by_slice(options.thisChunk, options.totalChunks)) + robocop_tests = mp.active_tests(exists=False, filters=filters, **mozinfo.info) options.extraPrefs.append('browser.search.suggest.enabled=true') options.extraPrefs.append('browser.search.suggest.prompted=true') @@ -835,9 +825,6 @@ def main(args): if options.testPath and options.testPath != test['name']: continue - if not test['name'] in my_tests: - continue - if 'disabled' in test: log.info( 'TEST-INFO | skipping %s | %s' % diff --git a/testing/mochitest/tests/SimpleTest/setup.js b/testing/mochitest/tests/SimpleTest/setup.js index 152cc8db82e7..cb4045584087 100644 --- a/testing/mochitest/tests/SimpleTest/setup.js +++ b/testing/mochitest/tests/SimpleTest/setup.js @@ -160,6 +160,7 @@ var RunSet = {}; RunSet.runall = function(e) { // Filter tests to include|exclude tests based on data in params.filter. // This allows for including or excluding tests from the gTestList + // TODO Only used by ipc tests, remove once those are implemented sanely if (params.testManifest) { getTestManifest("http://mochi.test:8888/" + params.testManifest, params, function(filter) { gTestList = filterTests(filter, gTestList, params.runOnly); RunSet.runtests(); }); } else { @@ -175,10 +176,6 @@ RunSet.runtests = function(e) { my_tests = skipTests(my_tests, params.startAt, params.endAt); } - if (params.totalChunks && params.thisChunk) { - my_tests = chunkifyTests(my_tests, params.totalChunks, params.thisChunk, params.chunkByDir, TestRunner.logger); - } - if (params.shuffle) { for (var i = my_tests.length-1; i > 0; --i) { var j = Math.floor(Math.random() * i); diff --git a/testing/mozbase/manifestparser/manifestparser/filters.py b/testing/mozbase/manifestparser/manifestparser/filters.py index 4912bdb7b8fa..46ef1e7c7717 100644 --- a/testing/mozbase/manifestparser/manifestparser/filters.py +++ b/testing/mozbase/manifestparser/manifestparser/filters.py @@ -9,6 +9,7 @@ possible to define custom filters if the built-in ones are not enough. """ from collections import defaultdict, MutableSequence +import itertools import os from .expression import ( @@ -212,18 +213,29 @@ class chunk_by_dir(InstanceFilter): dirs = dirs[:min(self.depth, len(dirs)-1)] path = os.sep.join(dirs) - if path not in tests_by_dir: + # don't count directories that only have disabled tests in them, + # but still yield disabled tests that are alongside enabled tests + if path not in ordered_dirs and 'disabled' not in test: ordered_dirs.append(path) tests_by_dir[path].append(test) - tests_per_chunk = float(len(tests_by_dir)) / self.total_chunks + tests_per_chunk = float(len(ordered_dirs)) / self.total_chunks start = int(round((self.this_chunk - 1) * tests_per_chunk)) end = int(round(self.this_chunk * tests_per_chunk)) for i in range(start, end): - for test in tests_by_dir[ordered_dirs[i]]: + for test in tests_by_dir.pop(ordered_dirs[i]): yield test + # find directories that only contain disabled tests. They still need to + # be yielded for reporting purposes. Put them all in chunk 1 for + # simplicity. + if self.this_chunk == 1: + disabled_dirs = [v for k, v in tests_by_dir.iteritems() + if k not in ordered_dirs] + for disabled_test in itertools.chain(*disabled_dirs): + yield disabled_test + class chunk_by_runtime(InstanceFilter): """