500542 - Mozmill test for opening a new tab

Status: VERIFIED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Tracy Walker [:tracy]]

Attachment: test case for Litmus ID 6044

Splitting off from bug 479500

Comment 1

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

Comment on attachment 385256 [details]
test case for Litmus ID 6044

> * The Original Code is Mozilla Corporation Code.
> *
> * The Initial Developer of the Original Code is
> * Tracy Walker <[e-mail]>.
> * Portions created by the Initial Developer are Copyright (C) 2009
> * the Initial Developer. All Rights Reserved.
> *
> * Contributor(s):

Can you please fix?

>var mozmill = {}; Components.utils.import('resource://mozmill/modules/mozmill.js', mozmill);
>var elementslib = {}; Components.utils.import('resource://mozmill/modules/elementslib.js', elementslib);

Not necessary anymore.

>var testNewWindow = function () {

Shouldn't it be new Tab?

>  // opens a new tab, by default it should open with an a blank (Untitled) page
>  controller.click(new elementslib.Elem(controller.menus['file-menu'].menu_newNavigatorTab));
>  controller.sleep(1000);

No need to sleep here. But you can use the gDelay way we have for other tests.

>  // checks that the new tab is opened with (Untitled) blank page
>  var tabTitle = new elementslib.Lookup(controller.window.document, '/id("main-window")/id("browser")/id("appcontent")/id("content")/anon({"anonid":"tabbox"})/anon({"anonid":"strip"})/anon({"anonid":"tabcontainer"})/{"label":"(Untitled)"}');
>  controller.waitForElement(tabTitle);
>  controller.assertNode(tabTitle);

Please use the value of the location bar here like:
  controller.assertValue(new elementslib.ID(controller.window.document, 'urlbar'), '');

>  // close the new Tab to reset state
>  controller.click(new elementslib.Elem(controller.menus['file-menu'].menu_close));
>}

Please also check the shortcut key. The toolbar button cannot be handled at the moment because it will be too difficult and it is not shown by default.

To close all the tabs you can call UtilsAPI.closeAllTabs from inside the teardownModule function.

Comment 2

[Tracy Walker [:tracy]]

(In reply to comment #1)
> (From update of attachment 385256 [details])
> > * The Original Code is Mozilla Corporation Code.
> > *
> > * The Initial Developer of the Original Code is
> > * Tracy Walker <[e-mail]>.
> > * Portions created by the Initial Developer are Copyright (C) 2009
> > * the Initial Developer. All Rights Reserved.
> > *
> > * Contributor(s):
> 
> Can you please fix?
> 

I was told by Clint that I am the initial developer. See bug 479500 comment #10.

> >var mozmill = {}; Components.utils.import('resource://mozmill/modules/mozmill.js', mozmill);
> >var elementslib = {}; Components.utils.import('resource://mozmill/modules/elementslib.js', elementslib);
> 
> Not necessary anymore.
> 

Why is the Components.utils.import not needed anymore?  Is this documented somewhere (not in the code)? 

> >var testNewWindow = function () {
> 
> Shouldn't it be new Tab?
>

heh, yes, I'll fix that.
 
> >  // opens a new tab, by default it should open with an a blank (Untitled) page
> >  controller.click(new elementslib.Elem(controller.menus['file-menu'].menu_newNavigatorTab));
> >  controller.sleep(1000);
> 
> No need to sleep here. But you can use the gDelay way we have for other tests.


Do need to sleep there need to let the tab load before starting next the next check. But what is gDelay? Where is it documented?

> 
> >  // checks that the new tab is opened with (Untitled) blank page
> >  var tabTitle = new elementslib.Lookup(controller.window.document, '/id("main-window")/id("browser")/id("appcontent")/id("content")/anon({"anonid":"tabbox"})/anon({"anonid":"strip"})/anon({"anonid":"tabcontainer"})/{"label":"(Untitled)"}');
> >  controller.waitForElement(tabTitle);
> >  controller.assertNode(tabTitle);
> 
> Please use the value of the location bar here like:
>   controller.assertValue(new elementslib.ID(controller.window.document,
> 'urlbar'), '');

I'm not checking the value of the URL bar. The check looking for the actual text for the title of the tab itself.

> 
> >  // close the new Tab to reset state
> >  controller.click(new elementslib.Elem(controller.menus['file-menu'].menu_close));
> >}
> 
> Please also check the shortcut key. The toolbar button cannot be handled at the
> moment because it will be too difficult and it is not shown by default.

what toolbar button?  what shortcut key?  This is just cleanup. The test case is finished. It doesn't matter the method of closing the tab. You were the one that said to use this menu method in previous review.

> 
> To close all the tabs you can call UtilsAPI.closeAllTabs from inside the
> teardownModule function.

where is this documented?

Comment 3

[Tracy Walker [:tracy]]

> > * The Original Code is Mozilla Corporation Code.
> > *
> > * The Initial Developer of the Original Code is
> > * Tracy Walker <[e-mail]>.
> > * Portions created by the Initial Developer are Copyright (C) 2009
> > * the Initial Developer. All Rights Reserved.
> > *
> > * Contributor(s):
> 
> Can you please fix?
> 

I was told by Clint that I am the initial developer. See bug 479500 comment
#10.

ah, I get it. you want them like this:

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

Comment 4

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

(In reply to comment #2)
> Why is the Components.utils.import not needed anymore?  Is this documented
> somewhere (not in the code)? 

Those are automatically initialized by Mozmill itself. If you start a new test from the template both will not be included anymore too. This should be reflected in the docs which will be updated after 1.2 has been released. My examples will show this too.

> > >  controller.click(new elementslib.Elem(controller.menus['file-menu'].menu_newNavigatorTab));
> > >  controller.sleep(1000);
> > 
> > No need to sleep here. But you can use the gDelay way we have for other tests.
> 
> Do need to sleep there need to let the tab load before starting next the next
> check. But what is gDelay? Where is it documented?

Then use waitForPageLoad to make sure the page has been definitely loaded. To the mentioned gDelay variable check the other tests in the repository. We use it as a global variable for testing purposes. So someone can easily change its value when a test has to be debugged. No need to modify each sleep instance.

> > >  // checks that the new tab is opened with (Untitled) blank page
> > >  var tabTitle = new elementslib.Lookup(controller.window.document, '/id("main-window")/id("browser")/id("appcontent")/id("content")/anon({"anonid":"tabbox"})/anon({"anonid":"strip"})/anon({"anonid":"tabcontainer"})/{"label":"(Untitled)"}');
> > >  controller.waitForElement(tabTitle);
> > >  controller.assertNode(tabTitle);
> > 
> > Please use the value of the location bar here like:
> >   controller.assertValue(new elementslib.ID(controller.window.document,
> > 'urlbar'), '');
> 
> I'm not checking the value of the URL bar. The check looking for the actual
> text for the title of the tab itself.

Ok, you are right. We should check for the tab title here. But in this case we have to go another way. The title is localized so we have to retrieve its value via the getProperty function from the UtilsAPI. You have to dynamically build the lookup string:

var title = UtilsAPI.getProperty("chrome://browser/locale/tabbrowser.properties", "tabs.untitled");
  controller.assertText(tab, title);

var tabTitle = new elementslib.Lookup(controller.window.document, '/id("main-window")/id("browser")/id("appcontent")/id("content")/anon({"anonid":"tabbox"})/anon({"anonid":"strip"})/anon({"anonid":"tabcontainer"})/{"label":"' + title + '"}/anon({"xbl:inherits":"value=label,crop,accesskey","class":"tab-text","crop":"end"})')
controller.assertNode(tabTitle);

> > Please also check the shortcut key. The toolbar button cannot be handled at the
> > moment because it will be too difficult and it is not shown by default.
> 
> what toolbar button?  what shortcut key?  This is just cleanup. The test case
> is finished. It doesn't matter the method of closing the tab. You were the one
> that said to use this menu method in previous review.

Sure, but this was a long time ago. Meanwhile the keypress function works reliable to simulate shortcuts across platforms. The litmus test included both other ways to check if a new tab is opened.

> > To close all the tabs you can call UtilsAPI.closeAllTabs from inside the
> > teardownModule function.
> 
> where is this documented?

Not yet. For now it is learning by coding.

Comment 5

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

(In reply to comment #3)
> I was told by Clint that I am the initial developer. See bug 479500 comment
> #10.
> 
> ah, I get it. you want them like this:

Yes, that's perfect.

Comment 6

[Tracy Walker [:tracy]]

Attachment: test case for Litmus ID 6044 v3

(In reply to comment #4)

> 
> Sure, but this was a long time ago. Meanwhile the keypress function works
> reliable to simulate shortcuts across platforms. The litmus test included both
> other ways to check if a new tab is opened.
> 
OK, those additional methods are really for BFT or FFT testing.  The smoketest level portion of this test is just open a New Tab.  I've done that in the test case.  Nothing more is needed for the Smoketest.

Comment 7

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

Comment on attachment 386023 [details]
test case for Litmus ID 6044 v3

>// Global timeout value
>const gTimeout = 10000;

As in the other tests it should be gDelay and sleep calls should be placed between the different steps to test.

>  // ensures current tab does not have blank page loaded
>  controller.open('http://www.mozilla.org');
>  controller.waitForPageLoad(controller.tabs.activeTab);

It's not needed but let it stay.

>  // opens a new tab, by default it should open with an a blank (Untitled) page
>  controller.click(new elementslib.Elem(controller.menus['file-menu'].menu_newNavigatorTab));
>  controller.sleep(500);
>  controller.waitForPageLoad(controller.tabs.activeTab);

This could cause a failure if it takes a bit longer to open the new tab. I have replaced the sleep call with the waitForEval function which checks for the number of open tabs.

>  // close the new Tab to reset state
>  UtilsAPI.closeAllTabs(controller);

This should be moved to teardownModule.

> OK, those additional methods are really for BFT or FFT testing.  The smoketest
> level portion of this test is just open a New Tab.  I've done that in the test
> case.  Nothing more is needed for the Smoketest.

When we mark this as fixed we will never look at it again. So I disagree. Those additional two checks are simple. I have added them to the end of the test.

Otherwise it can be checked-in. Thanks Tracy!

Comment 8

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

Checked-in as http://hg.mozilla.org/qa/mozmill-tests/rev/f88d610d1e3d

Comment 9

[Tracy Walker [:tracy]]

You're not getting the point of smoketests. In this case, if the main menu item works, then the smoketest passes. If the keyboard short fails, it's not a smoketest blocker.  

The smoketest for this in litmus should be changed.  New test cases at a BFT and/or FFT level should be made for the other methods of opening a new tab. Hmmm, in fact, the smoketest should be for clicking the "+" button as it's the most visible method to do this.

Comment 10

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

(In reply to comment #9)
> You're not getting the point of smoketests. In this case, if the main menu item
> works, then the smoketest passes. If the keyboard short fails, it's not a
> smoketest blocker.  

We cannot separate smoketest runs from other Litmus testgroups. It's not possible that way with Mozmill. I gave an proposal for that but it was denied. On the other hand we are so fast in processing those tests that we can run mostly all FFT tests in the same time as we need for manual Smoketests.

> The smoketest for this in litmus should be changed.  New test cases at a BFT
> and/or FFT level should be made for the other methods of opening a new tab.
> Hmmm, in fact, the smoketest should be for clicking the "+" button as it's the
> most visible method to do this.

Until this will happen the current Mozmill test reflects its current state. CC'ing Marcia so she can decide about how the test has to be updated.

Comment 11

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

This test fails now because of the backend change for controller.doubleClick. When bug 503049 has been fixed we can get it to work again.

Comment 12

[Dão Gottwald [:dao]]

Comment on attachment 386023 [details]
test case for Litmus ID 6044 v3

>  var tabTitle = new elementslib.Lookup(controller.window.document,'/id("main-window")/id("browser")/id("appcontent")/id("content")/anon({"anonid":"tabbox"})/anon({"anonid":"strip"})/anon({"anonid":"tabcontainer"})/{"label":"'+ title +'"}/anon({"xbl:inherits":"value=label,crop,accesskey","class":"tab-text","crop":"end"})')

I hope you don't expect developers to fix up these kind of tests when they fail, which they will, being tightly bound to a very specific DOM structure.

Comment 13

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

(In reply to comment #12)
> (From update of attachment 386023 [details])
> >  var tabTitle = new elementslib.Lookup(controller.window.document,'/id("main-window")/id("browser")/id("appcontent")/id("content")/anon({"anonid":"tabbox"})/anon({"anonid":"strip"})/anon({"anonid":"tabcontainer"})/{"label":"'+ title +'"}/anon({"xbl:inherits":"value=label,crop,accesskey","class":"tab-text","crop":"end"})')
> 
> I hope you don't expect developers to fix up these kind of tests when they
> fail, which they will, being tightly bound to a very specific DOM structure.

So do you have a better idea how we can get those elements?

Comment 14

[Dão Gottwald [:dao]]

I don't live in mozmill land, there might be limitations, but my first suggestion would be controller.window.gBrowser.selectedTab.label.

Comment 15

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

Checked in the changes we need with the new doubleClick implementation: http://hg.mozilla.org/qa/mozmill-tests/rev/78fb8dcd60ba

(In reply to comment #14)
> I don't live in mozmill land, there might be limitations, but my first
> suggestion would be controller.window.gBrowser.selectedTab.label.

The access is different as users will see it. We only work with DOM nodes and don't wanna access any JS object. Only when it is really necessary. If the tabbrowser implementation will change at some point we have to update our test. But for now this should be ok.

Comment 16

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

It looks like the intention of the test is to validate the end user interface, to this end the elements are looked up by resolving the interface node and validating it's contents.

If the intention of the test is to assert that this particular tab title is viewable in this particular piece of interface then you'll need to stick with the Lookup expression, although you can feel free to modify it to remove some attribute that may be causing it to be a little more brittle. 

If you're just using the tab title to validate that the tab was indeed created and the proper site loaded then go ahead and use gBrowser.selectedTab.label as it won't break when the interface implementation is changed.

And as a quick response to comment #12; it's QA's responsibility to update mozmill tests when major changes land in the product, not developers. The intention of mozmill test's are to automate QA's time, not to bog down the development process.

Comment 17

[Dão Gottwald [:dao]]

(In reply to comment #15)
> The access is different as users will see it. We only work with DOM nodes and
> don't wanna access any JS object.

gBrowser is a DOM node, and so is gBrowser.selectedTab. Moreover, it's the interface we care about. We absolutely don't care about the fact that #main-window contains #browser, which contains #appcontent, which contains #content (that is gBrowser, btw), which contains an element whose anonid is "tabbox", which contains an element whose anonid is "strip", which contains an element whose anonid is "tabcontainer", which contains an element whose label is Foo (that is gBrowser.selectedTab, presumably), which contains an element with some random attributes.

(In reply to comment #16)
> It looks like the intention of the test is to validate the end user interface,
> to this end the elements are looked up by resolving the interface node and
> validating it's contents.
> 
> If the intention of the test is to assert that this particular tab title is
> viewable in this particular piece of interface then you'll need to stick with
> the Lookup expression,

That doesn't imply the need for an XPath-like expression. In fact, the given expression seems rather unsuited not only because of its maintenance cost, but because it doesn't verify that the matched tab is the one that was previously added.

> And as a quick response to comment #12; it's QA's responsibility to update
> mozmill tests when major changes land in the product, not developers. The
> intention of mozmill test's are to automate QA's time, not to bog down the
> development process.

So mozmill tests won't appear on the mozilla-central tinderbox waterfall, or the tree won't be orange when they fail?

Comment 18

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

Seems like we should have a further discussion on a separate bug. I'll file a new one so we can discuss it there if improvements for Mozmill can be made.

Comment 19

[Dão Gottwald [:dao]]

You should probably reopen this bug, as the expression yields false positives:

> because it doesn't verify that the matched tab is the one that was previously
> added.

Comment 20

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

I made some changes so we only use two tabs and are closing the newly one immediately after the test. That way we also can get the double click to work on the tabstrip. With the former way there was no free space available on Windows.

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

Comment 21

[Dão Gottwald [:dao]]

If there are two tabs, it's guaranteed that the first tab's title isn't "(Untitled)"?

Comment 22

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

Yes. The first tab shows the mozilla.org website or an error page. Both have another title.

Comment 23

[Dão Gottwald [:dao]]

Except if something breaks and the page title isn't used correctly...

Comment 24

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

(In reply to comment #23)
> Except if something breaks and the page title isn't used correctly...

Which is definitely covered by a Mochitest, right? We don't wanna duplicate work.

Comment 25

[Dão Gottwald [:dao]]

I don't know if it's covered. But just using the right tab hardly means to duplicate work, anyway.

Comment 26

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

Made a small change to ensure that there is no blank tab open when running the test: http://hg.mozilla.org/qa/mozmill-tests/rev/edfcc347d2f7

Comment 27

[Dão Gottwald [:dao]]

That looks like tabTitle would be undefined they way you use it.

Comment 28

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

Correct and that's expected. No element will be found with this lookup string. Otherwise the test fail. Mozmill works a bit different than Mochitest. So this bug is really solved now.

Comment 29

[Dão Gottwald [:dao]]

I'm saying that 'tabTitle' is 'undefined'.
https://developer.mozilla.org/en/Core_JavaScript_1.5_Reference/Global_Properties/undefined

Comment 30

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

Oh I see. No, we don't have to take care of it in our tests. It is abstracted by the elementslib classes of Mozmill. But thanks for mentioning that.

Mikeal, can you check in the Mozmill code that we are using the correct way (===, !==) to compare values with undefined? I can see some inconsistencies.

Comment 31

[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.