684230 - test_bug434998.xul throws an exception

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[arno renevier]

Hi,
when patch for bug 678842 is applied, an error is thrown when running mochitest-chrome for editor/composer.

10 INFO TEST-INFO | chrome://mochitests/content/chrome/editor/composer/test/test_bug678842.html | [SimpleTest/SimpleTest.js, window.onerror] An error occurred: ok is not a function at chrome://mochitests/content/chrome/editor/composer/test/test_bug434998.xul:66

This probably did not happen before because test_bug434998.xul was the last one in chrome tests. Now, that there is a new chrome test, for whatever reason, the progresslistener onStateChange is called another time, when executing the next test.

Patch seems to be quite easy:
just call 
progress.removeProgressListener(progressListener, Components.interfaces.nsIWebProgress.NOTIFY_ALL);
before SimpleTest.finish().

But two questions come in my mind ?
* Why is code of one test executed when another test is in progress ?

* I noticed that on try server (probably also on inbound and central), a lot of mochitest pass while having some javascript execption uncatched. Shouldn't mochitest fail in that case ?

Comment 1

[arno renevier]

Attachment: progressListener at the end of test

patch proposal

Comment 2

[(no longer active)]

(In reply to arno renevier from comment #0)
> Hi,
> when patch for bug 678842 is applied, an error is thrown when running
> mochitest-chrome for editor/composer.
> 
> 10 INFO TEST-INFO |
> chrome://mochitests/content/chrome/editor/composer/test/test_bug678842.html
> | [SimpleTest/SimpleTest.js, window.onerror] An error occurred: ok is not a
> function at
> chrome://mochitests/content/chrome/editor/composer/test/test_bug434998.xul:66
> 
> This probably did not happen before because test_bug434998.xul was the last
> one in chrome tests. Now, that there is a new chrome test, for whatever
> reason, the progresslistener onStateChange is called another time, when
> executing the next test.
> 
> Patch seems to be quite easy:
> just call 
> progress.removeProgressListener(progressListener,
> Components.interfaces.nsIWebProgress.NOTIFY_ALL);
> before SimpleTest.finish().

Yes, that is the right idea.

> But two questions come in my mind ?
> * Why is code of one test executed when another test is in progress ?

Probably because the listener is catching the next pageload in this case...  But the ok function is now gone away because the script object from the previous page has been destroyed.  At least, that's what I think is happening!

> * I noticed that on try server (probably also on inbound and central), a lot
> of mochitest pass while having some javascript execption uncatched.
> Shouldn't mochitest fail in that case ?

Can you file bugs on individual examples?  The way that the mochitest framework works is that it loads a file, waits for the load to be finished, then checks a flag to see if the test has requested an explicit finish.  If that flag is set, it waits for a call to finish, otherwise it assumes that the test is finished and loads the next file.  If the js exception causes the execution sequence ultimately leading to finish being called to be aborted, then the test will time out.  I'm not sure if the framework tries to catch js exceptions explicitly though...

Comment 3

[(no longer active)]

Comment on attachment 558313 [details] [diff] [review]
progressListener at the end of test

Wrong patch.  :-)

Comment 4

[arno renevier]

Attachment: remove progressListener at the end of test

Comment 5

[(no longer active)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/021f1bd44f81

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

http://hg.mozilla.org/mozilla-central/rev/021f1bd44f81