Closed Bug 491927 Opened 15 years ago Closed 15 years ago

Mozmill test for navigating to a URL using the location bar

Categories

(Mozilla QA Graveyard :: Mozmill Tests, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: u279076, Assigned: u279076)

References

()

Details

(Whiteboard: [mozmill-smoketest][mozmill-awesomebar])

Attachments

(1 file, 12 obsolete files)

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090506 Minefield/3.6a1pre
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090506 Minefield/3.6a1pre

This test covers the work for automating the Litmus testcase 5914.

Reproducible: Always
Assigning to myself for test case work.
Assignee: nobody → ashughes
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attached file TESTCASE: GO button (obsolete) —
Verifying GO button state test case
Adding bug 490548 as blocking this bug.  This test relies on being able to remove focus from the location bar so I can check if the GO button disappears.

The following is the original code:
    controller.type(addressBar, 8);  // Backspace
    var searchBox = new elementslib.Lookup(controller.window.document, '/id("main-window")/id("navigator-toolbox")/id("nav-bar")/id("search-container")/id("searchbar")/anon({"anonid":"searchbar-textbox"})/anon({"class":"autocomplete-textbox-container"})/anon({"anonid":"textbox-input-box"})/anon({"anonid":"input"})');  
    controller.click(searchBox);

The following is the temporary workaround:
      controller.type(addressBar, 8);  // Backspace
      var node = addressBar.getNode();
      node.blur();
Depends on: 490548
Attachment #377262 - Attachment mime type: application/x-javascript → text/plain
Comment on attachment 377262 [details]
TESTCASE: GO button

Just a drive-by review...

I miss the license plate. Please check the other tests how it should look like.

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

You can remove this. You don't use this module anymore.

>// Test to make sure the GO button only appears while typing
>var testGoButtonOnTypeOnly = function() {

Please mention the litmus testcase # and use a jsdoc like comment.

>  // Open a new tab
>  controller.keypress(new elementslib.Lookup(controller.window.document, '/id("main-window")/id("navigator-toolbox")/id("nav-bar")/id("urlbar-container")/id("urlbar")/anon({"class":"autocomplete-textbox-container"})/anon({"anonid":"textbox-input-box"})/anon({"anonid":"input"})'),116,false,false,false,true);

Can you use the menu item please? It would be "new elementslib.Elem(controller.menus['file-menu'].menu_newNavigatorTab);" 

>  // Press tab to take focus off location bar
>  controller.keypress(new elementslib.ID(controller.window.document, "main-window"),9,false,false,false,false);

nit: indentation needs a fix.

>  // Verify that:
>  // 1. No GO button exists initially
>  // 2. GO button appears when typing occurs in the location bar
>  // 3. GO button disappears when focus is removed from the location bar

Seems like those comments are doubled.

>    var goButton = controller.window.document.getElementById("go-button");
>    var goButtonStyle = controller.window.getComputedStyle(goButton, "");

You should change the first line. When the visibility bug of Mozmill is fixed it will lead to less work. Use the elementslib to get the go botton. And add a comment that this is a workaround like "XXX: Workaround due to bug..."

>    // Verify no GO button visible
>    if (goButtonStyle.getPropertyValue("visibility") == "visible") {
>      throw("Error: Go button is visible but should be hidden!");
>    }

I hope we can get this eliminated in the future but it's a nice way for now. 

>    // Type a single character into the address bar
>    controller.open("about:blank");

Isn't a blank page already open?

>    // Verify GO button visible and continue if TRUE
>    goButtonStyle = controller.window.getComputedStyle(goButton, "");
>    if (goButtonStyle.getPropertyValue("visibility") == "collapse") {
>      throw("Error: Go button is hidden but should be visible!");
>    }

Can you move this and the other visibility check to a helper function inside this test?

Great work for the first test.
(In reply to comment #4)

Sorry, this test case was just for the bug I was witnessing.  It is not the actual test case that I am implementing for this bug.  I should have commented as such.

Anyway, I'll be attaching the actual test case shortly.
Attached file TEST CASE: 1st Draft (obsolete) —
Here is the first draft of the automated test case.  I believe it should cover all functionality expected with test case 5914.

Review away!
Anthony, that doesn't matter. As what I can see most of my comments still apply to your 1st draft. So please update the test accordingly. Thanks.
(In reply to comment #7)
> Anthony, that doesn't matter. As what I can see most of my comments still apply
> to your 1st draft. So please update the test accordingly. Thanks.

I was hoping to get more specific feedback about each test.  Any changes I make this morning will just be to that first test.  Please provide more specific reviews for each test.  It would be much appreciated.
Attached file TESTCASE: Rev1 (obsolete) —
I believe I have implemented most if not all of your suggestions whimboo.  Please review.
Attachment #377310 - Attachment is obsolete: true
Attachment #377465 - Flags: review?(hskupin)
Attachment #377465 - Attachment mime type: application/x-javascript → text/plain
Attached file TESTCASE: Rev3 Minimized (obsolete) —
Here is a minimized test case.  Apparently I was checking enough functionality for the FFT in Rev1.  This one is far simpler.  It checks the GO button appears when it is supposed to and typing of a URL then clicking the GO button.

Review?
Attachment #377465 - Attachment is obsolete: true
Attachment #377509 - Flags: review?(hskupin)
Attachment #377465 - Flags: review?(hskupin)
Adding depends on bug 493039.  Checking a URL in the address bar cannot be done using assertValue() as it is flawed.  The following is the accepted work around:

     var addressBar = new elementslib.Lookup(controller.window.document, '/id("main-window")/id("navigator-toolbox")/id("nav-bar")/id("urlbar-container")/id("urlbar")/anon({"class":"autocomplete-textbox-container"})/anon({"anonid":"textbox-input-box"})/anon({"anonid":"input"})');
     if (addressBar.getNode().value != URL) {
       throw "Error: URL in address bar does not match expected '" + URL +"'";
     }
Depends on: 493039
Attached file TESTCASE: Rev4 (obsolete) —
Here is revision 4.  Enjoy!
Attachment #377516 - Flags: review?(hskupin)
Attachment #377509 - Attachment is obsolete: true
Attachment #377509 - Flags: review?(hskupin)
Attached file TESTCASE: Rev5 (obsolete) —
Changed the method of getting addressBar under advisement of whimboo...

OLD:
  var addressBar = new elementslib.Lookup(controller.window.document, '/id("main-window")/id("navigator-toolbox")/id("nav-bar")/id("urlbar-container")/id("urlbar")/anon({"class":"autocomplete-textbox-container"})/anon({"anonid":"textbox-input-box"})/anon({"anonid":"input"})');

NEW:
var addressBar = new elementslib.ID(controller.window.document, 'urlbar');
Attachment #377516 - Attachment is obsolete: true
Attachment #377529 - Flags: review?(hskupin)
Attachment #377516 - Flags: review?(hskupin)
Attached file TESTCASE: Rev6 (obsolete) —
Added license block
Attachment #377529 - Attachment is obsolete: true
Attachment #377560 - Flags: review?(hskupin)
Attachment #377529 - Flags: review?(hskupin)
Attached file TESTCASE: Rev7 (obsolete) —
Made revisions as per whimboo's directions
Attachment #377560 - Attachment is obsolete: true
Attachment #377575 - Flags: review?(hskupin)
Attachment #377560 - Flags: review?(hskupin)
Attached file TESTCASE: Rev8 (obsolete) —
Attachment #377575 - Attachment is obsolete: true
Attachment #377602 - Flags: review?(hskupin)
Attachment #377575 - Flags: review?(hskupin)
Attached file Testcase r9 (obsolete) —
I've updated the test case to reflect API changes.  Please review.
Attachment #377602 - Attachment is obsolete: true
Attachment #381165 - Flags: review?(hskupin)
Attachment #377602 - Flags: review?(hskupin)
Comment on attachment 381165 [details]
Testcase r9

Ok, some smaller thing which have to be fixed but looks good.

>var isMac = mozmill.platform == "darwin";
>var isLinux = mozmill.platform == "linux";

We don't need this anymore. You can access it directly by using mozmill.isMac, mozmill.isWindows, or mozmill.isLinux.

>var gDelay = 500;

Please only use another value as 0 for testing purposes. For checkins we should set it to 0.

>var testGoButtonOnTypeOnly = function() {
>  // Start from a web page
>  controller.open("http://www.mozilla.org");
>  controller.sleep(gDelay);

Use waitForPageLoad or better check for an element here.

>  var locationBar = new elementslib.Lookup(controller.window.document, '/id("main-window")/id("navigator-toolbox")/id("nav-bar")/id("urlbar-container")/id("urlbar")/anon({"class":"autocomplete-textbox-container"})/anon({"anonid":"textbox-input-box"})/anon({"anonid":"input"})');

It's enough to use the following now:

var locationBar = new elementslib.ID(controller.window.document, "urlbar");

>  // Type a single character into the location bar
>  // controller.open("about:blank");

Please remove the unnecessary commented out command

>/**
> * Test clicking location bar, typing a URL and clicking the GO button
> */
> 
>var testClickLocationBarAndGo = function() {
>  // Start from a web page
>  controller.open("http://www.mozilla.org");
>  controller.sleep(gDelay);
>  
>  var locationBar = new elementslib.Lookup(controller.window.document, '/id("main-window")/id("navigator-toolbox")/id("nav-bar")/id("urlbar-container")/id("urlbar")/anon({"class":"autocomplete-textbox-container"})/anon({"anonid":"textbox-input-box"})/anon({"anonid":"input"})');

See the above comment.

>  typeURL(locationBar, "http://www.google.com/webhp?complete=1&hl=en");

You don't need this own function. Just call controller.type. It will also send single key events to the given element.

>  // Assertion
>  checkPageLoaded();

We only have one call to this function which makes it useless. Can you please move the code here directly? If you want it should be enough to use delayedAssertNode from the utils API.

>  checkURLValue(locationBar, "http://www.google.com/webhp?complete=1&hl=en");

Please move this also inline.


>  // Check if the URL bar matches the expected domain name
>  // XXX: Until the assertValue API is improved, use getNode().value (bug 493039)
>  if (aElement.getNode().value != aURL) {
>    throw "Error: URL in location bar does not match expected '" + aURL +"'";

That way isn't needed anymore since bug 493039 has been fixed now. Just use assertValue with the element and the expected URL as parameter.

>/**
> * Checks the visibility of an element
> * @param {element} aElement A DOM element
> * @param {boolean} aVisible Whether an element should be visible or not
> * @throws Error Element is visible but should be hidden
> * @throws Error Element is hidden but should be visible
> */
>var assertElementVisible = function(aElement, aVisible) {
>  // XXX: Until Mozmill tests fail when an invisible element is actioned,
>  //      use the style property (bug 490548)
>  var style = controller.window.getComputedStyle(aElement.getNode(), "");
>  var visibility = style.getPropertyValue("visibility");
> 
>  if (aVisible) {
>    if (visibility != 'visible')
>      throw "Error: Element is hidden but should be visible";
>  } else {
>    if (visibility == 'visible')
>      throw "Error: Element is visible but should be hidden";
>  }
>} 

That looks like I should move it to the utils API too. I would help us in the future.
Attachment #381165 - Flags: review?(hskupin) → review-
Attached patch Patch for Test Case 5914 (obsolete) — Splinter Review
Here is my first patch (be kind).  It adds testLocationBar.js to firefox/testToolbars and adds assertElementVisible() to testUtilsAPI.js.
Attachment #377262 - Attachment is obsolete: true
Attachment #381165 - Attachment is obsolete: true
Attachment #381440 - Flags: review?(hskupin)
Attachment #381440 - Attachment is patch: true
Attachment #381440 - Flags: review?(hskupin) → review-
Comment on attachment 381440 [details] [diff] [review]
Patch for Test Case 5914

That looks good Anthony! I think one more round and we are ready for checkin.

>diff --git a/firefox/testToolbars/testLocationBar.js b/firefox/testToolbars/testLocationBar.js

Please name it testGoButton.js. That's the name I suggested yesterday because we test the go button in this test. You can do a simple "hg rename" to change it.

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

Due to the fix on bug 488492 we don't need those both lines anymore. The needed resources are automatically imported now.

>+// Include necessary modules
>+var RELATIVE_ROOT = '../../shared-modules';
>+var MODULE_REQUIRES = ['UtilsAPI'];
>+
>+var setupModule = function(module) {
>+  module.controller = mozmill.getBrowserController();
>+  module.utils = collector.getModule('UtilsAPI');
>+}

Same applies to the getModule call. It will happen during the module collection. Remove the line.

>+  // Verify GO button is hidden
>+  var goButton = new elementslib.ID(controller.window.document, "go-button");
>+  utils.assertElementVisible(controller, goButton, false);

This has to be called via UtilsAPI.assertElementVisible now. It applies to all instances.

>+  // Type a single character into the location bar
>+  controller.keypress(null, "l", {metaKey: mozmill.isMac, ctrlKey: !mozmill.isMac});
>+  controller.keypress(locationBar, "w", {});

Can you add "focus" to the comment's beginning please?

>+  // Verify GO button visible and continue if TRUE
>+  utils.assertElementVisible(controller, goButton, true);
>+ 
>+  // Press Backspace to delete the typed character
>+  controller.keypress(locationBar, "VK_BACK_SPACE", {});
>+  
>+  // Press ESC to clear focus
>+  controller.keypress(locationBar, "VK_ESCAPE", {});   
>+      
>+  // Verify the GO button is hidden again
>+  utils.assertElementVisible(controller, goButton, false);

Could you add delays with gDelay as parameter between these steps? That makes it easier later to debug the test with only changing one variable.

>+  // Type a URL into the location bar

Same commit nit here.
>+  // Click the GO button
>+  var goButton = new elementslib.ID(controller.window.document, "go-button");

Where do you click? I cannot find a call to controller.click.

>+  // Check if the Google logo exists
>+  controller.assertNode(new elementslib.Name(controller.tabs.activeTab, "q"));

Can you please check that the go button is hidden again now? With this check we can detect a broken/missing click call.

>+  // Check if the URL bar matches the expected domain name
>+  controller.assertValue(locationBar, "http://www.google.com/webhp?complete=1&hl=en");
>+}
>+
>+
>+

Nit: Please only one empty line at the end.

>diff --git a/shared-modules/testUtilsAPI.js b/shared-modules/testUtilsAPI.js

>+ * Checks the visibility of an element
>+ * @param {controller} aController A Mozmill controller
>+ * @param {element} aElement A DOM element
>+ * @param {boolean} aVisible Whether an element should be visible or not

Please remove the brackets?

>+var assertElementVisible = function(aController, aElement, aVisible) {

Can you put it in-front of delayedAssertNode please?

>+}
>\ No newline at end of file

Please a new line at the end. Grab one from your test. :)
Attached patch Patch for Test Case 5914 (obsolete) — Splinter Review
Here's my second attempt at a patch.

*crosses fingers*
Attachment #381440 - Attachment is obsolete: true
Attachment #381565 - Flags: review?(hskupin)
Comment on attachment 381565 [details] [diff] [review]
Patch for Test Case 5914

>+// Include necessary modules
>+var RELATIVE_ROOT = '../../shared-modules';
>+var MODULE_REQUIRES = ['UtilsAPI'];
>+
>+var setupModule = function(module) {
>+  module.controller = mozmill.getBrowserController();
>+}
>+
>+var gDelay=0;

Please move it before setupModule.

>+  // Focus and type a single character into the location bar
>+  controller.keypress(null, "l", {metaKey: mozmill.isMac, ctrlKey: !mozmill.isMac});
>+  controller.sleep(gDelay);
>+  controller.keypress(locationBar, "w", {});

Can you put a delay behind each visual action? It's not needed when we use waitForPageLoad or delayedAssertNode afterward.

>+  // Press Backspace to delete the typed character
>+  controller.keypress(locationBar, "VK_BACK_SPACE", {});
>+  controller.sleep(gDelay);
>+  // Press ESC to clear focus
>+  controller.keypress(locationBar, "VK_ESCAPE", {});   

Please insert an empty line in-front of comments. That makes them more discoverable.

>+  // Wait for the page to load
>+  controller.waitForPageLoad();
>+  
>+  // Check if the Google logo exists
>+  controller.assertNode(new elementslib.Name(controller.tabs.activeTab, "q"));

Please use delayedAssertNode here. That will save us the unreliable waitForPageLoad.

> /**
>+ * Checks the visibility of an element
>+ * @param controller aController A Mozmill controller
>+ * @param element aElement A DOM element
>+ * @param boolean aVisible Whether an element should be visible or not
>+ * @throws Error Element is visible but should be hidden
>+ * @throws Error Element is hidden but should be visible
>+ */

Can you insert an empty line between the different sections?

Otherwise great work! Looks like that we can checkin today. With this changes r+.
Attachment #381565 - Flags: review?(hskupin) → review+
"Final" patch (I hope).
Attachment #381565 - Attachment is obsolete: true
Attachment #381624 - Flags: review?(hskupin)
Attachment #381624 - Flags: review?(hskupin) → review+
Comment on attachment 381624 [details] [diff] [review]
Patch for Test Case 5914

Looks good. I only wonder if we should with about:blank as start page for both tests. But we could change this later. Lets get this test into the repository.
I accidentally checked in the utilsAPI first. So we have two changesets:

http://hg.mozilla.org/qa/mozmill-tests/rev/85dc81b4b783
http://hg.mozilla.org/qa/mozmill-tests/rev/c08d2ecd2479

Thanks Anthony!
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090604 Shiretoko/3.5pre
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1pre) Gecko/20090604 Shiretoko/3.5pre
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1pre) Gecko/20090604 Shiretoko/3.5pre

Test works on all fronts.
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.
Component: Location Bar → Mozmill Tests
Product: Firefox → Mozilla QA
QA Contact: location.bar → mozmill-tests
Should have been marked verified long ago.
Status: RESOLVED → VERIFIED
Summary: [mozmill] Test for navigating to a URL using the location bar → Mozmill test for navigating to a URL using the location bar
Whiteboard: [mozmill-functional][mozmill-awesomebar]
Whiteboard: [mozmill-functional][mozmill-awesomebar] → [mozmill-smoketest][mozmill-awesomebar]
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: