From f5799d40342c335aa90e5f36ec09108046258c70 Mon Sep 17 00:00:00 2001 From: Mark Banner Date: Wed, 15 May 2019 05:23:55 +0000 Subject: [PATCH] Bug 1541419 - Split up and reduce test output of the xpcshell searchconfigs tests to improve test times. r=daleharvey This splits running of locales across 4 chunks, which can run in parallel better. The chunks can be run individually with '--tag=searchconfig1' etc. It also stops logging test output in the pass cases (unless we're in _testDebug=true mode). This makes less work on the python harness which was causing a bottleneck and slowing the tests down. Depends on D30399 Differential Revision: https://phabricator.services.mozilla.com/D30898 --HG-- rename : toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell.ini => toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-1.ini rename : toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell.ini => toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-2.ini rename : toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell.ini => toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-common.ini extra : moz-landing-system : lando --- toolkit/components/search/moz.build | 5 +- .../xpcshell/searchconfigs/head_chunk1.js | 8 +++ .../xpcshell/searchconfigs/head_chunk2.js | 8 +++ .../xpcshell/searchconfigs/head_chunk3.js | 8 +++ .../xpcshell/searchconfigs/head_chunk4.js | 8 +++ .../searchconfigs/head_searchconfig.js | 53 +++++++++++++------ .../xpcshell/searchconfigs/xpcshell-1.ini | 10 ++++ .../xpcshell/searchconfigs/xpcshell-2.ini | 10 ++++ .../xpcshell/searchconfigs/xpcshell-3.ini | 10 ++++ .../xpcshell/searchconfigs/xpcshell-4.ini | 10 ++++ .../{xpcshell.ini => xpcshell-common.ini} | 22 +++----- 11 files changed, 120 insertions(+), 32 deletions(-) create mode 100644 toolkit/components/search/tests/xpcshell/searchconfigs/head_chunk1.js create mode 100644 toolkit/components/search/tests/xpcshell/searchconfigs/head_chunk2.js create mode 100644 toolkit/components/search/tests/xpcshell/searchconfigs/head_chunk3.js create mode 100644 toolkit/components/search/tests/xpcshell/searchconfigs/head_chunk4.js create mode 100644 toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-1.ini create mode 100644 toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-2.ini create mode 100644 toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-3.ini create mode 100644 toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-4.ini rename toolkit/components/search/tests/xpcshell/searchconfigs/{xpcshell.ini => xpcshell-common.ini} (78%) diff --git a/toolkit/components/search/moz.build b/toolkit/components/search/moz.build index 457afcf3410e..5fb5c68b0746 100644 --- a/toolkit/components/search/moz.build +++ b/toolkit/components/search/moz.build @@ -5,7 +5,10 @@ # file, You can obtain one at http://mozilla.org/MPL/2.0/. XPCSHELL_TESTS_MANIFESTS += [ - 'tests/xpcshell/searchconfigs/xpcshell.ini', + 'tests/xpcshell/searchconfigs/xpcshell-1.ini', + 'tests/xpcshell/searchconfigs/xpcshell-2.ini', + 'tests/xpcshell/searchconfigs/xpcshell-3.ini', + 'tests/xpcshell/searchconfigs/xpcshell-4.ini', 'tests/xpcshell/xpcshell.ini', ] diff --git a/toolkit/components/search/tests/xpcshell/searchconfigs/head_chunk1.js b/toolkit/components/search/tests/xpcshell/searchconfigs/head_chunk1.js new file mode 100644 index 000000000000..1363e0637211 --- /dev/null +++ b/toolkit/components/search/tests/xpcshell/searchconfigs/head_chunk1.js @@ -0,0 +1,8 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +/* import-globals-from head_searchconfig.js */ + +"use strict"; + +Services.prefs.setIntPref("browser.search.config.test.section", 1); diff --git a/toolkit/components/search/tests/xpcshell/searchconfigs/head_chunk2.js b/toolkit/components/search/tests/xpcshell/searchconfigs/head_chunk2.js new file mode 100644 index 000000000000..9abc11c8e390 --- /dev/null +++ b/toolkit/components/search/tests/xpcshell/searchconfigs/head_chunk2.js @@ -0,0 +1,8 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +/* import-globals-from head_searchconfig.js */ + +"use strict"; + +Services.prefs.setIntPref("browser.search.config.test.section", 2); diff --git a/toolkit/components/search/tests/xpcshell/searchconfigs/head_chunk3.js b/toolkit/components/search/tests/xpcshell/searchconfigs/head_chunk3.js new file mode 100644 index 000000000000..5ef9c9a82ba7 --- /dev/null +++ b/toolkit/components/search/tests/xpcshell/searchconfigs/head_chunk3.js @@ -0,0 +1,8 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +/* import-globals-from head_searchconfig.js */ + +"use strict"; + +Services.prefs.setIntPref("browser.search.config.test.section", 3); diff --git a/toolkit/components/search/tests/xpcshell/searchconfigs/head_chunk4.js b/toolkit/components/search/tests/xpcshell/searchconfigs/head_chunk4.js new file mode 100644 index 000000000000..feada99efb0e --- /dev/null +++ b/toolkit/components/search/tests/xpcshell/searchconfigs/head_chunk4.js @@ -0,0 +1,8 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +/* import-globals-from head_searchconfig.js */ + +"use strict"; + +Services.prefs.setIntPref("browser.search.config.test.section", 4); diff --git a/toolkit/components/search/tests/xpcshell/searchconfigs/head_searchconfig.js b/toolkit/components/search/tests/xpcshell/searchconfigs/head_searchconfig.js index 414d1c564a4e..d4fe282a830e 100644 --- a/toolkit/components/search/tests/xpcshell/searchconfigs/head_searchconfig.js +++ b/toolkit/components/search/tests/xpcshell/searchconfigs/head_searchconfig.js @@ -85,6 +85,8 @@ class SearchConfigTest { await AddonTestUtils.promiseStartupManager(); await Services.search.init(); + // Note: we don't use the helper function here, so that we have at least + // one message output per process. Assert.ok(Services.search.isInitialized, "Should have correctly initialized the search service"); } @@ -112,7 +114,7 @@ class SearchConfigTest { } } - Assert.ok(!this._testDebug, "Should not have test debug turned on in production"); + this.assertOk(!this._testDebug, "Should not have test debug turned on in production"); } /** @@ -132,7 +134,7 @@ class SearchConfigTest { Services.search.reInit(); await reinitCompletePromise; - Assert.ok(Services.search.isInitialized, + this.assertOk(Services.search.isInitialized, "Should have completely re-initialization, if it fails check logs for if reinit was successful"); } @@ -143,7 +145,11 @@ class SearchConfigTest { if (this._testDebug) { return new Set(["by", "cn", "kz", "us", "ru", "tr"]); } - return Services.intl.getAvailableLocaleDisplayNames("region"); + const chunk = Services.prefs.getIntPref("browser.search.config.test.section", -1) - 1; + const regions = Services.intl.getAvailableLocaleDisplayNames("region"); + const chunkSize = Math.ceil(regions.length / 4); + const startPoint = chunk * chunkSize; + return regions.slice(startPoint, startPoint + chunkSize); } /** @@ -250,7 +256,7 @@ class SearchConfigTest { // If there's not included/excluded, then this shouldn't be the default anywhere. if (section == "default" && !hasIncluded && !hasExcluded) { - Assert.ok(!identifierIncluded, + this.assertOk(!identifierIncluded, `Should not be ${section} for any locale/region, currently set for ${infoString}`); return false; @@ -265,10 +271,10 @@ class SearchConfigTest { !this._localeRegionInSection(config.excluded, region, locale)); if (included || notExcluded) { - Assert.ok(identifierIncluded, `Should be ${section} for ${infoString}`); + this.assertOk(identifierIncluded, `Should be ${section} for ${infoString}`); return true; } - Assert.ok(!identifierIncluded, `Should not be ${section} for ${infoString}`); + this.assertOk(!identifierIncluded, `Should not be ${section} for ${infoString}`); return false; } @@ -316,33 +322,48 @@ class SearchConfigTest { Object.entries(this._config.domains).find(([key, value]) => this._localeRegionInSection(value.included, region, locale)); - Assert.ok(expectedDomain, + this.assertOk(expectedDomain, `Should have an expectedDomain for the engine in region: "${region}" locale: "${locale}"`); const engine = this._findEngine(engines, this._config.identifier); - Assert.ok(engine, "Should have an engine present"); + this.assertOk(engine, "Should have an engine present"); const searchForm = new URL(engine.searchForm); - Assert.ok(searchForm.host.endsWith(expectedDomain), + this.assertOk(searchForm.host.endsWith(expectedDomain), `Should have the correct search form domain for region: "${region}" locale: "${locale}". Got "${searchForm.host}", expected to end with "${expectedDomain}".`); for (const urlType of [URLTYPE_SUGGEST_JSON, URLTYPE_SEARCH_HTML]) { - info(`Checking urlType ${urlType}`); - const submission = engine.getSubmission("test", urlType); if (urlType == URLTYPE_SUGGEST_JSON && (this._config.noSuggestionsURL || domainConfig.noSuggestionsURL)) { - Assert.ok(!submission, "Should not have a submission url"); + this.assertOk(!submission, "Should not have a submission url"); } else if (this._config.searchUrlBase) { - Assert.equal(submission.uri.prePath + submission.uri.filePath, + this.assertEqual(submission.uri.prePath + submission.uri.filePath, this._config.searchUrlBase + domainConfig.searchUrlEnd, - `Should have the correct domain for region: "${region}" locale: "${locale}".`); + `Should have the correct domain for type: ${urlType} region: "${region}" locale: "${locale}".`); } else { - Assert.ok(submission.uri.host.endsWith(expectedDomain), - `Should have the correct domain for region: "${region}" locale: "${locale}". + this.assertOk(submission.uri.host.endsWith(expectedDomain), + `Should have the correct domain for type: ${urlType} region: "${region}" locale: "${locale}". Got "${submission.uri.host}", expected to end with "${expectedDomain}".`); } } } + + /** + * Helper functions which avoid outputting test results when there are no + * failures. These help the tests to run faster, and avoid clogging up the + * python test runner process. + */ + assertOk(value, message) { + if (!value || this._testDebug) { + Assert.ok(value, message); + } + } + + assertEqual(actual, expected, message) { + if (actual != expected || this._testDebug) { + Assert.equal(actual, expected, message); + } + } } diff --git a/toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-1.ini b/toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-1.ini new file mode 100644 index 000000000000..a1d68bacde09 --- /dev/null +++ b/toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-1.ini @@ -0,0 +1,10 @@ +[DEFAULT] +firefox-appdir = browser +head = head_searchconfig.js head_chunk1.js +dupe-manifest = +support-files = + ../../../../../../browser/locales/all-locales +tags=searchconfig searchconfig1 + +[include:xpcshell-common.ini] +skip-if = toolkit == 'android' diff --git a/toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-2.ini b/toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-2.ini new file mode 100644 index 000000000000..9d656ead9c05 --- /dev/null +++ b/toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-2.ini @@ -0,0 +1,10 @@ +[DEFAULT] +firefox-appdir = browser +head = head_searchconfig.js head_chunk2.js +dupe-manifest = +support-files = + ../../../../../../browser/locales/all-locales +tags=searchconfig searchconfig2 + +[include:xpcshell-common.ini] +skip-if = toolkit == 'android' diff --git a/toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-3.ini b/toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-3.ini new file mode 100644 index 000000000000..3d670805579d --- /dev/null +++ b/toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-3.ini @@ -0,0 +1,10 @@ +[DEFAULT] +firefox-appdir = browser +head = head_searchconfig.js head_chunk3.js +dupe-manifest = +support-files = + ../../../../../../browser/locales/all-locales +tags=searchconfig searchconfig3 + +[include:xpcshell-common.ini] +skip-if = toolkit == 'android' diff --git a/toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-4.ini b/toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-4.ini new file mode 100644 index 000000000000..f8e82b8d62a5 --- /dev/null +++ b/toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-4.ini @@ -0,0 +1,10 @@ +[DEFAULT] +firefox-appdir = browser +head = head_searchconfig.js head_chunk4.js +dupe-manifest = +support-files = + ../../../../../../browser/locales/all-locales +tags=searchconfig searchconfig4 + +[include:xpcshell-common.ini] +skip-if = toolkit == 'android' diff --git a/toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell.ini b/toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-common.ini similarity index 78% rename from toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell.ini rename to toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-common.ini index f42411873d51..91712534689e 100644 --- a/toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell.ini +++ b/toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-common.ini @@ -1,44 +1,36 @@ -[DEFAULT] -firefox-appdir = browser -head = head_searchconfig.js -skip-if = toolkit == 'android' -support-files = - ../../../../../../browser/locales/all-locales -tags=searchconfig - [test_amazon.js] # This is an extensive test and currently takes a long time. Therefore skip on # debug/asan and extend the timeout length otherwise. skip-if = debug || asan -requesttimeoutfactor = 2 +requesttimeoutfactor = 3 [test_baidu.js] # This is an extensive test and currently takes a long time. Therefore skip on # debug/asan and extend the timeout length otherwise. skip-if = debug || asan -requesttimeoutfactor = 2 +requesttimeoutfactor = 3 [test_bing.js] # This is an extensive test and currently takes a long time. Therefore skip on # debug/asan and extend the timeout length otherwise. skip-if = debug || asan -requesttimeoutfactor = 2 +requesttimeoutfactor = 3 [test_duckduckgo.js] # This is an extensive test and currently takes a long time. Therefore skip on # debug/asan and extend the timeout length otherwise. skip-if = debug || asan -requesttimeoutfactor = 2 +requesttimeoutfactor = 3 [test_ebay.js] # This is an extensive test and currently takes a long time. Therefore skip on # debug/asan and extend the timeout length otherwise. # Thunderbird doesn't provide eBay search engines. skip-if = debug || asan || appname == "thunderbird" -requesttimeoutfactor = 2 +requesttimeoutfactor = 3 [test_google.js] # This is an extensive test and currently takes a long time. Therefore skip on # debug/asan and extend the timeout length otherwise. skip-if = debug || asan -requesttimeoutfactor = 2 +requesttimeoutfactor = 3 [test_yandex.js] # This is an extensive test and currently takes a long time. Therefore skip on # debug/asan and extend the timeout length otherwise. skip-if = debug || asan -requesttimeoutfactor = 2 +requesttimeoutfactor = 3