475719 - Mozmill test for check top site - MSN

Status: VERIFIED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: patch v1.0

The following test checks the MSN website for several elements. That's the only thing we can do in MozMill. With the existing reftests for the top sites, layout issues will be already covered.

Adam and Mikeal, I still have problems with the waitForEval function. This time I want to wait until an image has been loaded. If I don't have this step, assertImageLoaded always fails. But using the following code I run into a timeout each time.

// Image has to be loaded first
controller.waitForEval("subject.complete === true", 1000, 100, img);
controller.assertImageLoaded(img);

How can it be solved? Could you have a look at the current implementation please?

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Further, can we somehow get a delay parameter for assert checks? Currently you have to call the following two function to be sure it will succeed:

controller.waitForElement(aNode);
controller.assertNode(aNode);

Would be nice to have something like 'controller.assertNode(aNode, 10000)' to be able to include a 10s timeout. If no timeout is given we should check immediately for existence of the node. That would also apply to other assert checks. For now I've moved it to a separate function in my test.

Comment 2

[Reed Loden [:reed]]

Comment on attachment 359273 [details] [diff] [review]
patch v1.0

> * The Original Code is mozilla.org code.
> *
> * The Initial Developer of the Original Code is
> * Netscape Communications Corporation.
> * Portions created by the Initial Developer are Copyright (C) 1998
> * the Initial Developer. All Rights Reserved.

This is wrong. The initial developer should definitely be "Mozilla Corporation" or "Mozilla Foundation", copyright date should be 2009, and original code should be something more detailed.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Thanks Reed. So what about this variant? Can we use it in that way?

 * The Original Code is MozMill Tests code.
 *
 * The Initial Developer of the Original Code is
 *   Henrik Skupin
 * Portions created by the Initial Developer are Copyright (C) 2009
 * the Initial Developer. All Rights Reserved.
 *
 * Contributor(s):
 *   Henrik Skupin <[e-mail]>

Tony, Clint, any other objections?

Comment 4

[Adam Christian]

As for the bug, it looks like you are trying to pass the elementslib object to waitsForEval and assertImageLoaded, I think what you want to do is the following:

    let img = new elementslib.XPath(controller.tabs.activeTab, "/html/body/div[@id='wrapper']/div[@id='page']/div[@id='nav']/div/div/div[1]/ul/li[" + ii + "]/a/img[1]");
    let link = new elementslib.XPath(controller.tabs.activeTab, "/html/body/div[@id='wrapper']/div[@id='page']/div[@id='nav']/div/div/div[1]/ul/li[" + ii + "]/a/span/strong");
    
    // Image has to be loaded first
    controller.waitForEval("subject.complete === true", 1000, 100, img.getNode());
    controller.assertImageLoaded(img);

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: patch v2

(In reply to comment #4)
>     // Image has to be loaded first
>     controller.waitForEval("subject.complete === true", 1000, 100,
> img.getNode());
>     controller.assertImageLoaded(img);

That works like a charm. Thanks Adam! Further I've updated the license text and hope that it is correct now.

Comment 6

[Mikeal Rogers [:mikeal] (mrogers@)]

Adam, don't we have a specific waitForImageLoaded API call?

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Mikeal, nothing that I was able to find in controller.js. It doesn't look so.

Comment 8

[Mikeal Rogers [:mikeal] (mrogers@)]

hrm... we have one in windmill. If it's not in now we should port it.

Comment 9

[cmtalbert]

Comment on attachment 359382 [details] [diff] [review]
patch v2

Reviewed on IRC:
whimboo: ctalbert: shall we use the license text as rob is using here?
[4:34pm] whimboo: https://bugzilla.mozilla.org/attachment.cgi?id=359375
[4:34pm] dveditz left the chat room. (Quit: dveditz)
[4:35pm] ctalbert: whimboo: this is the actual location for the license boilerplate: http://www.mozilla.org/MPL/boilerplate-1.1/
[4:35pm] ctalbert: but yes, that format is the proper format.
[4:36pm] whimboo: alright. haven't known that location
[4:36pm] ctalbert: But you can list yourself as the initial developer, it all depends on how you closely you feel ownership to that code.  I think in this case, it makes more sense to be mozilla.org code
[4:36pm] mw22: whimboo, hi, yes, well, I struggled terribly with the Mac, it's just so frustrating to get things done on that machine
[4:36pm] ctalbert: whimboo, yeah no one does.  That's why we have reviews (how I found out about it too)
[4:38pm] ctalbert: whimboo, lines 49 and 50 there are two blank lines, remove one of them.
[4:38pm] whimboo: mw22: that will be better in some months!!
[4:38pm] mw22: whimboo, I doubt it!
[4:38pm] ctalbert: That's actually the case between all the functions - just one line is fine between functions.
[4:39pm] whimboo: ctalbert: so we go with one blank line as separator at all
[4:39pm] whimboo: ok
[4:39pm] mw22: I'm looking for a window manager on the Mac that resembles Windows, anyone knows one?
[4:39pm] whimboo: for what do you need that?
[4:41pm] whimboo: tchung: i get approval
[4:42pm] mw22: I just don't get used to the Mac behavior
[4:42pm] mw22: ctalbert, whooaa!! that looks insane: http://feeds.mozilla.com/
[4:42pm] ctalbert: whimboo: as a style nit in iterating loops, people customarily use 'i'.  But there is no harm in you naming the interator 'ii'. 
[4:43pm] ctalbert: mw22, yeah, I know.  Seeing that, now I realize why I'm always behind on reading blogs.
[4:43pm] whimboo: ctalbert: will change
[4:43pm] whimboo: even the missing spaces between ii<4
[4:43pm] ctalbert: whimboo: that's up to you.  Just an FYI, really.  The rest of the patch looks good.
[4:44pm] ctalbert: yeah, those are nice, but also not mandatory.
[4:44pm] ctalbert: I'm not THAT picky 
[4:45pm] whimboo: but I. i wanna write friendly tests
[4:45pm] ctalbert: I'll give you an r+, and you can carry it forward once Adam figures out whether or not to port the waitForImage API in.  Or we can check it in as is, and add waitForImage later once it lands.  It's entirley up to you.  Let m eknow if you need me to check these in for you.
[4:45pm] whimboo: mw22: which details?
[4:45pm] ctalbert: whimboo: then friendly would be to add in the spaces 
[4:45pm] whimboo: already done

Comment 10

[Adam Christian]

We don't have a specific wait for image to load API call, but the code should be relatively straight forward if you look at assertImageLoaded and waitForElement. Maybe we should log this as a new bug, enhancement and Ill crank it out as soon as I have a spare cycle.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch v2.1

Small nits given on IRC fixed. Should be ready for check-in.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #10)
> We don't have a specific wait for image to load API call, but the code should
> be relatively straight forward if you look at assertImageLoaded and
> waitForElement. Maybe we should log this as a new bug, enhancement and Ill
> crank it out as soon as I have a spare cycle.

Filed as bug 475853.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1]]

First test checked-in. Yay!

http://hg.mozilla.org/qa/mozmill-tests/rev/bdad58ebf989

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

No failures anymore with bug 516729 fixed.

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Mass move of Mozmill Test related project bugs to newly created components. You can filter out those emails by using "Mozmill-Tests-to-MozillaQA" as criteria.