diff --git a/devtools/server/tests/unit/test_objectgrips-12.js b/devtools/server/tests/unit/test_objectgrips-12.js index e0c81523fb7..307ce54ce28 100644 --- a/devtools/server/tests/unit/test_objectgrips-12.js +++ b/devtools/server/tests/unit/test_objectgrips-12.js @@ -3,6 +3,8 @@ // Test getDisplayString. +Cu.import("resource://testing-common/PromiseTestUtils.jsm", this); + var gDebuggee; var gClient; var gThreadClient; @@ -125,6 +127,7 @@ function test_display_string() output: "Promise (fulfilled: 5)" }, { + // This rejection is left uncaught, see expectUncaughtRejection below. input: "Promise.reject(new Error())", output: "Promise (rejected: Error)" }, @@ -134,6 +137,8 @@ function test_display_string() } ]; + PromiseTestUtils.expectUncaughtRejection(/Error/); + gThreadClient.addOneTimeListener("paused", function(aEvent, aPacket) { const args = aPacket.frame.arguments; diff --git a/devtools/server/tests/unit/test_promises_actor_onpromisesettled.js b/devtools/server/tests/unit/test_promises_actor_onpromisesettled.js index 970e171a696..77012b1da4b 100644 --- a/devtools/server/tests/unit/test_promises_actor_onpromisesettled.js +++ b/devtools/server/tests/unit/test_promises_actor_onpromisesettled.js @@ -8,6 +8,8 @@ "use strict"; +Cu.import("resource://testing-common/PromiseTestUtils.jsm", this); + const { PromisesFront } = require("devtools/server/actors/promises"); var events = require("sdk/event/core"); @@ -52,6 +54,7 @@ function* testPromisesSettled(client, form, makeResolvePromise, let foundResolvedPromise = yield onPromiseSettled; ok(foundResolvedPromise, "Found our resolved promise"); + PromiseTestUtils.expectUncaughtRejection(r => r.message == resolution); onPromiseSettled = oncePromiseSettled(front, resolution, false, true); let rejectedPromise = makeRejectPromise(resolution); let foundRejectedPromise = yield onPromiseSettled; diff --git a/devtools/shared/acorn/tests/unit/head_acorn.js b/devtools/shared/acorn/tests/unit/head_acorn.js index 5cddb4355a9..3e06ca3c1a9 100644 --- a/devtools/shared/acorn/tests/unit/head_acorn.js +++ b/devtools/shared/acorn/tests/unit/head_acorn.js @@ -62,6 +62,11 @@ var listener = { } } + // Ignored until they are fixed in bug 1242968. + if (string.includes("JavaScript Warning")) { + return; + } + do_throw("head_acorn.js got console message: " + string + "\n"); } }; diff --git a/devtools/shared/pretty-fast/tests/unit/head_pretty-fast.js b/devtools/shared/pretty-fast/tests/unit/head_pretty-fast.js index af999f4deff..abde4b197e5 100644 --- a/devtools/shared/pretty-fast/tests/unit/head_pretty-fast.js +++ b/devtools/shared/pretty-fast/tests/unit/head_pretty-fast.js @@ -35,6 +35,11 @@ var listener = { } } + // Ignored until they are fixed in bug 1242968. + if (string.includes("JavaScript Warning")) { + return; + } + do_throw("head_pretty-fast.js got console message: " + string + "\n"); } }; diff --git a/dom/promise/tests/unit/test_monitor_uncaught.js b/dom/promise/tests/unit/test_monitor_uncaught.js index f42bef52596..7dd80d212c1 100644 --- a/dom/promise/tests/unit/test_monitor_uncaught.js +++ b/dom/promise/tests/unit/test_monitor_uncaught.js @@ -7,6 +7,10 @@ var { utils: Cu } = Components; Cu.import("resource://gre/modules/Timer.jsm", this); +Cu.import("resource://testing-common/PromiseTestUtils.jsm", this); + +// Prevent test failures due to the unhandled rejections in this test file. +PromiseTestUtils.disableUncaughtRejectionObserverForSelfTest(); add_task(function* test_globals() { Assert.equal(Promise.defer || undefined, undefined, "We are testing DOM Promise."); diff --git a/dom/push/PushService.jsm b/dom/push/PushService.jsm index 486c7d7be98..ddb7b2491d9 100644 --- a/dom/push/PushService.jsm +++ b/dom/push/PushService.jsm @@ -458,7 +458,7 @@ this.PushService = { // Before completing the activation check prefs. This will first check // connection.enabled pref and then check offline state. this._changeStateConnectionEnabledEvent(prefs.get("connection.enabled")); - }); + }).catch(Cu.reportError); } else { // This is only used for testing. Different tests require connecting to diff --git a/dom/push/PushServiceHttp2.jsm b/dom/push/PushServiceHttp2.jsm index efeedc80c82..7e2048c53f5 100644 --- a/dom/push/PushServiceHttp2.jsm +++ b/dom/push/PushServiceHttp2.jsm @@ -726,12 +726,15 @@ this.PushServiceHttp2 = { .then(record => this._subscribeResource(record) .then(recordNew => { if (this._mainPushService) { - this._mainPushService.updateRegistrationAndNotifyApp(aSubscriptionUri, - recordNew); + this._mainPushService + .updateRegistrationAndNotifyApp(aSubscriptionUri, recordNew) + .catch(Cu.reportError); } }, error => { if (this._mainPushService) { - this._mainPushService.dropRegistrationAndNotifyApp(aSubscriptionUri); + this._mainPushService + .dropRegistrationAndNotifyApp(aSubscriptionUri) + .catch(Cu.reportError); } }) ); diff --git a/dom/push/test/xpcshell/test_registration_success_http2.js b/dom/push/test/xpcshell/test_registration_success_http2.js index e7d3c897f3a..f955ba2688e 100644 --- a/dom/push/test/xpcshell/test_registration_success_http2.js +++ b/dom/push/test/xpcshell/test_registration_success_http2.js @@ -4,6 +4,15 @@ 'use strict'; Cu.import("resource://gre/modules/Services.jsm"); +Cu.import("resource://testing-common/PromiseTestUtils.jsm"); + +/////////////////// +// +// Whitelisting this test. +// As part of bug 1077403, the leaking uncaught rejection should be fixed. +// +// Instances of the rejection "record is undefined" may or may not appear. +PromiseTestUtils.thisTestLeaksUncaughtRejectionsAndShouldBeFixed(); const {PushDB, PushService, PushServiceHttp2} = serviceExports; diff --git a/dom/push/test/xpcshell/test_unregister_success_http2.js b/dom/push/test/xpcshell/test_unregister_success_http2.js index b6c77ca3d6d..c7c63f82933 100644 --- a/dom/push/test/xpcshell/test_unregister_success_http2.js +++ b/dom/push/test/xpcshell/test_unregister_success_http2.js @@ -4,6 +4,15 @@ 'use strict'; Cu.import("resource://gre/modules/Services.jsm"); +Cu.import("resource://testing-common/PromiseTestUtils.jsm"); + +/////////////////// +// +// Whitelisting this test. +// As part of bug 1077403, the leaking uncaught rejection should be fixed. +// +// Instances of the rejection "record is undefined" may or may not appear. +PromiseTestUtils.thisTestLeaksUncaughtRejectionsAndShouldBeFixed(); const {PushDB, PushService, PushServiceHttp2} = serviceExports; diff --git a/testing/xpcshell/head.js b/testing/xpcshell/head.js index da78df981e2..4df2a068d70 100644 --- a/testing/xpcshell/head.js +++ b/testing/xpcshell/head.js @@ -22,6 +22,7 @@ var _profileInitialized = false; _register_modules_protocol_handler(); var _Promise = Components.utils.import("resource://gre/modules/Promise.jsm", {}).Promise; +var _PromiseTestUtils = Components.utils.import("resource://testing-common/PromiseTestUtils.jsm", {}).PromiseTestUtils; // Support a common assertion library, Assert.jsm. var AssertCls = Components.utils.import("resource://testing-common/Assert.jsm", null).Assert; @@ -213,7 +214,6 @@ function _do_main() { function _do_quit() { _testLogger.info("exiting test"); - _Promise.Debugging.flushUncaughtErrors(); _quit = true; } @@ -499,16 +499,8 @@ function _execute_test() { // Call do_get_idle() to restore the factory and get the service. _fakeIdleService.activate(); - _Promise.Debugging.clearUncaughtErrorObservers(); - _Promise.Debugging.addUncaughtErrorObserver(function observer({message, date, fileName, stack, lineNumber}) { - let text = " A promise chain failed to handle a rejection: " + - message + " - rejection date: " + date; - _testLogger.error(text, - { - stack: _format_stack(stack), - source_file: fileName - }); - }); + _PromiseTestUtils.init(); + _PromiseTestUtils.Assert = Assert; // _HEAD_FILES is dynamically defined by . _load_files(_HEAD_FILES); @@ -539,6 +531,7 @@ function _execute_test() { } do_test_finished("MAIN run_test"); _do_main(); + _PromiseTestUtils.assertNoUncaughtRejections(); } catch (e) { _passed = false; // do_check failures are already logged and set _quit to true and throw @@ -587,6 +580,9 @@ function _execute_test() { }); }; + let thr = Components.classes["@mozilla.org/thread-manager;1"] + .getService().currentThread; + let func; while ((func = _cleanupFunctions.pop())) { let result; @@ -602,8 +598,6 @@ function _execute_test() { let complete = false; let promise = result.then(null, reportCleanupError); promise = promise.then(() => complete = true); - let thr = Components.classes["@mozilla.org/thread-manager;1"] - .getService().currentThread; while (!complete) { thr.processNextEvent(true); } @@ -613,8 +607,14 @@ function _execute_test() { // Restore idle service to avoid leaks. _fakeIdleService.deactivate(); - if (!_passed) - return; + try { + _PromiseTestUtils.ensureDOMPromiseRejectionsProcessed(); + _PromiseTestUtils.assertNoUncaughtRejections(); + _PromiseTestUtils.assertNoMoreExpectedRejections(); + } finally { + // It's important to terminate the module to avoid crashes on shutdown. + _PromiseTestUtils.uninit(); + } } /** @@ -1516,8 +1516,8 @@ function run_next_test() function _run_next_test() { if (_gTestIndex < _gTests.length) { - // Flush uncaught errors as early and often as possible. - _Promise.Debugging.flushUncaughtErrors(); + // Check for uncaught rejections as early and often as possible. + _PromiseTestUtils.assertNoUncaughtRejections(); let _properties; [_properties, _gRunningTest,] = _gTests[_gTestIndex++]; if (typeof(_properties.skip_if) == "function" && _properties.skip_if()) { @@ -1538,10 +1538,18 @@ function run_next_test() if (_properties._isTask) { _gTaskRunning = true; - _Task.spawn(_gRunningTest).then( - () => { _gTaskRunning = false; run_next_test(); }, - (ex) => { _gTaskRunning = false; do_report_unexpected_exception(ex); } - ); + _Task.spawn(_gRunningTest).then(() => { + _gTaskRunning = false; + run_next_test(); + }, ex => { + _gTaskRunning = false; + try { + do_report_unexpected_exception(ex); + } catch (ex) { + // The above throws NS_ERROR_ABORT and we don't want this to show up + // as an unhandled rejection later. + } + }); } else { // Exceptions do not kill asynchronous tests, so they'll time out. try { diff --git a/testing/xpcshell/selftest.py b/testing/xpcshell/selftest.py index 854d3268975..e389e4b4713 100644 --- a/testing/xpcshell/selftest.py +++ b/testing/xpcshell/selftest.py @@ -35,6 +35,23 @@ TEST_FAIL_STRING = "TEST-UNEXPECTED-FAIL" SIMPLE_PASSING_TEST = "function run_test() { do_check_true(true); }" SIMPLE_FAILING_TEST = "function run_test() { do_check_true(false); }" +SIMPLE_UNCAUGHT_REJECTION_TEST = ''' +function run_test() { + Promise.reject(new Error("Test rejection.")); + do_check_true(true); +} +''' + +SIMPLE_UNCAUGHT_REJECTION_JSM_TEST = ''' +Components.utils.import("resource://gre/modules/Promise.jsm"); + +Promise.reject(new Error("Test rejection.")); + +function run_test() { + do_check_true(true); +} +''' + ADD_TEST_SIMPLE = ''' function run_test() { run_next_test(); } @@ -53,6 +70,26 @@ add_test(function test_failing() { }); ''' +ADD_TEST_UNCAUGHT_REJECTION = ''' +function run_test() { run_next_test(); } + +add_test(function test_uncaught_rejection() { + Promise.reject(new Error("Test rejection.")); + run_next_test(); +}); +''' + +ADD_TEST_UNCAUGHT_REJECTION_JSM = ''' +Components.utils.import("resource://gre/modules/Promise.jsm"); + +function run_test() { run_next_test(); } + +add_test(function test_uncaught_rejection() { + Promise.reject(new Error("Test rejection.")); + run_next_test(); +}); +''' + CHILD_TEST_PASSING = ''' function run_test () { run_next_test(); } @@ -424,6 +461,7 @@ tail = shuffle=shuffle, verbose=verbose, sequential=True, + testingModulesDir=os.path.join(objdir, '_tests', 'modules'), utility_path=self.utility_path), msg="""Tests should have %s, log: ======== @@ -802,6 +840,30 @@ add_test({ self.assertInLog(TEST_FAIL_STRING) self.assertNotInLog(TEST_PASS_STRING) + def testUncaughtRejection(self): + """ + Ensure a simple test with an uncaught rejection is reported. + """ + self.writeFile("test_simple_uncaught_rejection.js", SIMPLE_UNCAUGHT_REJECTION_TEST) + self.writeManifest(["test_simple_uncaught_rejection.js"]) + + self.assertTestResult(False) + self.assertEquals(1, self.x.testCount) + self.assertEquals(0, self.x.passCount) + self.assertEquals(1, self.x.failCount) + + def testUncaughtRejectionJSM(self): + """ + Ensure a simple test with an uncaught rejection from Promise.jsm is reported. + """ + self.writeFile("test_simple_uncaught_rejection_jsm.js", SIMPLE_UNCAUGHT_REJECTION_JSM_TEST) + self.writeManifest(["test_simple_uncaught_rejection_jsm.js"]) + + self.assertTestResult(False) + self.assertEquals(1, self.x.testCount) + self.assertEquals(0, self.x.passCount) + self.assertEquals(1, self.x.failCount) + def testAddTestSimple(self): """ Ensure simple add_test() works. @@ -839,6 +901,30 @@ add_test({ self.assertEquals(0, self.x.passCount) self.assertEquals(1, self.x.failCount) + def testAddTestUncaughtRejection(self): + """ + Ensure add_test() with an uncaught rejection is reported. + """ + self.writeFile("test_add_test_uncaught_rejection.js", ADD_TEST_UNCAUGHT_REJECTION) + self.writeManifest(["test_add_test_uncaught_rejection.js"]) + + self.assertTestResult(False) + self.assertEquals(1, self.x.testCount) + self.assertEquals(0, self.x.passCount) + self.assertEquals(1, self.x.failCount) + + def testAddTestUncaughtRejectionJSM(self): + """ + Ensure add_test() with an uncaught rejection from Promise.jsm is reported. + """ + self.writeFile("test_add_test_uncaught_rejection_jsm.js", ADD_TEST_UNCAUGHT_REJECTION_JSM) + self.writeManifest(["test_add_test_uncaught_rejection_jsm.js"]) + + self.assertTestResult(False) + self.assertEquals(1, self.x.testCount) + self.assertEquals(0, self.x.passCount) + self.assertEquals(1, self.x.failCount) + def testAddTaskTestSingle(self): """ Ensure add_test_task() with a single passing test works. diff --git a/toolkit/components/jsdownloads/src/DownloadCore.jsm b/toolkit/components/jsdownloads/src/DownloadCore.jsm index 01d5eabe616..7b2a508165a 100644 --- a/toolkit/components/jsdownloads/src/DownloadCore.jsm +++ b/toolkit/components/jsdownloads/src/DownloadCore.jsm @@ -2101,6 +2101,9 @@ this.DownloadCopySaver.prototype = { // In case an error occurs while setting up the chain of objects for // the download, ensure that we release the resources of the saver. backgroundFileSaver.finish(Cr.NS_ERROR_FAILURE); + // Since we're not going to handle deferSaveComplete.promise below, + // we need to make sure that the rejection is handled. + deferSaveComplete.promise.catch(() => {}); throw ex; } diff --git a/toolkit/components/jsdownloads/src/DownloadImport.jsm b/toolkit/components/jsdownloads/src/DownloadImport.jsm index ba85761c0ea..6c175c0239c 100644 --- a/toolkit/components/jsdownloads/src/DownloadImport.jsm +++ b/toolkit/components/jsdownloads/src/DownloadImport.jsm @@ -174,7 +174,7 @@ this.DownloadImport.prototype = { yield this.list.add(download); if (resumeDownload) { - download.start(); + download.start().catch(() => {}); } else { yield download.refresh(); } diff --git a/toolkit/components/jsdownloads/src/DownloadIntegration.jsm b/toolkit/components/jsdownloads/src/DownloadIntegration.jsm index aeceb5f2b76..839035b9d44 100644 --- a/toolkit/components/jsdownloads/src/DownloadIntegration.jsm +++ b/toolkit/components/jsdownloads/src/DownloadIntegration.jsm @@ -1080,7 +1080,7 @@ this.DownloadObserver = { this._wakeTimer = null; for (let download of this._canceledOfflineDownloads) { - download.start(); + download.start().catch(() => {}); } }, diff --git a/toolkit/components/jsdownloads/src/DownloadLegacy.js b/toolkit/components/jsdownloads/src/DownloadLegacy.js index 48218d87ad9..c4b63808551 100644 --- a/toolkit/components/jsdownloads/src/DownloadLegacy.js +++ b/toolkit/components/jsdownloads/src/DownloadLegacy.js @@ -243,7 +243,7 @@ DownloadLegacyTransfer.prototype = { } // Start the download before allowing it to be controlled. Ignore errors. - aDownload.start().then(null, () => {}); + aDownload.start().catch(() => {}); // Start processing all the other events received through nsITransfer. this._deferDownload.resolve(aDownload); diff --git a/toolkit/components/jsdownloads/src/DownloadStore.jsm b/toolkit/components/jsdownloads/src/DownloadStore.jsm index 8ad7e720d63..83bc2824fbb 100644 --- a/toolkit/components/jsdownloads/src/DownloadStore.jsm +++ b/toolkit/components/jsdownloads/src/DownloadStore.jsm @@ -124,8 +124,8 @@ this.DownloadStore.prototype = { try { if (!download.succeeded && !download.canceled && !download.error) { // Try to restart the download if it was in progress during the - // previous session. - download.start(); + // previous session. Ignore errors. + download.start().catch(() => {}); } else { // If the download was not in progress, try to update the current // progress from disk. This is relevant in case we retained diff --git a/toolkit/components/jsdownloads/test/browser/browser_DownloadPDFSaver.js b/toolkit/components/jsdownloads/test/browser/browser_DownloadPDFSaver.js index 63e1174656b..e08e500f78c 100644 --- a/toolkit/components/jsdownloads/test/browser/browser_DownloadPDFSaver.js +++ b/toolkit/components/jsdownloads/test/browser/browser_DownloadPDFSaver.js @@ -88,7 +88,7 @@ add_task(function* test_cancel_pdf_download() { }); yield test_download_windowRef(tab, download); - download.start(); + download.start().catch(() => {}); // Immediately cancel the download to test that it is erased correctly. yield download.cancel(); diff --git a/toolkit/components/jsdownloads/test/unit/common_test_Download.js b/toolkit/components/jsdownloads/test/unit/common_test_Download.js index e353452f7d6..8bedc32a627 100644 --- a/toolkit/components/jsdownloads/test/unit/common_test_Download.js +++ b/toolkit/components/jsdownloads/test/unit/common_test_Download.js @@ -32,7 +32,7 @@ function promiseStartDownload(aSourceUrl) { } return promiseNewDownload(aSourceUrl).then(download => { - download.start(); + download.start().catch(() => {}); return download; }); } @@ -64,7 +64,7 @@ function promiseStartDownload_tryToKeepPartialData() { partFilePath: targetFilePath + ".part" }, }); download.tryToKeepPartialData = true; - download.start(); + download.start().catch(() => {}); } else { // Start a download using nsIExternalHelperAppService, that is configured // to keep partially downloaded data by default. @@ -435,7 +435,7 @@ add_task(function* test_empty_progress_tryToKeepPartialData() partFilePath: targetFilePath + ".part" }, }); download.tryToKeepPartialData = true; - download.start(); + download.start().catch(() => {}); } else { // Start a download using nsIExternalHelperAppService, that is configured // to keep partially downloaded data by default. @@ -491,7 +491,7 @@ add_task(function* test_empty_noprogress() } }; - download.start(); + download.start().catch(() => {}); } else { // When testing DownloadLegacySaver, the download is already started when it // is created, and it may have already made all needed property change @@ -856,7 +856,7 @@ add_task(function* test_cancel_midway_restart_tryToKeepPartialData_false() // Restart the download from the beginning. mustInterruptResponses(); - download.start(); + download.start().catch(() => {}); yield promiseDownloadMidway(download); yield promisePartFileReady(download); @@ -1143,7 +1143,7 @@ add_task(function* test_whenSucceeded_after_restart() // we can verify getting a reference before the first download attempt. download = yield promiseNewDownload(httpUrl("interruptible.txt")); promiseSucceeded = download.whenSucceeded(); - download.start(); + download.start().catch(() => {}); } else { // When testing DownloadLegacySaver, the download is already started when it // is created, thus we cannot get the reference before the first attempt. @@ -1156,7 +1156,7 @@ add_task(function* test_whenSucceeded_after_restart() // The second request is allowed to complete. continueResponses(); - download.start(); + download.start().catch(() => {}); // Wait for the download to finish by waiting on the whenSucceeded promise. yield promiseSucceeded; @@ -1343,7 +1343,7 @@ add_task(function* test_error_restart() source: httpUrl("source.txt"), target: targetFile, }); - download.start(); + download.start().catch(() => {}); } else { download = yield promiseStartLegacyDownload(null, { targetFile: targetFile }); @@ -2186,7 +2186,7 @@ add_task(function* test_platform_integration() source: httpUrl("source.txt"), target: targetFile, }); - download.start(); + download.start().catch(() => {}); } // Wait for the whenSucceeded promise to be resolved first. diff --git a/toolkit/components/jsdownloads/test/unit/test_DownloadIntegration.js b/toolkit/components/jsdownloads/test/unit/test_DownloadIntegration.js index 316407a81ee..9839cd9da9f 100644 --- a/toolkit/components/jsdownloads/test/unit/test_DownloadIntegration.js +++ b/toolkit/components/jsdownloads/test/unit/test_DownloadIntegration.js @@ -215,7 +215,7 @@ add_task(function* test_notifications() let download3 = yield promiseNewDownload(httpUrl("interruptible.txt")); let promiseAttempt1 = download1.start(); let promiseAttempt2 = download2.start(); - download3.start(); + download3.start().catch(() => {}); // Add downloads to list. yield list.add(download1); @@ -250,8 +250,8 @@ add_task(function* test_no_notifications() let list = yield promiseNewList(isPrivate); let download1 = yield promiseNewDownload(httpUrl("interruptible.txt")); let download2 = yield promiseNewDownload(httpUrl("interruptible.txt")); - download1.start(); - download2.start(); + download1.start().catch(() => {}); + download2.start().catch(() => {}); // Add downloads to list. yield list.add(download1); @@ -316,7 +316,7 @@ add_task(function* test_suspend_resume() { return Task.spawn(function* () { let download = yield promiseNewDownload(httpUrl("interruptible.txt")); - download.start(); + download.start().catch(() => {}); list.add(download); return download; }); diff --git a/toolkit/components/jsdownloads/test/unit/test_DownloadList.js b/toolkit/components/jsdownloads/test/unit/test_DownloadList.js index dfb97b6f45b..93650169124 100644 --- a/toolkit/components/jsdownloads/test/unit/test_DownloadList.js +++ b/toolkit/components/jsdownloads/test/unit/test_DownloadList.js @@ -348,7 +348,7 @@ add_task(function* test_history_expiration() // Work with one finished download and one canceled download. yield downloadOne.start(); - downloadTwo.start(); + downloadTwo.start().catch(() => {}); yield downloadTwo.cancel(); // We must replace the visits added while executing the downloads with visits @@ -471,7 +471,7 @@ add_task(function* test_DownloadSummary() // Add a public download that has been canceled midway. let canceledPublicDownload = yield promiseNewDownload(httpUrl("interruptible.txt")); - canceledPublicDownload.start(); + canceledPublicDownload.start().catch(() => {}); yield promiseDownloadMidway(canceledPublicDownload); yield canceledPublicDownload.cancel(); yield publicList.add(canceledPublicDownload); @@ -479,7 +479,7 @@ add_task(function* test_DownloadSummary() // Add a public download that is in progress. let inProgressPublicDownload = yield promiseNewDownload(httpUrl("interruptible.txt")); - inProgressPublicDownload.start(); + inProgressPublicDownload.start().catch(() => {}); yield promiseDownloadMidway(inProgressPublicDownload); yield publicList.add(inProgressPublicDownload); @@ -488,7 +488,7 @@ add_task(function* test_DownloadSummary() source: { url: httpUrl("interruptible.txt"), isPrivate: true }, target: getTempFile(TEST_TARGET_FILE_NAME).path, }); - inProgressPrivateDownload.start(); + inProgressPrivateDownload.start().catch(() => {}); yield promiseDownloadMidway(inProgressPrivateDownload); yield privateList.add(inProgressPrivateDownload); diff --git a/toolkit/components/search/nsSearchService.js b/toolkit/components/search/nsSearchService.js index 0b50401b4db..3e10ac69e59 100644 --- a/toolkit/components/search/nsSearchService.js +++ b/toolkit/components/search/nsSearchService.js @@ -4453,6 +4453,13 @@ SearchService.prototype = { }, _addObservers: function SRCH_SVC_addObservers() { + if (this._observersAdded) { + // There might be a race between synchronous and asynchronous + // initialization for which we try to register the observers twice. + return; + } + this._observersAdded = true; + Services.obs.addObserver(this, SEARCH_ENGINE_TOPIC, false); Services.obs.addObserver(this, QUIT_APPLICATION_TOPIC, false); @@ -4495,6 +4502,7 @@ SearchService.prototype = { () => shutdownState ); }, + _observersAdded: false, _removeObservers: function SRCH_SVC_removeObservers() { Services.obs.removeObserver(this, SEARCH_ENGINE_TOPIC); diff --git a/toolkit/components/search/tests/xpcshell/test_hidden.js b/toolkit/components/search/tests/xpcshell/test_hidden.js index f49344cd8f7..0079d05a1a8 100644 --- a/toolkit/components/search/tests/xpcshell/test_hidden.js +++ b/toolkit/components/search/tests/xpcshell/test_hidden.js @@ -47,10 +47,6 @@ add_task(function* async_init() { add_task(function* sync_init() { let reInitPromise = asyncReInit(); // Synchronously check the current default engine, to force a sync init. - // XXX For some reason forcing a sync init while already asynchronously - // reinitializing causes a shutdown warning related to engineMetadataService's - // finalize method having already been called. Seems harmless for the purpose - // of this test. do_check_false(Services.search.isInitialized); do_check_eq(Services.search.currentEngine.name, "hidden"); do_check_true(Services.search.isInitialized); diff --git a/toolkit/modules/moz.build b/toolkit/modules/moz.build index 2683b2bc6b7..27dffb4603b 100644 --- a/toolkit/modules/moz.build +++ b/toolkit/modules/moz.build @@ -9,6 +9,10 @@ BROWSER_CHROME_MANIFESTS += ['tests/browser/browser.ini'] MOCHITEST_MANIFESTS += ['tests/mochitest/mochitest.ini'] MOCHITEST_CHROME_MANIFESTS += ['tests/chrome/chrome.ini'] +TESTING_JS_MODULES += [ + 'tests/PromiseTestUtils.jsm', +] + SPHINX_TREES['toolkit_modules'] = 'docs' EXTRA_JS_MODULES += [ diff --git a/toolkit/modules/tests/PromiseTestUtils.jsm b/toolkit/modules/tests/PromiseTestUtils.jsm new file mode 100644 index 00000000000..d60b785a580 --- /dev/null +++ b/toolkit/modules/tests/PromiseTestUtils.jsm @@ -0,0 +1,241 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +/* + * Detects and reports unhandled rejections during test runs. Test harnesses + * will fail tests in this case, unless the test whitelists itself. + */ + +"use strict"; + +this.EXPORTED_SYMBOLS = [ + "PromiseTestUtils", +]; + +const { classes: Cc, interfaces: Ci, utils: Cu, results: Cr } = Components; + +Cu.import("resource://gre/modules/Services.jsm", this); + +// Keep "JSMPromise" separate so "Promise" still refers to DOM Promises. +let JSMPromise = Cu.import("resource://gre/modules/Promise.jsm", {}).Promise; + +// For now, we need test harnesses to provide a reference to Assert.jsm. +let Assert = null; + +this.PromiseTestUtils = { + /** + * Array of objects containing the details of the Promise rejections that are + * currently left uncaught. This includes DOM Promise and Promise.jsm. When + * rejections in DOM Promises are consumed, they are removed from this list. + * + * The objects contain at least the following properties: + * { + * message: The error message associated with the rejection, if any. + * date: Date object indicating when the rejection was observed. + * id: For DOM Promise only, the Promise ID from PromiseDebugging. This is + * only used for tracking and should not be checked by the callers. + * stack: nsIStackFrame, SavedFrame, or string indicating the stack at the + * time the rejection was triggered. May also be null if the + * rejection was triggered while a script was on the stack. + * } + */ + _rejections: [], + + /** + * When an uncaught rejection is detected, it is ignored if one of the + * functions in this array returns true when called with the rejection details + * as its only argument. When a function matches an expected rejection, it is + * then removed from the array. + */ + _rejectionIgnoreFns: [], + + /** + * Called only by the test infrastructure, registers the rejection observers. + * + * This should be called only once, and a matching "uninit" call must be made + * or the tests will crash on shutdown. + */ + init() { + if (this._initialized) { + Cu.reportError("This object was already initialized."); + return; + } + + PromiseDebugging.addUncaughtRejectionObserver(this); + + // Promise.jsm rejections are only reported to this observer when requested, + // so we don't have to store a key to remove them when consumed. + JSMPromise.Debugging.addUncaughtErrorObserver( + rejection => this._rejections.push(rejection)); + + this._initialized = true; + }, + _initialized: false, + + /** + * Called only by the test infrastructure, unregisters the observers. + */ + uninit() { + if (!this._initialized) { + return; + } + + PromiseDebugging.removeUncaughtRejectionObserver(this); + JSMPromise.Debugging.clearUncaughtErrorObservers(); + + this._initialized = false; + }, + + /** + * Called only by the test infrastructure, spins the event loop until the + * messages for pending DOM Promise rejections have been processed. + */ + ensureDOMPromiseRejectionsProcessed() { + let observed = false; + let observer = { + onLeftUncaught: promise => { + if (PromiseDebugging.getState(promise).reason === + this._ensureDOMPromiseRejectionsProcessedReason) { + observed = true; + } + }, + onConsumed() {}, + }; + + PromiseDebugging.addUncaughtRejectionObserver(observer); + Promise.reject(this._ensureDOMPromiseRejectionsProcessedReason); + while (!observed) { + Services.tm.mainThread.processNextEvent(true); + } + PromiseDebugging.removeUncaughtRejectionObserver(observer); + }, + _ensureDOMPromiseRejectionsProcessedReason: {}, + + /** + * Called only by the tests for PromiseDebugging.addUncaughtRejectionObserver + * and for JSMPromise.Debugging, disables the observers in this module. + */ + disableUncaughtRejectionObserverForSelfTest() { + this.uninit(); + }, + + /** + * Called by tests that have been whitelisted, disables the observers in this + * module. For new tests where uncaught rejections are expected, you should + * use the more granular expectUncaughtRejection function instead. + */ + thisTestLeaksUncaughtRejectionsAndShouldBeFixed() { + this.uninit(); + }, + + /** + * Sets or updates the Assert object instance to be used for error reporting. + */ + set Assert(assert) { + Assert = assert; + }, + + // UncaughtRejectionObserver + onLeftUncaught(promise) { + let message = "(Unable to convert rejection reason to string.)"; + try { + let reason = PromiseDebugging.getState(promise).reason; + if (reason === this._ensureDOMPromiseRejectionsProcessedReason) { + // Ignore the special promise for ensureDOMPromiseRejectionsProcessed. + return; + } + message = reason.message || ("" + reason); + } catch (ex) {} + + // It's important that we don't store any reference to the provided Promise + // object or its value after this function returns in order to avoid leaks. + this._rejections.push({ + id: PromiseDebugging.getPromiseID(promise), + message, + date: new Date(), + stack: PromiseDebugging.getRejectionStack(promise), + }); + }, + + // UncaughtRejectionObserver + onConsumed(promise) { + // We don't expect that many unhandled rejections will appear at the same + // time, so the algorithm doesn't need to be optimized for that case. + let id = PromiseDebugging.getPromiseID(promise); + let index = this._rejections.findIndex(rejection => rejection.id == id); + // If we get a consumption notification for a rejection that was left + // uncaught before this module was initialized, we can safely ignore it. + if (index != -1) { + this._rejections.splice(index, 1); + } + }, + + /** + * Informs the test suite that the test code will generate a Promise rejection + * that will still be unhandled when the test file terminates. + * + * This method must be called once for each instance of Promise that is + * expected to be uncaught, even if the rejection reason is the same for each + * instance. + * + * If the expected rejection does not occur, the test will fail. + * + * @param regExpOrCheckFn + * This can either be a regular expression that should match the error + * message of the rejection, or a check function that is invoked with + * the rejection details object as its first argument. + */ + expectUncaughtRejection(regExpOrCheckFn) { + let checkFn = !("test" in regExpOrCheckFn) ? regExpOrCheckFn : + rejection => regExpOrCheckFn.test(rejection.message); + this._rejectionIgnoreFns.push(checkFn); + }, + + /** + * Fails the test if there are any uncaught rejections at this time that have + * not been whitelisted using expectUncaughtRejection. + * + * Depending on the configuration of the test suite, this function might only + * report the details of the first uncaught rejection that was generated. + * + * This is called by the test suite at the end of each test function. + */ + assertNoUncaughtRejections() { + // Ask Promise.jsm to report all uncaught rejections to the observer now. + JSMPromise.Debugging.flushUncaughtErrors(); + + // If there is any uncaught rejection left at this point, the test fails. + while (this._rejections.length > 0) { + let rejection = this._rejections.shift(); + + // If one of the ignore functions matches, ignore the rejection, then + // remove the function so that each function only matches one rejection. + let index = this._rejectionIgnoreFns.findIndex(f => f(rejection)); + if (index != -1) { + this._rejectionIgnoreFns.splice(index, 1); + continue; + } + + // Report the error. This operation can throw an exception, depending on + // the configuration of the test suite that handles the assertion. + Assert.ok(false, + `A promise chain failed to handle a rejection:` + + ` ${rejection.message} - rejection date: ${rejection.date}`+ + ` - stack: ${rejection.stack}`); + } + }, + + /** + * Fails the test if any rejection indicated by expectUncaughtRejection has + * not yet been reported at this time. + * + * This is called by the test suite at the end of each test file. + */ + assertNoMoreExpectedRejections() { + // Only log this condition is there is a failure. + if (this._rejectionIgnoreFns.length > 0) { + Assert.equal(this._rejectionIgnoreFns.length, 0, + "Unable to find a rejection expected by expectUncaughtRejection."); + } + }, +}; diff --git a/toolkit/modules/tests/xpcshell/test_ObjectUtils_strict.js b/toolkit/modules/tests/xpcshell/test_ObjectUtils_strict.js index b3a8863cda9..44572e6001a 100644 --- a/toolkit/modules/tests/xpcshell/test_ObjectUtils_strict.js +++ b/toolkit/modules/tests/xpcshell/test_ObjectUtils_strict.js @@ -1,12 +1,7 @@ "use strict"; var {ObjectUtils} = Components.utils.import("resource://gre/modules/ObjectUtils.jsm", {}); -var {Promise} = Components.utils.import("resource://gre/modules/Promise.jsm", {}); - -add_task(function* init() { - // The code will cause uncaught rejections on purpose. - Promise.Debugging.clearUncaughtErrorObservers(); -}); +var {PromiseTestUtils} = Components.utils.import("resource://testing-common/PromiseTestUtils.jsm", {}); add_task(function* test_strict() { let loose = { a: 1 }; @@ -16,11 +11,13 @@ add_task(function* test_strict() { loose.b || undefined; // Should not throw. strict.a; // Should not throw. + PromiseTestUtils.expectUncaughtRejection(/No such property: "b"/); Assert.throws(() => strict.b, /No such property: "b"/); "b" in strict; // Should not throw. strict.b = 2; strict.b; // Should not throw. + PromiseTestUtils.expectUncaughtRejection(/No such property: "c"/); Assert.throws(() => strict.c, /No such property: "c"/); "c" in strict; // Should not throw. loose.c = 3; diff --git a/toolkit/modules/tests/xpcshell/test_Promise.js b/toolkit/modules/tests/xpcshell/test_Promise.js index ab2f3b23384..6f39bb0fd2e 100644 --- a/toolkit/modules/tests/xpcshell/test_Promise.js +++ b/toolkit/modules/tests/xpcshell/test_Promise.js @@ -5,10 +5,10 @@ Components.utils.import("resource://gre/modules/Promise.jsm"); Components.utils.import("resource://gre/modules/Services.jsm"); Components.utils.import("resource://gre/modules/Task.jsm"); +Components.utils.import("resource://testing-common/PromiseTestUtils.jsm"); -// Deactivate the standard xpcshell observer, as it turns uncaught -// rejections into failures, which we don't want here. -Promise.Debugging.clearUncaughtErrorObservers(); +// Prevent test failures due to the unhandled rejections in this test file. +PromiseTestUtils.disableUncaughtRejectionObserverForSelfTest(); //////////////////////////////////////////////////////////////////////////////// //// Test runner diff --git a/toolkit/modules/tests/xpcshell/test_PromiseUtils.js b/toolkit/modules/tests/xpcshell/test_PromiseUtils.js index ff9ba3688b5..5f1ac8b7d0b 100644 --- a/toolkit/modules/tests/xpcshell/test_PromiseUtils.js +++ b/toolkit/modules/tests/xpcshell/test_PromiseUtils.js @@ -5,6 +5,7 @@ Components.utils.import("resource://gre/modules/PromiseUtils.jsm"); Components.utils.import("resource://gre/modules/Timer.jsm"); +Components.utils.import("resource://testing-common/PromiseTestUtils.jsm"); // Tests for PromiseUtils.jsm function run_test() { @@ -98,8 +99,9 @@ add_task(function* test_reject_resolved_promise() { /* Test for the case when a rejected Promise is * passed to the reject method */ add_task(function* test_reject_resolved_promise() { + PromiseTestUtils.expectUncaughtRejection(/This one rejects/); let def = PromiseUtils.defer(); - let p = new Promise((resolve, reject) => reject(new Error("This on rejects"))); + let p = new Promise((resolve, reject) => reject(new Error("This one rejects"))); def.reject(p); yield Assert.rejects(def.promise, Promise, "Rejection with a rejected promise uses the passed promise itself as the reason of rejection"); }); diff --git a/toolkit/mozapps/extensions/test/xpcshell/test_update_webextensions.js b/toolkit/mozapps/extensions/test/xpcshell/test_update_webextensions.js index 5da4ecc6174..c0d4940b3bd 100644 --- a/toolkit/mozapps/extensions/test/xpcshell/test_update_webextensions.js +++ b/toolkit/mozapps/extensions/test/xpcshell/test_update_webextensions.js @@ -1,5 +1,17 @@ "use strict"; +Components.utils.import("resource://testing-common/PromiseTestUtils.jsm", this); + +/////////////////// +// +// Whitelisting this test. +// As part of bug 1077403, the leaking uncaught rejection should be fixed. +// +// thisTestLeaksUncaughtRejectionsAndShouldBeFixed +if (!TEST_UNPACKED) { + PromiseTestUtils.expectUncaughtRejection(/NS_ERROR_FILE_NOT_FOUND/); +} + const TOOLKIT_ID = "toolkit@mozilla.org"; // We don't have an easy way to serve update manifests from a secure URL. diff --git a/toolkit/mozapps/extensions/test/xpcshell/test_webextension.js b/toolkit/mozapps/extensions/test/xpcshell/test_webextension.js index 28ed1177c60..c290486e2e1 100644 --- a/toolkit/mozapps/extensions/test/xpcshell/test_webextension.js +++ b/toolkit/mozapps/extensions/test/xpcshell/test_webextension.js @@ -2,6 +2,21 @@ * http://creativecommons.org/publicdomain/zero/1.0/ */ +Components.utils.import("resource://testing-common/PromiseTestUtils.jsm", this); + +/////////////////// +// +// Whitelisting this test. +// As part of bug 1077403, the leaking uncaught rejection should be fixed. +// +// thisTestLeaksUncaughtRejectionsAndShouldBeFixed +if (TEST_UNPACKED) { + let codeAsString = ("" + Components.results.NS_ERROR_FILE_NOT_FOUND); + PromiseTestUtils.expectUncaughtRejection(r => r.message == codeAsString); +} else { + PromiseTestUtils.expectUncaughtRejection(/Failed to open input source/); +} + const ID = "webextension1@tests.mozilla.org"; const PREF_SELECTED_LOCALE = "general.useragent.locale"; diff --git a/toolkit/mozapps/extensions/test/xpcshell/test_webextension_icons.js b/toolkit/mozapps/extensions/test/xpcshell/test_webextension_icons.js index 6f113b6c859..37f51b09f77 100644 --- a/toolkit/mozapps/extensions/test/xpcshell/test_webextension_icons.js +++ b/toolkit/mozapps/extensions/test/xpcshell/test_webextension_icons.js @@ -2,6 +2,23 @@ * http://creativecommons.org/publicdomain/zero/1.0/ */ +Components.utils.import("resource://testing-common/PromiseTestUtils.jsm", this); + +/////////////////// +// +// Whitelisting this test. +// As part of bug 1077403, the leaking uncaught rejection should be fixed. +// +// thisTestLeaksUncaughtRejectionsAndShouldBeFixed +for (let i = 0; i < 4; i++) { + if (TEST_UNPACKED) { + let codeAsString = ("" + Components.results.NS_ERROR_FILE_NOT_FOUND); + PromiseTestUtils.expectUncaughtRejection(r => r.message == codeAsString); + } else { + PromiseTestUtils.expectUncaughtRejection(/Failed to open input source/); + } +} + const ID = "webextension1@tests.mozilla.org"; const profileDir = gProfD.clone();