779127 - Mozmill test for Switch to Tab feature

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect, P2)

Description

[Andreea Matei [:AndreeaMatei]]

This is the tracking bug for a new mozmill test for switching tabs
https://moztrap.mozilla.org/manage/case/327/

It is going to follow the steps in MozTrap: 
1. Open three new tabs in Firefox
2. Navigate to three different pages 
3. Focus the location bar
4. Type a word from the title of one of the other pages into the location bar and select the item with Switch to Tab
5. Verify the switch is made and the page is not loaded again

Comment 1

[u279076]

I was trying to give this test to a volunteer contributor. Would it be possible for you to take something else?

Comment 2

[Andreea Matei [:AndreeaMatei]]

Sure, no problem.

Comment 3

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

(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #1)
> I was trying to give this test to a volunteer contributor. Would it be
> possible for you to take something else?

This is obsolete now. Andreea, if you don't mind you can continue to work on this test.

Comment 4

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v1 (all branches)

Followed moztrap steps. 
For the tricky part which is selecting the "Swith to tab" item from the results list, I've looked for the element with the "actiontype" attribute (it's value is 'switchtab'). After switching, we verify the location bar contains the string typed before.
In the end, we check that there are only the 3 pages opened from the start.

Comment 5

[Alex Lakatos[:AlexLakatos]]

Comment on attachment 649285 [details] [diff] [review]
patch v1 (all branches)

>diff --git a/tests/functional/testAwesomeBar/testSuggestBookmarks.js b/tests/functional/testAwesomeBar/testSuggestBookmarks.js
Why are you changing testSuggestBookmarks.js in this bug? If you need to edit it, open a new bug.

>+function testSwitchToTabs() {
>+  for each(var page in LOCAL_TEST_PAGES) {
>+    controller.open(page.url);
>+    controller.waitForPageLoad();
>+    tabBrowser.openTab();
>+  }
>+
>+  tabBrowser.closeTab();
I don't like this here. There should be a more elegant way to do it.

>+  for each(page in LOCAL_TEST_PAGES) {
>+		// Type the strings from LOCAL_TEST_PAGES in the location bar
>+		locationBar.focus({type: "shortcut"});
>+		locationBar.type(page.string);
The indentation is way off here. Please correct it.

>+    // Go through all results and click the one with 'actiontype' attribute
>+    for(var index = 0; index < autoCompleteLength; index++) {
>+      var richlistItem = locationBar.autoCompleteResults.getResult(index);
You can use i here instead of index. Saves space.

>+        expect.ok(locationBar.contains(page.string), 
>+				          "Location bar should contain the page string");
>+			}
The indentation is way off here. Please correct it.

Comment 6

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v1.1 (all branches)

Sorry about that, I've changed the settings for my editor.
Added an if for the issue with last tab to close.

Comment 7

[Alex Lakatos[:AlexLakatos]]

Comment on attachment 649595 [details] [diff] [review]
patch v1.1 (all branches)

>+const LOCAL_TEST_PAGES = [
>+   {url: LOCAL_TEST_FOLDER + 'mozilla_community.html', string: "community"},
>+   {url: LOCAL_TEST_FOLDER + 'mozilla_contribute.html', string: "contribute"},
>+   {url: LOCAL_TEST_FOLDER + 'mozilla_governance.html', string: "governance"}
You have three spaces here. Please have 2, as the style guide indicates

>+/*
>+ * Test Switch to Tab feature 
>+ */
>+function testSwitchToTab() {
>+  for(var i = 0; i < LOCAL_TEST_PAGES.length; i++) {
>+    controller.open(LOCAL_TEST_PAGES[i].url);
>+    controller.waitForPageLoad();
>+
>+    if(i != 2) 
>+      tabBrowser.openTab();
>+  }
I'd rather have here 
  for(var i = 0; i < LOCAL_TEST_PAGES.length; i++) {
    if(i > 0) {
      tabBrowser.openTab();
    }
    controller.open(LOCAL_TEST_PAGES[i].url);
    controller.waitForPageLoad();
for consistency with http://hg.mozilla.org/qa/mozmill-tests/file/c9cc7d5a8269/tests/endurance/testBookmarks_OpenAndClose/test1.js#l45

>+        expect.ok(locationBar.contains(page.string), 
>+                  "Location bar should contain the page string");
You should add a check here to see if the current tab has the right index.

Comment 8

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v2 (all branches)

Addressed all requests.
Added a check for the current selected index and also edited manifest.ini with the moztrap_case.

Comment 9

[Alex Lakatos[:AlexLakatos]]

Comment on attachment 650026 [details] [diff] [review]
patch v2 (all branches)

(In reply to Andreea Matei [:AndreeaMatei] from comment #8)
> Created attachment 650026 [details] [diff] [review]
> patch v2 (all branches)
> 
> Addressed all requests.
> Added a check for the current selected index and also edited manifest.ini
> with the moztrap_case.
That got reverted in bug 757916. Please have it at the end of the file instead.

Comment 10

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v2.1 (all branches)

Added moztrap id at the end of the test.

Comment 11

[Alex Lakatos[:AlexLakatos]]

Comment on attachment 650150 [details] [diff] [review]
patch v2.1 (all branches)

Looks ok. Over to Dave/Henrik for additional review.

Comment 12

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

Comment on attachment 650150 [details] [diff] [review]
patch v2.1 (all branches)

>+  for(var i = 0; i < LOCAL_TEST_PAGES.length; i++) {

nit: missing spaces after for.

>+    if(i > 0) {
>+      tabBrowser.openTab();
>+    }

Why is that if necessary. It doesn't matter when we open another empty tab. Also I would make use of forEach as we do in other tests.

>+  for (var page = 0; page < LOCAL_TEST_PAGES.length; page++) {

Same as above. forEach please.

>+    // Get the visible results from the autocomplete list
>+    var autoCompleteList = locationBar.autoCompleteResults.getElement({type:"results"});

Can't you make use of autoCompleteResults.visibleResults()? Also this should be put after the isOpened waitFor call.

>+    controller.waitFor(function() {

nit: missing space between function and the opening bracket.

>+    for(i = 0; i < autoCompleteList.getNode().getNumberOfVisibleRows(); i++) {
>+      var richlistItem = locationBar.autoCompleteResults.getResult(i);

We should be able to re-use the elements got from visibleResults().

>+
>+      if(richlistItem.getNode().hasAttribute("actiontype")) {

Does actiontype only indicate the switch-to-tab feature? Which value does it have in this case?

>+        expect.ok(locationBar.contains(LOCAL_TEST_PAGES[page].string), 
>+                  "Location bar should contain the page string");

I wouldn't use contains() here but do a expect.match() call. The thing we will not see any details of the expected and target string. Also before we click we probably should check the auto-complete entry for the string too. 

>+        expect.equal(tabBrowser.selectedIndex, page, 
>+                     "Current tab has the expected index");

I would better assert that the right url is displayed via utils.assertUrlEqual().

>+   // Check the number of pages loaded
>+   expect.equal(tabBrowser.length, LOCAL_TEST_PAGES.length, 
>+                "Only the required test pages have been loaded");

Why do we need that? What else should have opened another tab?

Also, has been done a check which of those tests are already covered by the browser chrome tests?

Comment 13

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v3 (all branches)

I have made all the requested changes.

The if for opening new tab was because a blank tab will remain opened after loading all three pages. And that needs to be closed so we type the first string in the last loaded page.
Changing to forEach, I added tabBrowser.closeTab() to handle this issue.

The "actiontype" attribute is present only on Switch To Tab items and it's value is "switchtab".

Comment 14

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 651752 [details] [diff] [review]
patch v3 (all branches)

>+  tabBrowser = new tabs.tabBrowser(controller);  

Remove trailing whitespace.

>+  for each (var page in LOCAL_TEST_PAGES) {
>+    controller.open(page.url);
>+    controller.waitForPageLoad();
>+    tabBrowser.openTab();
>+  }

Henrik actually suggested using forEach as shown below.

LOCAL_TEST_PAGES.forEach(function (page) {
  controller.open(page.url);
  controller.waitForPageLoad();
  tabBrowser.openTab();
});

https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Global_Objects/Array/forEach

>+  tabBrowser.closeTab();

I think Henrik's point is that we are not concerned with this empty tab being open. It should not adversely affect the test. We are more concerned with such things in endurance tests because we are consciously wanted to accumulate a specific number of entities.

>+  for each (page in LOCAL_TEST_PAGES) {

As above, use forEach.

>+    // Go through all results and click the one with 'actiontype' attribute
>+    for each (var richlistItem in autoCompleteList) {

If this is an array then we can use forEach here too.

>+      if(richlistItem.getNode().hasAttribute("actiontype")) {

Missing space after if.

Also, given your comment that the value is "switchtab", I feel that would be a better indication that this is the correct item.

>+        expect.match(page.string, /[\w+(?!\_)]/.exec(locationBar.value), 
>+                     "Location bar should contain the page string");

Message should be "Location bar contains " + page.string

>+        // Check the right url is displayed
>+        utils.assertLoadedUrlEqual(controller, page.url);

This comment is not needed, the following line of code is descriptive enough.

Comment 15

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

(In reply to Dave Hunt (:davehunt) from comment #14)
> >+        expect.match(page.string, /[\w+(?!\_)]/.exec(locationBar.value), 
> >+                     "Location bar should contain the page string");
> 
> Message should be "Location bar contains " + page.string

That's not necessary. The match() function takes care about. Also why do you run exec on it? If you want to do a full match use equal, otherwise we have an expect.contains() method.

Comment 16

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v3.1

Addressed all requests.
As discussed on IRC, I used expect.contain() instead of expect.match().
Also used forEach and the value of "actiontype" attribute.

Comment 17

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

Comment on attachment 652392 [details] [diff] [review]
patch v3.1

>Bug 779127 - Mozmill test for Switch to Tab feature. r=alex.lakatos, r=dhunt, r=hskupin

Please only reference the official peers of the module as reviewer. You all mostly give feedback to each other.

>+  LOCAL_TEST_PAGES.forEach(function (page) {

Parameters should use an 'a' prefix. So please update all in this test to aPage.

>+    // Check the autocomplete list is open
>+    controller.waitFor(function () {
>+      return locationBar.autoCompleteResults.isOpened;
>+    }, "Autocomplete popup has been opened");

No need for the comment.

>+    // Get the visible results from the autocomplete list
>+    var autoCompleteList = locationBar.autoCompleteResults.visibleResults;
>+
>+    controller.waitFor(function () {
>+      return autoCompleteList.length != 0;
>+    }, "Waiting for autocomplete results to load");

No need for the comment too. Also remove the extra empty line. I think we could fail here on heavily loaded systems when the auto-complete entry isn't shown directly but takes a bit longer. Can you please test it so we can ensure we do not fail?

>+    autoCompleteList.forEach(function (richlistItem) {

'a' prefix too.

>+      var item = locationBar.autoCompleteResults.getUnderlinedText(richlistItem, 
>+                                                                   "title");
>+      if (richlistItem.getNode().getAttribute("actiontype") === "switchtab") {
>+        expect.equal(page.string, item.toString().toLowerCase(), 
>+                     "The page title matches the underlined text");
>+        controller.click(richlistItem);
>+
>+        expect.contain(locationBar.value, page.string,
>+                       "Search string in the location bar");
>+
>+        utils.assertLoadedUrlEqual(controller, page.url);
>+      }
>+    });

So what if switch to tab is not present at all? We have to fail and this doesn't happen right now.

Comment 18

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v4

Updated as requested. 
Sorry about commit message, on that week you were in PTO I had to get review from Vlad and Alex first, then Dave.

I have tested the waitFor with heavy system and it passes, probably is not an issue since is mostly just one result or max two.

Comment 19

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 652762 [details] [diff] [review]
patch v4

>+    // Go through all results and click the one with 'actiontype' attribute

This doesn't reflect the code, but should probably not contain attribute details. I would suggest "...and click 'switch to tab'"

>+      } else {
>+        expect.ok(!aRichlistItem.getNode().hasAttribute("actiontype"), 
>+                  "Expected a 'Switch to tab' item to be present");

This is just checking that the current item does not have the 'actiontype' attribute, and this would likely not cause the test to fail if 'switch to tab' was not present. What we need is probably a boolean variable that is initialised with false and set to true within the if statement. Then we can simply assert that it is true after the forEach.

Comment 20

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v5

Changed the comment, added switchToTab boolean variable and checked it's value after forEach.

Comment 21

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

Comment on attachment 653304 [details] [diff] [review]
patch v5

>+    controller.waitFor(function () {
>+      return locationBar.autoCompleteResults.isOpened;
>+    }, "Autocomplete popup has been opened");
>+
>+    var autoCompleteList = locationBar.autoCompleteResults.visibleResults;
>+    controller.waitFor(function () {
>+      return autoCompleteList.length != 0;
>+    }, "Waiting for autocomplete results to load");

Would we need the first waitFor(), given that we check the visibleResults later? Also I think we can run into intermittent failures with the 2nd waitFor(). Shouldn't exactly one match be displayed in the auto-complete list for the given text?

>+        controller.click(aRichlistItem);
>+
>+        expect.contain(locationBar.value, aPage.string,
>+                       "Search string in the location bar");
>+
>+        utils.assertLoadedUrlEqual(controller, aPage.url);

How fast are we here? Could this cause problems directly checking the URL? If we have a delay assertLoadedUrlEqual would load the wrong page into the currently selected tab. I think that you simple want to run a single except.contains against the full page URL here. The search string is not that necessary here because we know that it will be part of the URL.

>+    expect.ok(switchToTab, "Expected 'Switch to tab' item to be present");

Looks good. But I also think we should skip the loop after we found the first switch to tab entry. There is no need to check the remaining entries.

Comment 22

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v5.1

I have changed the second waitFor to use directly the autoCompleteResults length, unfortunately without the first waitFor the test fails to load the results. Probably it buys a little time for the results to get populated.
Tested with heavily loaded system as well. 

If we really want to get rid of this waitFor, I think we will need a "trick" like in testVisibleItemsMax test where we used a short sleep while typing the string.
We have more than one result because of the Mozilla Firefox bookmark folder which contains "community" page for example.

I had to replace the last forEach with a normal for in order to skip the loop using "break". forEach has to go through all steps, can't skip it.

When used expect.contain against the full URL page, I got a difference of "http://" which our page.url has and locationBar.value doesn't. So instead of locationBar.value we use activeTab.location.href.

Comment 23

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 654150 [details] [diff] [review]
patch v5.1

Review of attachment 654150 [details] [diff] [review]:
-----------------------------------------------------------------

::: tests/functional/testAwesomeBar/testSwitchToTab.js
@@ +39,5 @@
> +
> +  LOCAL_TEST_PAGES.forEach(function (aPage) {
> +    // Type the strings from LOCAL_TEST_PAGES in the location bar
> +    locationBar.focus({type: "shortcut"});
> +    locationBar.type(aPage.string);

Do we need to wait for aPage.string to appear in the locationBar.value as we have with other tests recently?

@@ +41,5 @@
> +    // Type the strings from LOCAL_TEST_PAGES in the location bar
> +    locationBar.focus({type: "shortcut"});
> +    locationBar.type(aPage.string);
> +
> +    controller.waitFor(function () {

I've done a bit of testing without this, and it appears that on all but the first loop it is necessary to wait for the autoCompleteList to open, as autoCompleteResults.length does not reset to 0 after the first loop.

@@ +52,5 @@
> +
> +    // Go through all results and click 'Switch to tab'
> +    var autoCompleteList = locationBar.autoCompleteResults.visibleResults;
> +    var switchToTab = false;
> +    for (i = 0; i < autoCompleteList.length; i++) {

You could use autoCompleteList.some here to stop processing once the callback returns true. See https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Global_Objects/Array/some

Comment 24

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v5

Addressed all requests, also used autocompleteList.some() and it's working as expected.

Comment 25

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 661236 [details] [diff] [review]
patch v5

Review of attachment 661236 [details] [diff] [review]:
-----------------------------------------------------------------

::: tests/functional/testAwesomeBar/testSwitchToTab.js
@@ +63,5 @@
> +                     "The page title matches the underlined text");
> +        controller.click(aRichlistItem);
> +        expect.equal(controller.tabs.activeTab.location.href, aPage.url,
> +                     "Active tab url should equal the page url");
> +      }

In order to stop checking the remaining items you would need to return true here. You could do this by simply returning switchToTab, which is true once the switch tab is found. Otherwise, this patch looks great.

Comment 26

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v5.1

Thanks Dave. Updated accordingly to the last comment.

Comment 27

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v5.2

Corrected the return.

Comment 28

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 662927 [details] [diff] [review]
patch v5.2

Review of attachment 662927 [details] [diff] [review]:
-----------------------------------------------------------------

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/4599129c281e (default)

Comment 29

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

This fails:
http://mozmill-ci.blargon7.com/#/functional/report/fac9ae8444a3cbcc1dcf9cbaff153ec3

 The page title matches the underlined text - 'contribute' should equal 'contribut'
Active tab url should equal the page url - 'http://localhost:43336/mozilla_community.html' should equal 'http://localhost:43336/mozilla_contribute.html' 

Backed out:
http://hg.mozilla.org/qa/mozmill-tests/rev/ffe37160c968

Comment 30

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

This bug is a quarterly goal so please finish it off by end of this week.

Comment 31

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v5.3

When checking the underlined string, I replaced the expect.equal with an assert.waitFor as used in the other tests from awesome bar in bug 792394. 
I believe an assert is more appropriate here instead an expect since from the report we see that after the first failure, it won't click the richlistItem so the next expect will also fail.
Tested multiple times on heavy loaded Mac OS X 10.7.4 and passed internal reviews.

Comment 32

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

Comment on attachment 665466 [details] [diff] [review]
patch v5.3

Review of attachment 665466 [details] [diff] [review]:
-----------------------------------------------------------------

Haven't really given a review here yet, so this test looks fine. There are some other things which slipped through and which we should fix.

::: tests/functional/testAwesomeBar/testSwitchToTab.js
@@ +38,5 @@
> +
> +  LOCAL_TEST_PAGES.forEach(function (aPage) {
> +    locationBar.focus({type: "shortcut"});
> +    locationBar.type(aPage.string);
> +    controller.waitFor(function () {

Please do me a favor and change the controller.waitFor() calls to assert/expect.waitFor().

@@ +54,5 @@
> +    // Go through all results and click 'Switch to tab'
> +    var autoCompleteList = locationBar.autoCompleteResults.visibleResults;
> +    var switchToTab = false;
> +    autoCompleteList.some(function (aRichlistItem) {
> +      var item = locationBar.autoCompleteResults.getUnderlinedText(aRichlistItem,

You should do `var switchToTab = autoCompleteList.some(` to get the result back if at least one item was a switchToTab entry. Btw. good call in using .some() here!

@@ +67,5 @@
> +        expect.equal(controller.tabs.activeTab.location.href, aPage.url,
> +                     "Active tab url should equal the page url");
> +      }
> +
> +      return switchToTab;

There is no need to handle switchToTab in this callback. Save the comparison result from above in a temporary variable and return it at the end.

@@ +70,5 @@
> +
> +      return switchToTab;
> +    });
> +
> +    expect.ok(switchToTab, "Expected 'Switch to tab' item to be present");

why are you making use of 'Expected' here? Please remove it.

Comment 33

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v5.4

Updated as requested. 
Passed internal reviews.

Comment 34

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

Comment on attachment 665884 [details] [diff] [review]
patch v5.4

Review of attachment 665884 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great. Just one more thing before we can land the patch.

::: tests/functional/testAwesomeBar/testSwitchToTab.js
@@ +60,5 @@
> +      if (aRichlistItem.getNode().getAttribute("actiontype") === "switchtab") {
> +        switchTabPresent = true;
> +        assert.waitFor(function () {
> +          return item.toString().toLowerCase() === aPage.string;
> +        }, "The page title matches the underlined text");

How does some() behave if we assert here? Do we completely stop the test or only this iteration? I think that we even don't need the extra variable. We can return true at the end of the if condition or false at the end of the callback.

Comment 35

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Henrik Skupin (:whimboo) from comment #32)
> Haven't really given a review here yet, so this test looks fine. There are
> some other things which slipped through and which we should fix.

Thanks for the additional eyes on this patch - you made some good points.

Comment 36

[Andreea Matei [:AndreeaMatei]]

Did some testing and assert completely stops the test, not just the iteration. But I'm not sure why we'd want to go forward with the rest of the test if we fail there.
We could get rid of the extra variable by returning true at the end of the if condition, since some() needs the callback to return true in order to stop the iteration.

Comment 37

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

(In reply to Andreea Matei [:AndreeaMatei] from comment #36)
> We could get rid of the extra variable by returning true at the end of the
> if condition, since some() needs the callback to return true in order to
> stop the iteration.

Right, that's what I was thinking of. Please make this change and we can get it landed. Thanks!

Comment 38

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v5.5

Updated as requested.

Comment 39

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

Comment on attachment 666471 [details] [diff] [review]
patch v5.5

Review of attachment 666471 [details] [diff] [review]:
-----------------------------------------------------------------

::: tests/functional/testAwesomeBar/testSwitchToTab.js
@@ +63,5 @@
> +
> +        controller.click(aRichlistItem);
> +        expect.equal(controller.tabs.activeTab.location.href, aPage.url,
> +                     "Active tab url should equal the page url");
> +        return true;

It's not enough to return true here. You should always return a value. That means code outside of this if clause has to return false.

Comment 40

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v6 [checked-in]

Returning true inside if and false outside the if clause.

Comment 41

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

Lets hope it sticks this time. We will close as fixed once no failures can be seen in tomorrows testrun.

http://hg.mozilla.org/qa/mozmill-tests/rev/f8fc87bb2cd7 (default)

Comment 42

[Andreea Matei [:AndreeaMatei]]

I believe we might have the same issue like on bug 792394 (which will be fixed next week), only on Mac OS X, because we use underlined here also.

Comment 43

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

Looks like it's stable enough that we can call it fixed now. The remaining issue will be taken care of in bug 792394.

Comment 44

[Andreea Matei [:AndreeaMatei]]

Attachment: [beta, esr10, release] patch v1

Patch to add the test into beta, esr10 and release branches (updated with the timeout for the database).

Comment 45

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

Comment on attachment 673241 [details] [diff] [review]
[beta, esr10, release] patch v1

We don't want this patch yet until it is still failing. Please add the proposed 4s wait and then we can backport a combined patch.

Comment 46

[Andreea Matei [:AndreeaMatei]]

Attachment: Sleep for places v1 [checked-in]

Adding timeout sleep in order to help adding the history items to the database.

Comment 47

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

Comment on attachment 673241 [details] [diff] [review]
[beta, esr10, release] patch v1

We do not want to backport new tests except for endurance tests.

Comment 48

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

Comment on attachment 673588 [details] [diff] [review]
Sleep for places v1 [checked-in]

http://hg.mozilla.org/qa/mozmill-tests/rev/2e61e9030e3d (default)

If that works we want to get this backported to the aurora branch.

Comment 49

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

(In reply to Henrik Skupin (:whimboo) from comment #48)
> http://hg.mozilla.org/qa/mozmill-tests/rev/2e61e9030e3d (default)
> 
> If that works we want to get this backported to the aurora branch.

Given that it makes sense to have this sleep on aurora too, I have transplanted the patch:

http://hg.mozilla.org/qa/mozmill-tests/rev/5bac83f2625c (aurora)