consolidate main TB window size in mozmill tests

RESOLVED FIXED in Thunderbird 64.0

Status

enhancement
RESOLVED FIXED
10 months ago
10 months ago

People

(Reporter: aceman, Assigned: aceman)

Tracking

Trunk
Thunderbird 64.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

In bug 1387704 we identified that some TB mozmill tests resize the main app window and hard-code the dimensions at some places. Let's consolidate that into some global constants and helper functions so that mistakes do not happen.
Depends on: 1387704
Posted patch 1493461.patch (obsolete) — Splinter Review
This could do it.
It appears at one spot we were resizing the window to a non-integer width so I added the Math.round, so the requested width matches what we read back.

Some of the sleep(0)s are removed as resize_to() already contains the sleep(0). The same with wait_for_resize().

Try run:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=516d2a1ab508ba4609ef3b1227505cf4636c8dc4
Attachment #9011257 - Flags: review?(jorgk)
Posted patch 1493461.patch v1.1 (obsolete) — Splinter Review
I forgot to simplify 2 places.
Attachment #9011257 - Attachment is obsolete: true
Attachment #9011257 - Flags: review?(jorgk)
Attachment #9011269 - Flags: review?(jorgk)
This conflicts with changes from bug 1493513.
Comment on attachment 9011269 [details] [diff] [review]
1493461.patch v1.1

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

r+ with comments ;-)

::: mail/test/mozmill/message-header/test-message-header.js
@@ +888,5 @@
>      let toolbar = mc.e("header-view-toolbar");
>      let mode = toolbar.getAttribute("mode");
>  
>      // Get really big, so that we can figure out how big we actually want to be.
> +    resize_to(1200, gDefaultWindowHeight, mc);

gDefaultWindowHeight=768, the test had 600. That's intentional, yes? Does the height matter?

@@ +904,5 @@
>      toolbar.setAttribute("mode", mode);
>  
>      // And resize to half way between the big and small widths, so that we
>      //  can toggle the mode to force the overflow.
> +    resize_to(Math.round((bigWidth + smallWidth) / 2), gDefaultWindowHeight, mc);

and here

@@ +925,5 @@
>      if (expandedHeadersTopBox.clientHeight != tallHeight)
>        throw new Error("The header box should have returned to its original size!");
>  
>      // And make our window big to achieve the same effect as the just icons mode.
> +    resize_to(1200, gDefaultWindowHeight, mc);

and here and many times more below.

::: mail/test/mozmill/quick-filter-bar/test-display-issues.js
@@ +66,5 @@
>                   "Collapsy bar width:", qfbCollapsy.clientWidth,
>                   "shrunk?", qfbCollapsy.getAttribute("shrink")]);
>    }
>  
>    function assertCollapsed(width) {

Remove parameter since you removed it from the calls?

@@ +74,5 @@
>        throw new Error("The collapsy bar should be shrunk!");
>      if (qfbExemplarLabel.clientWidth != 0)
>        throw new Error("The exemplar label should be collapsed!");
>    }
>    function assertExpanded(width) {

and here?

::: mail/test/mozmill/shared-modules/test-window-helpers.js
@@ +816,5 @@
> + * @param aWidth       the requested window width
> + * @param aHeight      the requested window height
> + * @param aController  window controller
> + */
> +function resize_to(aWidth, aHeight, aController) {

Having the controller last seems awkward. Should be first, no?
Attachment #9011269 - Flags: review?(jorgk) → review+
(In reply to Jorg K (GMT+2) from comment #4)
> ::: mail/test/mozmill/message-header/test-message-header.js
> gDefaultWindowHeight=768, the test had 600. That's intentional, yes? Does
> the height matter?

I think it doesn't.
 
> ::: mail/test/mozmill/quick-filter-bar/test-display-issues.js
> @@ +66,5 @@
> >                   "Collapsy bar width:", qfbCollapsy.clientWidth,
> >                   "shrunk?", qfbCollapsy.getAttribute("shrink")]);
> >    }
> >  
> >    function assertCollapsed(width) {
> 
> Remove parameter since you removed it from the calls?

Yes, thanks.
 
> ::: mail/test/mozmill/shared-modules/test-window-helpers.js
> > +function resize_to(aWidth, aHeight, aController) {
> 
> Having the controller last seems awkward. Should be first, no?

OK, no problem.
Thanks.
Attachment #9011269 - Attachment is obsolete: true
Attachment #9011674 - Flags: review+
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/1b38cacc09e5
consolidate main TB window size handling in MozMill tests. r=jorgk
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 64.0
You need to log in before you can comment on or make changes to this bug.