Last Comment Bug 725015 - [SeaMonkey] permanent "dom/tests/mochitest/bugs/test_resize_move_windows.html | Test timed out."
: [SeaMonkey] permanent "dom/tests/mochitest/bugs/test_resize_move_windows.html...
Status: VERIFIED FIXED
[perma-orange]
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: P2 major (vote)
: mozilla13
Assigned To: Serge Gautherie (:sgautherie)
:
Mentors:
http://mxr.mozilla.org/mozilla-centra...
Depends on:
Blocks: SmTestFail 565541
  Show dependency treegraph
 
Reported: 2012-02-07 10:54 PST by Serge Gautherie (:sgautherie)
Modified: 2012-03-02 05:10 PST (History)
3 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
verified
verified
affected


Attachments
(Av1) Add missing 'return', Set needed preference, Improve some nits (3.92 KB, patch)
2012-02-07 11:57 PST, Serge Gautherie (:sgautherie)
no flags Details | Diff | Review
(Av2) Add missing 'return', Set needed preference, Improve some nits (3.40 KB, patch)
2012-02-20 16:04 PST, Serge Gautherie (:sgautherie)
no flags Details | Diff | Review
(Av3) Add missing 'return', Set needed preference, Make todo() message more explicit [Checked in: Comments 11 and 15] (1.39 KB, patch)
2012-02-28 07:46 PST, Serge Gautherie (:sgautherie)
mounir: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Review

Description Serge Gautherie (:sgautherie) 2012-02-07 10:54:25 PST
SM 2.8:
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Beta/1328566990.1328572485.22195.gz
WINNT 5.2 comm-beta debug test mochitests-2/5 on 2012/02/06 14:23:10
is already failing.

SM 2.9:
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Aurora/1328598005.1328601563.15240.gz
WINNT 5.2 comm-aurora debug test mochitests-2/5 on 2012/02/06 23:00:05
is already failing.

SM 2.10:
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1328503421.1328505166.30593.gz
Linux comm-central-trunk debug test mochitests-2/5 on 2012/02/05 20:43:41
{
...
14285 INFO TEST-PASS | /tests/dom/tests/mochitest/bugs/test_resize_move_windows.html | parameter screenY should be taken into account
14286 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/bugs/test_resize_move_windows.html | Test timed out.
SCREENSHOT: data:image/png;base64,[...]
14287 INFO TEST-END | /tests/dom/tests/mochitest/bugs/test_resize_move_windows.html | finished in 308908ms
}
NB: Screenshot simply shows the small opened window :-|

Reproducible on my local Windows 2000.

*****

This test needs to ensure "dom.disable_window_move_resize" is set to false!
http://mxr.mozilla.org/comm-central/search?string=dom.disable_window_move_resize&case=1&find=\.js
Comment 1 Serge Gautherie (:sgautherie) 2012-02-07 11:57:13 PST
Created attachment 595119 [details] [diff] [review]
(Av1) Add missing 'return', Set needed preference, Improve some nits
Comment 2 Mounir Lamouri (:mounir) 2012-02-08 05:42:11 PST
Comment on attachment 595119 [details] [diff] [review]
(Av1) Add missing 'return', Set needed preference, Improve some nits

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

A few of those changes are not very easy to understand. Could you give more context?

::: dom/tests/mochitest/bugs/test_resize_move_windows.html
@@ +72,3 @@
>    if (condition() || times == 0) {
>      test();
> +    SimpleTest.executeSoon(next);

Why?

@@ +261,5 @@
>    });
>    });
>  }
>  
> +SpecialPowers.pushPrefEnv({"set": [["dom.disable_window_move_resize", false]]}, runTests);

I would prefer:
SpecialPowers.pushPrefEnv({"set": [["dom.disable_window_move_resize", false]]}, function() {
SimpleTest.waitForFocus(function() {
  //
})
});

@@ +269,4 @@
>    if (screen.width <= 200 || screen.height <= 200) {
> +    todo(false, "Screen needs to be at least 200x200px to run this test.");
> +    SimpleTest.executeSoon(SimpleTest.finish);
> +    return;

Why?

@@ +281,5 @@
>      // However, passing size/position parameters to window.open should work.
>      var w = window.open("data:text/html,<script>" +
>        "function check(next) {" +
>        "  var is_range = function(aTest, aValue, aRange, aMsg) {" +
> +      "    window.opener.ok(aValue - aRange < aTest && aTest < aValue + aRange, aMsg);" +

Does not seem useful.

@@ +313,5 @@
>                checkChangeIsDisabled(w, function() {
>                  w.close();
>  
>                  restoreValues();
> +                SimpleTest.executeSoon(SimpleTest.finish);

Why?
Comment 3 Serge Gautherie (:sgautherie) 2012-02-08 06:23:22 PST
(In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #2)

> ::: dom/tests/mochitest/bugs/test_resize_move_windows.html
> @@ +72,3 @@
> >    if (condition() || times == 0) {
> >      test();
> > +    SimpleTest.executeSoon(next);
> 
> Why?

executeSoon(), here and elsewhere, lets the calling callback fully end, fwiw: optional.

> I would prefer:
> SpecialPowers.pushPrefEnv({"set": [["dom.disable_window_move_resize",
> false]]}, function() {
> SimpleTest.waitForFocus(function() {
>   //
> })
> });

As you prefer: I will do.

> @@ +269,4 @@
> >    if (screen.width <= 200 || screen.height <= 200) {
> > +    todo(false, "Screen needs to be at least 200x200px to run this test.");
> > +    SimpleTest.executeSoon(SimpleTest.finish);
> > +    return;
> 
> Why?

(Why what?)

> @@ +281,5 @@
> >      // However, passing size/position parameters to window.open should work.
> >      var w = window.open("data:text/html,<script>" +
> >        "function check(next) {" +
> >        "  var is_range = function(aTest, aValue, aRange, aMsg) {" +
> > +      "    window.opener.ok(aValue - aRange < aTest && aTest < aValue + aRange, aMsg);" +
> 
> Does not seem useful.

Easier to read to me: optional.
Comment 4 Benjamin Smedberg [:bsmedberg] 2012-02-09 08:37:24 PST
Please note that I just commited something with the wrong bug number in it: if you see a patch about category entries and observer notifications, you want bug 725016, not this bug.
Comment 5 Treeherder Robot 2012-02-17 02:21:38 PST
mak77%bonardo.net
https://tbpl.mozilla.org/php/getParsedLog.php?id=9409823&tree=Firefox
Rev3 Fedora 12x64 mozilla-central pgo test mochitests-2/5 on 2012-02-17 01:00:48

14408 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/bugs/test_resize_move_windows.html | Test timed out.
Comment 6 Serge Gautherie (:sgautherie) 2012-02-17 07:30:10 PST
(In reply to TinderboxPushlog Robot from comment #5)
> mak77%bonardo.net
> https://tbpl.mozilla.org/php/getParsedLog.php?id=9409823&tree=Firefox

Not this SeaMonkey bug.
Comment 7 Serge Gautherie (:sgautherie) 2012-02-20 16:04:11 PST
Created attachment 598997 [details] [diff] [review]
(Av2) Add missing 'return', Set needed preference, Improve some nits

Av1, without executeSoon() and with s/200/201/g.
Comment 8 Mounir Lamouri (:mounir) 2012-02-28 06:27:51 PST
Comment on attachment 598997 [details] [diff] [review]
(Av2) Add missing 'return', Set needed preference, Improve some nits

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

Dropping the review request because I would like to see the fixed patch.

::: dom/tests/mochitest/bugs/test_resize_move_windows.html
@@ -22,5 @@
>  SimpleTest.waitForExplicitFinish();
>  
>  var previousX, previousY, previousWidth, previousHeight;
>  
> -

no need to remove this line.

@@ +67,5 @@
>   * If times < 0, the event loop will be hitten as long as the condition isn't
>   * true or the test doesn't time out.
>   */
> +function hitEventLoop(condition, test, times, next)
> +{

no need to change the style

@@ +266,3 @@
>  SimpleTest.waitForFocus(function() {
> +  if (screen.width < 201 || screen.height < 201) {
> +    todo(false, "Screen needs to be at least 201x201px to run this test.");

Ok to change the todo description (I would have preferred "The screen needs to be bigger than 200px*200px to run this test.") but I don't think you need to change the condition.

@@ +278,5 @@
>      // However, passing size/position parameters to window.open should work.
>      var w = window.open("data:text/html,<script>" +
>        "function check(next) {" +
>        "  var is_range = function(aTest, aValue, aRange, aMsg) {" +
> +      "    window.opener.ok(aValue - aRange < aTest && aTest < aValue + aRange, aMsg);" +

I would prefer if you could drop that change.
Comment 9 Serge Gautherie (:sgautherie) 2012-02-28 07:46:15 PST
Created attachment 601274 [details] [diff] [review]
(Av3) Add missing 'return', Set needed preference, Make todo() message more explicit
[Checked in: Comments 11 and 15]

Av2, with comment 8 suggestion(s).
Comment 10 Mounir Lamouri (:mounir) 2012-02-28 07:56:55 PST
Comment on attachment 601274 [details] [diff] [review]
(Av3) Add missing 'return', Set needed preference, Make todo() message more explicit
[Checked in: Comments 11 and 15]

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

Thanks :)

r=me
Comment 11 Serge Gautherie (:sgautherie) 2012-02-28 11:31:20 PST
Comment on attachment 601274 [details] [diff] [review]
(Av3) Add missing 'return', Set needed preference, Make todo() message more explicit
[Checked in: Comments 11 and 15]

https://hg.mozilla.org/mozilla-central/rev/30b4f99a137c


Succeeded as
https://tbpl.mozilla.org/?tree=Try&rev=6d1a41cb8058


[Approval Request Comment]
Regression caused by (bug #): Bug 565541.
User impact if declined: None, but perma-orange (timeout) on SeaMonkey.
Testing completed (on m-c, etc.): Try and this comment.
Risk to taking this patch (and alternatives if risky): None, test only.
String changes made by this patch: None.
Comment 12 Mozilla RelEng Bot 2012-02-28 12:02:02 PST
Try run for 6d1a41cb8058 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=6d1a41cb8058
Results (out of 30 total builds):
    exception: 1
    success: 27
    warnings: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/sgautherie.bz@free.fr-6d1a41cb8058
Comment 13 Alex Keybl [:akeybl] 2012-02-28 13:20:52 PST
Comment on attachment 601274 [details] [diff] [review]
(Av3) Add missing 'return', Set needed preference, Make todo() message more explicit
[Checked in: Comments 11 and 15]

[Triage Comment]
Test only change - approved for Aurora 12 and Beta 11. Note that beta 5 will be the last FF11 beta for which we'll accept test changes.
Comment 14 Serge Gautherie (:sgautherie) 2012-02-29 13:33:07 PST
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1330540575.1330541423.13527.gz&fulltext=1
Linux comm-central-trunk debug test mochitests-2/5 on 2012/02/29 10:36:15
{
14453 INFO TEST-END | /tests/dom/tests/mochitest/bugs/test_resize_move_windows.html | finished in 11118ms
}

V.Fixed
Comment 15 Jens Hatlak (:InvisibleSmiley) 2012-02-29 15:12:27 PST
Comment on attachment 601274 [details] [diff] [review]
(Av3) Add missing 'return', Set needed preference, Make todo() message more explicit
[Checked in: Comments 11 and 15]

http://hg.mozilla.org/releases/mozilla-aurora/rev/b5ef80040f72
http://hg.mozilla.org/releases/mozilla-beta/rev/529498671c4c
Comment 16 Serge Gautherie (:sgautherie) 2012-03-01 06:20:47 PST
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Aurora/1330595464.1330597769.12144.gz&fulltext=1
Linux comm-aurora debug test mochitests-2/5 on 2012/03/01 01:51:04
{
14303 INFO TEST-END | /tests/dom/tests/mochitest/bugs/test_resize_move_windows.html | finished in 12910ms
}

seamonkey2.9: verified.
Comment 17 Serge Gautherie (:sgautherie) 2012-03-02 05:10:57 PST
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Beta/1330650199.1330652139.30264.gz
WINNT 5.2 comm-beta debug test mochitests-2/5 on 2012/03/01 17:03:19

firefox11: verified.

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