Bug 1541419 - Split up and reduce test output of the xpcshell searchconfigs tests to improve test times. r=daleharvey
authorMark Banner <standard8@mozilla.com>
Wed, 15 May 2019 05:23:55 +0000
changeset 535789 0e0c4b2166dc3753033cd9121bdd598e355f8ef9
parent 535788 26e0b9e416db3545459b36d7b9631b8f6c25e54d
child 535790 b1406a7e07ff3e12aa20a3b644ff4a86b8045dd5
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdaleharvey
bugs1541419
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
toolkit/components/search/moz.build
toolkit/components/search/tests/xpcshell/searchconfigs/head_chunk1.js
toolkit/components/search/tests/xpcshell/searchconfigs/head_chunk2.js
toolkit/components/search/tests/xpcshell/searchconfigs/head_chunk3.js
toolkit/components/search/tests/xpcshell/searchconfigs/head_chunk4.js
toolkit/components/search/tests/xpcshell/searchconfigs/head_searchconfig.js
toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-1.ini
toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-2.ini
toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-3.ini
toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-4.ini
toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-common.ini
toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell.ini
--- a/toolkit/components/search/moz.build
+++ b/toolkit/components/search/moz.build
@@ -1,16 +1,19 @@
 # -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*-
 # vim: set filetype=python:
 # This Source Code Form is subject to the terms of the Mozilla Public
 # 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/.
 
 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',
 ]
 
 XPIDL_SOURCES += [
     'nsISearchService.idl',
 ]
 
 XPIDL_MODULE = 'toolkit_search'
new file mode 100644
--- /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);
new file mode 100644
--- /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);
new file mode 100644
--- /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);
new file mode 100644
--- /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);
--- a/toolkit/components/search/tests/xpcshell/searchconfigs/head_searchconfig.js
+++ b/toolkit/components/search/tests/xpcshell/searchconfigs/head_searchconfig.js
@@ -80,16 +80,18 @@ class SearchConfigTest {
 
     // Disable region checks.
     Services.prefs.setBoolPref("browser.search.geoSpecificDefaults", false);
     Services.prefs.setCharPref("browser.search.geoip.url", "");
 
     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");
   }
 
   /**
    * Runs the test.
    */
   async run() {
@@ -107,17 +109,17 @@ class SearchConfigTest {
         const engines = await Services.search.getVisibleEngines();
         const isPresent = this._assertAvailableEngines(region, locale, engines);
         if (isPresent) {
           this._assertCorrectDomains(region, locale, engines);
         }
       }
     }
 
-    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");
   }
 
   /**
    * Causes re-initialization of the SearchService with the new region and locale.
    *
    * @param {string} region
    *   The two-letter region code.
    * @param {string} locale
@@ -127,28 +129,32 @@ class SearchConfigTest {
     Services.prefs.setStringPref("browser.search.region", region.toUpperCase());
     const reinitCompletePromise =
       SearchTestUtils.promiseSearchNotification("reinit-complete");
     Services.locale.availableLocales = [locale];
     Services.locale.requestedLocales = [locale];
     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");
   }
 
   /**
    * @returns {Set} the list of regions for the tests to run with.
    */
   get _regions() {
     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);
   }
 
   /**
    * @returns {array} the list of locales for the tests to run with.
    */
   async _getLocales() {
     if (this._testDebug) {
       return ["be", "en-US", "kk", "tr", "ru", "zh-CN"];
@@ -245,35 +251,35 @@ class SearchConfigTest {
     const infoString = `region: "${region}" locale: "${locale}"`;
     const config = this._config[section];
     const hasIncluded = "included" in config;
     const hasExcluded = "excluded" in config;
     const identifierIncluded = !!this._findEngine(engines, this._config.identifier);
 
     // 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;
     }
 
     // If there's no included section, we assume the engine is default everywhere
     // and we should apply the exclusions instead.
     let included = (hasIncluded &&
       this._localeRegionInSection(config.included, region, locale));
 
     let notExcluded = (hasExcluded &&
      !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;
   }
 
   /**
    * Asserts whether the engine is correctly set as default or not.
    *
    * @param {string} region
    *   The two-letter region code.
@@ -311,38 +317,53 @@ class SearchConfigTest {
    * @param {array} engines
    *   The current visible engines.
    */
   _assertCorrectDomains(region, locale, engines) {
     const [expectedDomain, domainConfig] =
       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);
+    }
+  }
 }
rename from toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell.ini
rename to toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-1.ini
--- a/toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell.ini
+++ b/toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-1.ini
@@ -1,44 +1,10 @@
 [DEFAULT]
 firefox-appdir = browser
-head = head_searchconfig.js
-skip-if = toolkit == 'android'
+head = head_searchconfig.js head_chunk1.js
+dupe-manifest =
 support-files =
   ../../../../../../browser/locales/all-locales
-tags=searchconfig
+tags=searchconfig searchconfig1
 
-[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
-[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
-[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
-[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
-[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
-[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
-[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
+[include:xpcshell-common.ini]
+skip-if = toolkit == 'android'
copy from toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell.ini
copy to toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-2.ini
--- a/toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell.ini
+++ b/toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-2.ini
@@ -1,44 +1,10 @@
 [DEFAULT]
 firefox-appdir = browser
-head = head_searchconfig.js
-skip-if = toolkit == 'android'
+head = head_searchconfig.js head_chunk2.js
+dupe-manifest =
 support-files =
   ../../../../../../browser/locales/all-locales
-tags=searchconfig
+tags=searchconfig searchconfig2
 
-[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
-[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
-[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
-[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
-[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
-[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
-[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
+[include:xpcshell-common.ini]
+skip-if = toolkit == 'android'
new file mode 100644
--- /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'
new file mode 100644
--- /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'
copy from toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell.ini
copy to toolkit/components/search/tests/xpcshell/searchconfigs/xpcshell-common.ini
--- 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