1011597 - Use promise-based listener in test_ImapPump as a demo of Promise in testing

Status: RESOLVED FIXED

(MailNews Core :: Testing Infrastructure, defect)

Description

[Kent James (:rkent)]

I propose that we start to move mailnews XPCSHELL tests away from asyncTestUtils and toward promise and Task.jsm - based async management. As an initial step in this, convert test_impaPump.js to Promises.

Comment 1

[Kent James (:rkent)]

Attachment: 1011579_1.patch

Work in progress

Comment 2

[Kent James (:rkent)]

Try run: https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=f7e46b6b64db

Comment 3

[Kent James (:rkent)]

Attachment: rev 2

Just some tweaks, including using function*

Comment 4

[Kent James (:rkent)]

The main point of this is to make sure that we are happy with the overall pattern here, including the naming conventions. I used more standard module naming for example (PromiseTestUtils.jsm)instead of the local pattern (which would be promiseTestUtils.js) OK?

Also as we discussed on IRC, one big difference here is that you need to do a new PromiseTestUtils.PromiseUrlListener(); each time the listener is used, while the old listener could be reused. I still think that is the best way to do it, as promises once resolved cannot be reused.

Comment 5

[Joshua Cranmer [:jcranmer]]

Comment on attachment 8424348 [details] [diff] [review]
rev 2

Review of attachment 8424348 [details] [diff] [review]:
-----------------------------------------------------------------

I think the promise/nsIUrlListener interconversion approach may be more desirable in front-end code in the future, but hg moves are cheap and we should probably solidify our future Promises story before making it more widely available outside of tests.

::: mailnews/test/resources/PromiseTestUtils.jsm
@@ +3,5 @@
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +/**
> + * This file provides utilities useful in using Promises and Task.jsm
> + * with mailnews tests

Nit: missing .

@@ +29,5 @@
> +
> +PromiseTestUtils.PromiseUrlListener = function(aWrapped) {
> +  this.wrapped = aWrapped ? aWrapped.QueryInterface(Ci.nsIUrlListener) : null;
> +  this._deferred = Promise.defer();
> +}

Missing a semicolon here.

@@ +44,5 @@
> +      this.wrapped.OnStopRunningUrl(aUrl, aExitCode);
> +    if (aExitCode == Cr.NS_OK)
> +      this._deferred.resolve();
> +    else
> +      this._deferred.reject();

this._deferred.reject(aExitCode) perhaps?

Comment 6

[Kent James (:rkent)]

Checked in https://hg.mozilla.org/comm-central/rev/8200f186be46