760013 - Split testSuggestHistoryBookmarks in 2 tests

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Remus Pop (:RemusPop)]

testSuggestHistoryBookmarks contains 2 test functions so we'll need to split it into 2 files.

Comment 1

[Remus Pop (:RemusPop)]

This will block bug 742550. So first we will split the tests and then do the fixes in bug 742550.

Comment 2

[Remus Pop (:RemusPop)]

Attachment: patch v1(default)

Splitted into 2 tests and updated the manifest. testSuggestBookmarks has added content from testSuggestHistory so it meets the requirements.

Comment 3

[Remus Pop (:RemusPop)]

Attachment: patch v1(aurora, beta, release)

Manifest file has been removed so it applies on specified branches.

Comment 4

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

Comment on attachment 628678 [details] [diff] [review]
patch v1(default)

+++ b/tests/functional/testAwesomeBar/testSuggestBookmarks.js

>+const TIMEOUT = 5000;

Please get rid of this unused constant.

>+  // We must open the blank page so the autocomplete result isn't "Swith to tab"

nit: 'doesn't show "Switch to tab"'

>+++ b/tests/functional/testAwesomeBar/testSuggestHistory.js

>+/**
>+ * Check history and bookmarked (done in testStarInAutocomplete()) items appear in autocomplete list.
>+ */
>+var testSuggestHistoryAndBookmarks = function() {

We do not test bookmarks here.

>+  // Wait for 4 seconds to work around Firefox LAZY ADD of items to the DB
>+  controller.sleep(4000);

Don't we need this for the other test? I would think so.
 
>+  // Define the path to the first auto-complete result
>+  var richlistItem = locationBar.autoCompleteResults.getResult(0);
>+
>+  // Get the visible results from the autocomplete list. Verify it is 1
>+  controller.waitFor(function () {
>+    return locationBar.autoCompleteResults.isOpened;
>+  }, "Autocomplete list has been opened");

When you change the order of those lines of code in the other test, please do the same here. I would like to have it consistent. If that's part of the fix for the failure then revert those lines in the other file.

Comment 5

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

Comment on attachment 628680 [details] [diff] [review]
patch v1(aurora, beta, release)

As mentioned before, please wait with backport patches until the one for default has been fully reviewed. Marking as obsolete.

Comment 6

[Remus Pop (:RemusPop)]

(In reply to Henrik Skupin (:whimboo) from comment #4)
> Comment on attachment 628678 [details] [diff] [review]
> >+  // Wait for 4 seconds to work around Firefox LAZY ADD of items to the DB
> >+  controller.sleep(4000);
> 
> Don't we need this for the other test? I would think so.

It works for me for the other test.
 
> >+  // Define the path to the first auto-complete result
> >+  var richlistItem = locationBar.autoCompleteResults.getResult(0);
> >+
> >+  // Get the visible results from the autocomplete list. Verify it is 1
> >+  controller.waitFor(function () {
> >+    return locationBar.autoCompleteResults.isOpened;
> >+  }, "Autocomplete list has been opened");
> 
> When you change the order of those lines of code in the other test, please
> do the same here. I would like to have it consistent. If that's part of the
> fix for the failure then revert those lines in the other file.

If I wouldn't change the order in testSuggestBookmarks, it would gracefully fail. It is a part of the fix in testSuggestHistory, but to have consistency I will change it in testSuggestBookmarks.

Comment 7

[Remus Pop (:RemusPop)]

Attachment: Patch v2 (default) [checked-in]

Addressed all requests.

Comment 8

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

Landed on default:
http://hg.mozilla.org/qa/mozmill-tests/rev/d4391abf9f5f

If no new regressions have been introduced please backport the patch to older branches tomorrow.

Comment 9

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

Comment on attachment 628720 [details] [diff] [review]
Patch v2 (default) [checked-in]

Remus, please come up with a patch for older branches so that we can backport the split. Thanks.

Comment 10

[Remus Pop (:RemusPop)]

Attachment: patch v2 (aurora-beta-release) [checked-in]

Applies cleanly on all branches, but not esr10.

Comment 11

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

Comment on attachment 629169 [details] [diff] [review]
patch v2 (aurora-beta-release) [checked-in]

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/61a2c4e41512 (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/f182be061b3a (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/bb632d9e9e60 (release)

Comment 12

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

Please check why it doesn't cleanly apply on esr10, and come up with another patch for that branch.

Comment 13

[Remus Pop (:RemusPop)]

Attachment: patch v2 (esr10) [checked-in]

Updated to work on Firefox ESR10.

Comment 14

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

Landed on esr:
http://hg.mozilla.org/qa/mozmill-tests/rev/8d8247beaa6d

Comment 15

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

Remus, please also update the appropriate litmus tests.

Comment 16

[Remus Pop (:RemusPop)]

Cc-ing Vlad G as he is the owner of AwesomeBar tests.
Vlad, we need the litmus test splitted in two for all branches, including esr10.
For reference purposes here is the link to the test in esr10:
https://litmus.mozilla.org/show_test.cgi?id=41811

Comment 17

[Vlad [QA]]

Hi Henrik.
I've split the test in to two separate tests and you can see them here:
https://litmus.mozilla.org/show_test.cgi?id=41811
https://litmus.mozilla.org/show_test.cgi?id=63749

Please tell me I have to make more modifications. 
I've edited the original test and added a new one, that is why the numbers aren't consecutive.

Comment 18

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

Vlad, thanks for doing that. Here some notes:

* I don't think we should continue to mention a fresh profile as default action. Instead the tester should use the 'Clear Recent History' dialog and remove all browsing and history data. We should mention the data loss and offer the new profile as option.

* Both tests have a spelling error: 'highlated'

* Those tests will also have to land for the Firefox 13, 14, and Aurora branch on Litmus. I don't think we should care about Firefox 12.

Comment 19

[Vlad [QA]]

I've modify a little bit the tests and also I've cloned them on Firefox 13, 14 ,Aurora branch.

The testcase ID are: 66525,66526 on Firefox 13
                     66527,66528 on Firefox 14
                     66529,66530 on Aurora

Comment 20

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

Tests are not synchronous and have different content in how to reset the bookmarks and history. I think it is not enough to say 'Start Firefox 'Clear Recent History' from Menu - Tools.' only but also to specify what has to be done there. Please contact me on IRC so we can sort this out.

Comment 21

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

Vlad, we are still waiting for a response on updating the litmus tests. Can you please give us a reply?

Comment 22

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

Vlad, can you please make sure the Litmus test is updated? Otherwise this request will be obsolete soon when we fully switch over to moztrap.

Comment 23

[Vlad [QA]]

Hi Henrik. 
I've modify the tests in litmus. Please take a look and tell me what you think.
Thanks

Comment 24

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

(In reply to Vlad [QA] from comment #23)
> I've modify the tests in litmus. Please take a look and tell me what you
> think.

Which tests are those? It would be very helpful if you could point out links to all of them. Thanks.

Comment 25

[Vlad [QA]]

These are the split testcases:
https://litmus.mozilla.org/show_test.cgi?id=41811
https://litmus.mozilla.org/show_test.cgi?id=63749

(In reply to Henrik Skupin (:whimboo) from comment #24)
> (In reply to Vlad [QA] from comment #23)
> > I've modify the tests in litmus. Please take a look and tell me what you
> > think.
> 
> Which tests are those? It would be very helpful if you could point out links
> to all of them. Thanks.

Comment 26

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

Looks fine. Thanks Vlad.