Make Mozmill-test testAccessLocationBar local

RESOLVED FIXED

Status

Mozilla QA
Mozmill Tests
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: ashughes, Assigned: ashughes)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [litmus-data])

Attachments

(3 attachments, 3 obsolete attachments)

(Assignee)

Description

8 years ago
Module: testAwesomeBar/testAccessLocationBar.js
Test-page: Any 3 pages from test-files/layout/mozilla*
(Assignee)

Updated

8 years ago
Blocks: 579965
(Assignee)

Updated

8 years ago
Assignee: nobody → anthony.s.hughes
Status: NEW → ASSIGNED
(Assignee)

Updated

8 years ago
Blocks: 575986
(Assignee)

Comment 1

8 years ago
Created attachment 464634 [details] [diff] [review]
Patch v1 (default)
Attachment #464634 - Flags: review?(aaron.train)
Comment on attachment 464634 [details] [diff] [review]
Patch v1 (default)

> // Include necessary modules
> var RELATIVE_ROOT = '../../shared-modules';
> var MODULE_REQUIRES = ['PlacesAPI', 'ToolbarAPI'];

const
 
>+  // Second - Arrow down to open the autocomplete list, displaying
>+  // the most recent visit first, then arrow down again to the first entry, 
>+  // in this case mozilla.html
>   controller.keypress(locationBar.urlbar, "VK_DOWN", {});
>-  controller.sleep(gDelay);
>+  controller.sleep(DELAY);
I've been told to simply use 100 instead of DELAY

r-, otherwise patch looks good
Attachment #464634 - Flags: review?(aaron.train) → review-
As well, 

+  // Open a few different sites to create a small history 
+  // NOTE: about:blank doesn't appear in history and clears the page 
+  //       for clean test arena
+  for each (page in LOCAL_TEST_PAGES) {
+    locationBar.loadURL(page);
     controller.waitForPageLoad();
   }

for each (var page in LOCAL_TEST_PAGES)
(Assignee)

Comment 4

8 years ago
Created attachment 464961 [details] [diff] [review]
Patch v2 (default)
Attachment #464634 - Attachment is obsolete: true
Attachment #464961 - Flags: review?(aaron.train)

Updated

8 years ago
Attachment #464961 - Flags: review?(aaron.train) → review+
(Assignee)

Updated

8 years ago
Attachment #464961 - Flags: review?(hskupin)
Comment on attachment 464961 [details] [diff] [review]
Patch v2 (default)

>+const LOCAL_TEST_PAGES = [
>+                          LOCAL_TEST_FOLDER + 'layout/mozilla.html',
>+                          LOCAL_TEST_FOLDER + 'layout/mozilla_projects.html',
>+                          LOCAL_TEST_FOLDER + 'layout/mozilla_mission.html',
>+                          'about:blank'

2 spaces as indent for arrays please.

>+  controller.waitForEval("subject.selectedIndex == 0", TIMEOUT, 100,
>+                         locationBar.autoCompleteResults);
>+  locationBar.contains("mozilla");

We should change the order of the pages. With moving up the projects page and check for 'projects' instead of 'mozilla' we have a reliable test.

>+  // Check that 'mozilla' is in the url bar
>+  locationBar.contains("mozilla");

Please also check for projects.

If the project page doesn't have an image, simply use another element for verification of a successful load.
Attachment #464961 - Flags: review?(hskupin) → review-
(Assignee)

Comment 6

8 years ago
Created attachment 465042 [details] [diff] [review]
Patch v3 (default)
Attachment #464961 - Attachment is obsolete: true
Attachment #465042 - Flags: review?(hskupin)
Comment on attachment 465042 [details] [diff] [review]
Patch v3 (default)

>+const LOCAL_TEST_PAGES = [
>+                           LOCAL_TEST_FOLDER + 'layout/mozilla_projects.html',
>+                           LOCAL_TEST_FOLDER + 'layout/mozilla.html',
>+                           LOCAL_TEST_FOLDER + 'layout/mozilla_mission.html',
>+                           'about:blank'
>+                         ];

Please correct the indentation.

>   controller.keypress(locationBar.urlbar, "VK_DOWN", {});
>-  controller.sleep(gDelay);
>+  controller.sleep(100);
>   controller.keypress(locationBar.urlbar, "VK_DOWN", {});
>-  controller.sleep(gDelay);
>+  controller.sleep(100);
> 
>-  // checks that the first item in the drop down list is selected.
>-  controller.waitForEval("subject.selectedIndex == 0",
>-                         gTimeout, 100, locationBar.autoCompleteResults);
>-  locationBar.contains("getpersonas");
>+  // Check that the first item in the drop down list is selected
>+  controller.waitForEval("subject.selectedIndex == 0", TIMEOUT, 100,
>+                         locationBar.autoCompleteResults);
>+  locationBar.contains("projects");

Probably my fault from the last review but this should fail. We have to test for mission. Was a test-run successful?

>+  // Check that 'mozilla' is in the url bar
>+  locationBar.contains("projects");

nit: Please update both lines according to the comment above.
Attachment #465042 - Flags: review?(hskupin) → review-
(Assignee)

Comment 8

8 years ago
Created attachment 465475 [details] [diff] [review]
Patch v4 (default)
Attachment #465042 - Attachment is obsolete: true
Attachment #465475 - Flags: review?(hskupin)
(Assignee)

Comment 9

8 years ago
> Probably my fault from the last review but this should fail. We have to test
> for mission. Was a test-run successful?

Patch v4 (attached) addresses your comments in comment 7.  Unfortunately, I cannot verify whether this completely passes due to the dependent failure.  Let's get the localization addressed first so we can resolve the failure.
Which dependent failure do you mean?
(Assignee)

Comment 11

8 years ago
(In reply to comment #10)
> Which dependent failure do you mean?

It's the 1st bug marked as Dependent on this bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=575986
Comment on attachment 465475 [details] [diff] [review]
Patch v4 (default)

>-  // Check that getpersonas is in the url bar
>-  locationBar.contains("getpersonas");
>+  // Check that 'mission' is in the url bar
>+  locationBar.contains("mission");
> }

Now with the move to the local page we should be able to compare the URL directly. There shouldn't be a need to check only parts of it. Can you please fix this?

Otherwise r=me (given that the patch has been tested and run successfully)
Attachment #465475 - Flags: review?(hskupin) → review+
(Assignee)

Updated

8 years ago
Keywords: checkin-needed
(Assignee)

Comment 13

8 years ago
(In reply to comment #8)
> Created attachment 465475 [details] [diff] [review]
> Patch v4 (default)

Landed on default with comment 12 addressed:
http://hg.mozilla.org/qa/mozmill-tests/rev/8be37f99b646
(Assignee)

Comment 14

8 years ago
Created attachment 478005 [details] [diff] [review]
Backport patch [1.9.2]
(Assignee)

Comment 15

8 years ago
(In reply to comment #14)
> Created attachment 478005 [details] [diff] [review]
> Backport patch

Landed on mozilla1.9.2:
http://hg.mozilla.org/qa/mozmill-tests/rev/56f6915532a4
(Assignee)

Comment 16

8 years ago
Created attachment 478015 [details] [diff] [review]
Backport patch [1.9.1]
(Assignee)

Updated

8 years ago
Attachment #478005 - Attachment description: Backport patch → Backport patch [1.9.2]
(Assignee)

Comment 17

8 years ago
(In reply to comment #16)
> Created attachment 478015 [details] [diff] [review]
> Backport patch [1.9.1]

Landed on mozilla1.9.1:
http://hg.mozilla.org/qa/mozmill-tests/rev/0a69eedf2cad
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
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: Mozmill Tests → Mozmill Tests
Product: Testing → Mozilla QA
You need to log in before you can comment on or make changes to this bug.