commit 41c62af8d756dd9748a8343edada689e41f3922d
parent 78fe31c4173265f42a2dc00cfa085f20a6c7203e
Author: James Teow <jteow@mozilla.com>
Date: Mon, 15 Dec 2025 15:10:07 +0000
Bug 1696279 - Improve SearchObserver test utility - r=search-reviewers,scunnane
Differential Revision: https://phabricator.services.mozilla.com/D276306
Diffstat:
3 files changed, 109 insertions(+), 137 deletions(-)
diff --git a/toolkit/components/search/tests/xpcshell/head_search.js b/toolkit/components/search/tests/xpcshell/head_search.js
@@ -504,11 +504,19 @@ async function enableEnterprise() {
* A simple observer to ensure we get only the expected notifications.
*/
class SearchObserver {
- constructor(expectedNotifications, returnEngineForNotification = false) {
+ /**
+ *
+ * @param {Array<[string, string|null]>} expectedNotifications
+ * An array of [notificationType, engineName] tuples. Use null for
+ * engineName if we don't care which engine triggered the notification.
+ */
+ constructor(expectedNotifications) {
this.observer = this.observer.bind(this);
this.deferred = Promise.withResolvers();
- this.expectedNotifications = expectedNotifications;
- this.returnEngineForNotification = returnEngineForNotification;
+ this.expectedNotifications = expectedNotifications.map(([type, name]) => {
+ return { type, name };
+ });
+ this.receivedNotifications = [];
Services.obs.addObserver(this.observer, SearchUtils.TOPIC_ENGINE_MODIFIED);
@@ -520,10 +528,18 @@ class SearchObserver {
}
handleTimeout() {
+ let stillExpecting = this.expectedNotifications
+ .map(n => `[type: ${n.type}, name: ${n.name}]`)
+ .join(", ");
+ let received = this.receivedNotifications
+ .map(n => `[type: ${n.type}, name: ${n.engineName}]`)
+ .join(", ");
+
this.deferred.reject(
new Error(
- "Waiting for Notifications timed out, only received: " +
- this.expectedNotifications.join(",")
+ `Waiting for Notifications timed out:
+ still expecting - [${stillExpecting || "(none)"}]
+ received - [${received || "(none);"}]`
)
);
}
@@ -534,20 +550,25 @@ class SearchObserver {
0,
"Should be expecting a notification"
);
- Assert.equal(
- data,
- this.expectedNotifications[0],
- "Should have received the next expected notification"
+
+ let maybeEngine = subject.QueryInterface(Ci.nsISearchEngine);
+ let engineName = maybeEngine?.name ?? null;
+ this.receivedNotifications.push({ type: data, engineName });
+
+ let matchIndex = this.expectedNotifications.findIndex(
+ expected =>
+ expected.type == data &&
+ (expected.name == null || expected.name == engineName)
);
- if (
- this.returnEngineForNotification &&
- data == this.returnEngineForNotification
- ) {
- this.engineToReturn = subject.QueryInterface(Ci.nsISearchEngine);
+ if (matchIndex == -1) {
+ info(
+ `SearchObserver received unexpected notification: ${data}, ${engineName}`
+ );
+ return;
}
- this.expectedNotifications.shift();
+ this.expectedNotifications.splice(matchIndex, 1);
if (!this.expectedNotifications.length) {
clearTimeout(this.timeout);
diff --git a/toolkit/components/search/tests/xpcshell/test_defaultEngine_fallback.js b/toolkit/components/search/tests/xpcshell/test_defaultEngine_fallback.js
@@ -80,19 +80,19 @@ async function checkFallbackDefaultRegion(checkPrivate) {
"Sanity check engines are different"
);
- const observer = new SearchObserver(
+ const observer = new SearchObserver([
+ [expectedDefaultNotification, defaultEngine.name],
[
- expectedDefaultNotification,
// For hiding (removing) the engine.
SearchUtils.MODIFIED_TYPE.CHANGED,
- SearchUtils.MODIFIED_TYPE.REMOVED,
+ otherEngine.name,
],
- expectedDefaultNotification
- );
+ [SearchUtils.MODIFIED_TYPE.REMOVED, otherEngine.name],
+ ]);
await Services.search.removeEngine(otherEngine);
- let notified = await observer.promise;
+ await observer.promise;
Assert.ok(otherEngine.hidden, "Should have hidden the removed engine");
Assert.equal(
@@ -100,11 +100,6 @@ async function checkFallbackDefaultRegion(checkPrivate) {
defaultEngine.name,
"Should have reverted the defaultEngine to the region default"
);
- Assert.equal(
- notified.name,
- defaultEngine.name,
- "Should have notified the correct default engine"
- );
}
add_task(async function test_default_fallback_to_region_default() {
@@ -132,37 +127,21 @@ async function checkFallbackFirstVisible(checkPrivate) {
"Sanity check engines are different"
);
- const observer = new SearchObserver(
- checkPrivate
- ? [
- expectedDefaultNotification,
- // For hiding (removing) the engine.
- SearchUtils.MODIFIED_TYPE.CHANGED,
- SearchUtils.MODIFIED_TYPE.REMOVED,
- ]
- : [
- expectedDefaultNotification,
- // For hiding (removing) the engine.
- SearchUtils.MODIFIED_TYPE.CHANGED,
- SearchUtils.MODIFIED_TYPE.REMOVED,
- ],
- expectedDefaultNotification
- );
+ const observer = new SearchObserver([
+ [expectedDefaultNotification, "generalEngine"],
+ [SearchUtils.MODIFIED_TYPE.CHANGED, otherEngine.name],
+ [SearchUtils.MODIFIED_TYPE.REMOVED, otherEngine.name],
+ ]);
await Services.search.removeEngine(otherEngine);
- let notified = await observer.promise;
+ await observer.promise;
Assert.equal(
(await getDefault(checkPrivate)).name,
"generalEngine",
"Should have set the default engine to the first visible general engine"
);
- Assert.equal(
- notified.name,
- "generalEngine",
- "Should have notified the correct default general engine"
- );
}
add_task(async function test_default_fallback_to_first_gen_visible() {
@@ -187,26 +166,25 @@ add_task(async function test_default_fallback_when_no_others_visible() {
"Should only have one visible engine"
);
- const observer = new SearchObserver(
- [
- // Unhiding of the default engine.
- SearchUtils.MODIFIED_TYPE.CHANGED,
- // Change of the default.
- SearchUtils.MODIFIED_TYPE.DEFAULT,
- // Unhiding of the default private.
- SearchUtils.MODIFIED_TYPE.CHANGED,
- SearchUtils.MODIFIED_TYPE.DEFAULT_PRIVATE,
- // Hiding the engine.
- SearchUtils.MODIFIED_TYPE.CHANGED,
- SearchUtils.MODIFIED_TYPE.REMOVED,
- ],
- SearchUtils.MODIFIED_TYPE.DEFAULT
- );
+ let lastEngine = visibleEngines.at(-1);
+
+ const observer = new SearchObserver([
+ // Unhiding of the default engine.
+ [SearchUtils.MODIFIED_TYPE.CHANGED, appDefault.name],
+ // Change of the default.
+ [SearchUtils.MODIFIED_TYPE.DEFAULT, appDefault.name],
+ // Unhiding of the default private.
+ [SearchUtils.MODIFIED_TYPE.CHANGED, appPrivateDefault.name],
+ [SearchUtils.MODIFIED_TYPE.DEFAULT_PRIVATE, appPrivateDefault.name],
+ // Hiding the engine.
+ [SearchUtils.MODIFIED_TYPE.CHANGED, lastEngine.name],
+ [SearchUtils.MODIFIED_TYPE.REMOVED, lastEngine.name],
+ ]);
// Now remove the last engine, which should set the new default.
await Services.search.removeEngine(visibleEngines[visibleEngines.length - 1]);
- let notified = await observer.promise;
+ await observer.promise;
Assert.equal(
(await getDefault(false)).name,
@@ -218,11 +196,6 @@ add_task(async function test_default_fallback_when_no_others_visible() {
appPrivateDefault.name,
"Should fallback to the app default private engine after removing all engines"
);
- Assert.equal(
- notified.name,
- appDefault.name,
- "Should have notified the correct default engine"
- );
Assert.ok(
!appPrivateDefault.hidden,
"Should have unhidden the app default private engine"
@@ -253,24 +226,21 @@ add_task(async function test_default_fallback_remove_default_no_visible() {
"Should only have one visible engine"
);
- const observer = new SearchObserver(
- [
- // Unhiding of the default engine.
- SearchUtils.MODIFIED_TYPE.CHANGED,
- // Change of the default.
- SearchUtils.MODIFIED_TYPE.DEFAULT,
- SearchUtils.MODIFIED_TYPE.DEFAULT_PRIVATE,
- // Hiding the engine.
- SearchUtils.MODIFIED_TYPE.CHANGED,
- SearchUtils.MODIFIED_TYPE.REMOVED,
- ],
- SearchUtils.MODIFIED_TYPE.DEFAULT
- );
+ const observer = new SearchObserver([
+ // Unhiding of the default engine.
+ [SearchUtils.MODIFIED_TYPE.CHANGED, "generalEngine"],
+ // Change of the default.
+ [SearchUtils.MODIFIED_TYPE.DEFAULT, "generalEngine"],
+ [SearchUtils.MODIFIED_TYPE.DEFAULT_PRIVATE, "generalEngine"],
+ // Hiding the engine.
+ [SearchUtils.MODIFIED_TYPE.CHANGED, appDefault.name],
+ [SearchUtils.MODIFIED_TYPE.REMOVED, appDefault.name],
+ ]);
// Now remove the last engine, which should set the new default.
await Services.search.removeEngine(appDefault);
- let notified = await observer.promise;
+ await observer.promise;
Assert.equal(
(await getDefault(false)).name,
@@ -282,11 +252,6 @@ add_task(async function test_default_fallback_remove_default_no_visible() {
"generalEngine",
"Should fallback the default private engine to the first general search engine"
);
- Assert.equal(
- notified.name,
- "generalEngine",
- "Should have notified the correct default engine"
- );
Assert.ok(
!Services.search.getEngineByName("generalEngine").hidden,
"Should have unhidden the new engine"
@@ -335,24 +300,21 @@ add_task(
"Should only have one visible engine"
);
- const observer = new SearchObserver(
- [
- // Unhiding of the default engine.
- SearchUtils.MODIFIED_TYPE.CHANGED,
- // Change of the default.
- SearchUtils.MODIFIED_TYPE.DEFAULT,
- SearchUtils.MODIFIED_TYPE.DEFAULT_PRIVATE,
- // Hiding the engine.
- SearchUtils.MODIFIED_TYPE.CHANGED,
- SearchUtils.MODIFIED_TYPE.REMOVED,
- ],
- SearchUtils.MODIFIED_TYPE.DEFAULT
- );
+ const observer = new SearchObserver([
+ // Unhiding of the default engine.
+ [SearchUtils.MODIFIED_TYPE.CHANGED, "default"],
+ // Change of the default.
+ [SearchUtils.MODIFIED_TYPE.DEFAULT, "default"],
+ [SearchUtils.MODIFIED_TYPE.DEFAULT_PRIVATE, "default"],
+ // Hiding the engine.
+ [SearchUtils.MODIFIED_TYPE.CHANGED, appPrivateDefault.name],
+ [SearchUtils.MODIFIED_TYPE.REMOVED, appPrivateDefault.name],
+ ]);
// Now remove the last engine, which should set the new default.
await Services.search.removeEngine(appPrivateDefault);
- let notified = await observer.promise;
+ await observer.promise;
Assert.equal(
(await getDefault(false)).name,
@@ -364,11 +326,6 @@ add_task(
"default",
"Should fallback the private engine to the first engine that isn't a general search engine"
);
- Assert.equal(
- notified.name,
- "default",
- "Should have notified the correct default engine"
- );
Assert.ok(
!Services.search.getEngineByName("default").hidden,
"Should have unhidden the new engine"
@@ -397,10 +354,10 @@ async function checkNonBuiltinFallback(checkPrivate) {
await setDefault(checkPrivate, addedEngine);
- const observer = new SearchObserver(
- [expectedDefaultNotification, SearchUtils.MODIFIED_TYPE.REMOVED],
- expectedDefaultNotification
- );
+ const observer = new SearchObserver([
+ [expectedDefaultNotification, defaultEngine.name],
+ [SearchUtils.MODIFIED_TYPE.REMOVED, addedEngine.name],
+ ]);
// Remove the current engine...
await Services.search.removeEngine(addedEngine);
@@ -412,12 +369,7 @@ async function checkNonBuiltinFallback(checkPrivate) {
"Should revert to the app default engine"
);
- let notified = await observer.promise;
- Assert.equal(
- notified.name,
- defaultEngine.name,
- "Should have notified the correct default engine"
- );
+ await observer.promise;
}
add_task(async function test_default_fallback_non_builtin() {
diff --git a/toolkit/components/search/tests/xpcshell/test_notifications.js b/toolkit/components/search/tests/xpcshell/test_notifications.js
@@ -22,14 +22,14 @@ add_setup(async function () {
});
add_task(async function test_addingEngine_opensearch() {
- const addEngineObserver = new SearchObserver(
+ const addEngineObserver = new SearchObserver([
[
// engine-added
// Engine was added to the store by the search service.
SearchUtils.MODIFIED_TYPE.ADDED,
+ "Test search engine",
],
- SearchUtils.MODIFIED_TYPE.ADDED
- );
+ ]);
await SearchTestUtils.installOpenSearchEngine({
url: `${gHttpURL}/opensearch/generic1.xml`,
@@ -37,39 +37,38 @@ add_task(async function test_addingEngine_opensearch() {
engine = await addEngineObserver.promise;
- let retrievedEngine = Services.search.getEngineByName("Test search engine");
- Assert.equal(engine, retrievedEngine);
+ engine = Services.search.getEngineByName("Test search engine");
+ Assert.ok(engine, "Should have added the engine");
});
add_task(async function test_addingEngine_webExtension() {
- const addEngineObserver = new SearchObserver(
+ const addEngineObserver = new SearchObserver([
[
// engine-added
// Engine was added to the store by the search service.
SearchUtils.MODIFIED_TYPE.ADDED,
+ "Example Engine",
],
- SearchUtils.MODIFIED_TYPE.ADDED
- );
+ ]);
await SearchTestUtils.installSearchExtension({
name: "Example Engine",
});
- let webExtensionEngine = await addEngineObserver.promise;
+ await addEngineObserver.promise;
- let retrievedEngine = Services.search.getEngineByName("Example Engine");
- Assert.equal(webExtensionEngine, retrievedEngine);
+ let webExtensionEngine = Services.search.getEngineByName("Example Engine");
+ Assert.ok(webExtensionEngine, "Should have added the web extension engine");
});
async function defaultNotificationTest(
setPrivateDefault,
expectNotificationForPrivate
) {
- const defaultObserver = new SearchObserver([
- expectNotificationForPrivate
- ? SearchUtils.MODIFIED_TYPE.DEFAULT_PRIVATE
- : SearchUtils.MODIFIED_TYPE.DEFAULT,
- ]);
+ let expected = expectNotificationForPrivate
+ ? [[SearchUtils.MODIFIED_TYPE.DEFAULT_PRIVATE, engine.name]]
+ : [[SearchUtils.MODIFIED_TYPE.DEFAULT, engine.name]];
+ const defaultObserver = new SearchObserver(expected);
await Services.search[setPrivateDefault ? "setDefaultPrivate" : "setDefault"](
engine,
Ci.nsISearchService.CHANGE_REASON_UNKNOWN
@@ -112,12 +111,12 @@ add_task(async function test_removeEngine() {
);
const removedObserver = new SearchObserver([
- SearchUtils.MODIFIED_TYPE.DEFAULT,
- SearchUtils.MODIFIED_TYPE.DEFAULT_PRIVATE,
- SearchUtils.MODIFIED_TYPE.REMOVED,
+ [SearchUtils.MODIFIED_TYPE.DEFAULT, appDefaultEngine.name],
+ [SearchUtils.MODIFIED_TYPE.DEFAULT_PRIVATE, appDefaultEngine.name],
+ [SearchUtils.MODIFIED_TYPE.REMOVED, engine.name],
]);
await Services.search.removeEngine(engine);
- await removedObserver;
+ await removedObserver.promise;
});
