458810 - Implement search service test suite

Status: RESOLVED DUPLICATE of bug 1120642

(Firefox :: Search, defect, P4)

Comment 1

[David Teller [:Yoric] - still alive but not very active]

Attachment: One more test. Covering: asynchronous initialization, observer notification, downloading and installing sherlock, downloading and installing opensearch.

Comment 2

[David Teller [:Yoric] - still alive but not very active]

Attachment: One more test. Covering: asynchronous initialization, observer notification, downloading and installing sherlock, downloading and installing opensearch.

Here it is, adapted to the suggestions in bug 699856.

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 577983 [details] [diff] [review]
One more test. Covering: asynchronous initialization, observer notification, downloading and installing sherlock, downloading and installing opensearch.

>diff --git a/toolkit/components/search/tests/xpcshell/test_645970.js b/toolkit/components/search/tests/xpcshell/test_645970.js

>+  do_timeout(1000, function() {
>+      do_throw("timeout");
>+    })

Is this really necessary? The test harness should be taking care of this... (also applies to the other test)

>diff --git a/toolkit/components/search/tests/xpcshell/test_downloadAndAddEngine.js b/toolkit/components/search/tests/xpcshell/test_downloadAndAddEngine.js

>+/* ***** BEGIN LICENSE BLOCK *****

You should generally use the PD license for new tests, where possible (see https://www.mozilla.org/MPL/headers/ ).

>+Components.utils.import("resource://gre/modules/Services.jsm");
>+var gPrefService = Cc["@mozilla.org/preferences-service;1"]
>+                    .getService(Ci.nsIPrefService)
>+                    .QueryInterface(Ci.nsIPrefBranch);

Services.prefs

>+function setTimeout(aCallback, aDelay) {
>+	var timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
>+  timer.initWithCallback(new TimerCallback(aCallback), aDelay, timer.TYPE_ONE_SHOT);
>+}

You can pass aCallback to initWithCallback directly, no need for TimerCallback.

>+const files = [

Unused?

>+var IOService = Components.classes["@mozilla.org/network/io-service;1"]
>+                .getService(Components.interfaces.nsIIOService);

Services.io

>+function getResponse(aSet) {

Also seems to be unused?

>+function run_test() {

>+  // The search service needs to be started after the jarURIs pref has been
>+  // set in order to initiate it correctly
>+  let searchService = Cc["@mozilla.org/browser/search-service;1"]
>+                       .getService(Ci.nsIBrowserSearchService);

Services.search

>+  var observerService = Cc["@mozilla.org/observer-service;1"]
>+    .getService(Ci.nsIObserverService);

Services.obs

>+  var observer = { observe: function(aSubject, aTopic, aData) {
 
function observer(aSubject, aTopic, aData) {
}

More tests are great, thanks for this!

Comment 4

[Matthew N. [:MattN]]

David, do you think you'll have time to work on another version anytime soon?  If not, it's something I'd be interested in finishing up since I'm working on some other search bugs.

Comment 5

[David Teller [:Yoric] - still alive but not very active]

(In reply to Matthew N. [:MattN] from comment #4)
> David, do you think you'll have time to work on another version anytime
> soon?  If not, it's something I'd be interested in finishing up since I'm
> working on some other search bugs.

Yes, I have a few tests, waiting for bug 699856.

Comment 6

[David Teller [:Yoric] - still alive but not very active]

Attachment: Testing metadata opening/migration

Comment 7

[:Gavin Sharp [email: [e-mail]]]

We have a bunch of search service tests now. Is this bug still tracking something useful?

Comment 8

[David Teller [:Yoric] - still alive but not very active]

The search service deserves more test, but that's probably not a bug.

Comment 9

[Marco Bonardo [:mak]]

it's unclear if this is adding tests that are currently missing

Comment 10

[Mark Banner (:standard8)]

I think the test sections here are covered, if they're not, then bug 1120642 will definitely cover them. Duping to that as it is clearer.