Last Comment Bug 684230 - test_bug434998.xul throws an exception
: test_bug434998.xul throws an exception
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Editor (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: mozilla9
Assigned To: arno renevier
:
Mentors:
Depends on:
Blocks: 652494
  Show dependency treegraph
 
Reported: 2011-09-02 07:21 PDT by arno renevier
Modified: 2011-09-06 04:04 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
progressListener at the end of test (43.00 KB, patch)
2011-09-05 11:21 PDT, arno renevier
no flags Details | Diff | Splinter Review
remove progressListener at the end of test (1.04 KB, patch)
2011-09-05 12:12 PDT, arno renevier
ehsan: review+
Details | Diff | Splinter Review

Description arno renevier 2011-09-02 07:21:09 PDT
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 2011-09-05 11:21:40 PDT
Created attachment 558313 [details] [diff] [review]
progressListener at the end of test

patch proposal
Comment 2 :Ehsan Akhgari 2011-09-05 11:58:32 PDT
(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 :Ehsan Akhgari 2011-09-05 11:59:04 PDT
Comment on attachment 558313 [details] [diff] [review]
progressListener at the end of test

Wrong patch.  :-)
Comment 4 arno renevier 2011-09-05 12:12:39 PDT
Created attachment 558324 [details] [diff] [review]
remove progressListener at the end of test
Comment 6 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-09-06 04:04:18 PDT
http://hg.mozilla.org/mozilla-central/rev/021f1bd44f81

Note You need to log in before you can comment on or make changes to this bug.