761984 - Failure in testFocusAndSearch | Disconnect Error: Application unexpectedly closed

Status: RESOLVED FIXED

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

Description

[Remus Pop (:RemusPop)]

This seems to be an intermittent issue appearing on Nightly and Aurora. Windows and Linux are affected for now.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

It's more and more happening now. Please skip this test for now asap.

Comment 2

[Remus Pop (:RemusPop)]

Attachment: skip test v1 (default, aurora, beta) [checked-in]

Skips the test.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Comment on attachment 630876 [details] [diff] [review]
skip test v1 (default, aurora, beta) [checked-in]

Pushed:
http://hg.mozilla.org/qa/mozmill-tests/rev/5261453b6744 (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/8584638c969f (aurora)

Remus, please disable those litmus tests on the aurora branch.

Comment 4

[Remus Pop (:RemusPop)]

Disabled in Litmus: https://litmus.mozilla.org/show_test.cgi?id=15706

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Remus, are you able to reproduce this problem on your local boxes on any platform? Would you mind to run some tests?

Comment 6

[Remus Pop (:RemusPop)]

I have this testrun from a Ubuntu 12.04 machine:
http://mozmill-crowd.blargon7.com/#/functional/report/87961186c2b807b7747aa7e6d400cb48
No failure.
I'll try to run them on 11.04 and 11.10.

Comment 7

[Remus Pop (:RemusPop)]

This is the report on 11.04:
http://mozmill-crowd.blargon7.com/#/functional/report/87961186c2b807b7747aa7e6d401c3a3

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

This is happening across platforms. So please try out each of those. If you can't reproduce we probably have to try to reproduce on the CI master.

Comment 9

[Remus Pop (:RemusPop)]

No luck in getting a disconnect error.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

This happened yesterday with the next upcoming Firefox 14 beta build:
http://mozmill-ci.blargon7.com/#/functional/report/e67171ea696231bb192f565615000625

Probably try with such a build.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

So we got this failure across all platforms for the Firefox 14.0b8 release candidate. I was running a single testrun on OS X but nothing has been shown. So I get the impression that this is somewhat related to the cpu load. I will trigger an on-demand testrun now so we can check if it happens in such a case.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

No failures when I triggered the ondemand job:
http://mozmill-ondemand.blargon7.com/#/functional/reports?branch=14.0&platform=All&from=2012-06-20&to=2012-06-20

So I'm going to disable the test on beta too:
http://hg.mozilla.org/qa/mozmill-tests/rev/710f4d67b0bb

Remus, please update the Litmus tests if not done automatically yet. Thanks.

Comment 13

[Remus Pop (:RemusPop)]

Disabled Litmus id 64053.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

We cannot disable this one Litmus test but we have to remove the Mozmill group from all of them across branches.

Comment 15

[Maniac Vlad Florin (:vladmaniac)]

(In reply to Henrik Skupin (:whimboo) from comment #14)
> We cannot disable this one Litmus test but we have to remove the Mozmill
> group from all of them across branches.

Removed the Mozmill group from the test across branches: 

https://litmus.mozilla.org/show_test.cgi?id=40804 (esr)
https://litmus.mozilla.org/show_test.cgi?id=55699 (Firefox 13)
https://litmus.mozilla.org/show_test.cgi?id=64053 (Firefox 14)
https://litmus.mozilla.org/show_test.cgi?id=15706 (Aurora)

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Given that we fixed a lot of those application disconnect errors lately, I wonder if we could re-enable this test. Vald, could you try with a loaded system before we backout the skip patch? Thanks.

Comment 17

[Maniac Vlad Florin (:vladmaniac)]

(In reply to Henrik Skupin (:whimboo) from comment #16)
> Given that we fixed a lot of those application disconnect errors lately, I
> wonder if we could re-enable this test. Vald, could you try with a loaded
> system before we backout the skip patch? Thanks.

My plan here is to try with win XP, since this system has a history of giving us the most failures. I'll just flood the VMs with extra tasks and come back with the results shortly

Comment 18

[Maniac Vlad Florin (:vladmaniac)]

So I do not see this test failing again: 
Ran on Ubuntu 11.04 and a win XP heavy loaded VM with both mozmill 1.5.13 and mozmill 1.5.15 

http://mozmill-crowd.blargon7.com/#/functional/report/89726f6b98208a209e7ce2df1035edda
http://mozmill-crowd.blargon7.com/#/functional/report/89726f6b98208a209e7ce2df10359e32

I think we can re-enalbe the test

Comment 19

[Maniac Vlad Florin (:vladmaniac)]

Attachment: re enable test patch 1.0

Re enables the test

Comment 20

[Maniac Vlad Florin (:vladmaniac)]

Comment on attachment 644272 [details] [diff] [review]
re enable test patch 1.0

adding Dave

Comment 21

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

Comment on attachment 644272 [details] [diff] [review]
re enable test patch 1.0

Patch was not necessary - the original patch to disable the test has been backed out:
http://hg.mozilla.org/qa/mozmill-tests/rev/36c5cf88e466 (default)

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Relanded skip patch because it's failing across all platforms:

http://hg.mozilla.org/qa/mozmill-tests/rev/31f2e3267ae6

Comment 23

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

Thanks Henrik. What's the next action for this if it can't be replicated locally?

Comment 24

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

The first thing we have to determine is which search engine is used and which web page gets opened when the test gets run on our box in MV. We do not use a local page yet, which could fix it at all.

Comment 25

[Maniac Vlad Florin (:vladmaniac)]

(In reply to Henrik Skupin (:whimboo) [away 07/27 - 08/05] from comment #24)
> The first thing we have to determine is which search engine is used and
> which web page gets opened when the test gets run on our box in MV. We do
> not use a local page yet, which could fix it at all.

I can't say about the box in MV, but the test uses Google search engine and opens a google search page. We might want to use a custom search engine but I think that implies installing it first in the setupModule as a test prerequisite.

Comment 26

[Maniac Vlad Florin (:vladmaniac)]

The local test page which installs a custom search engine is http://mozqa.com/data/firefox/search/opensearch.html

Comment 27

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

Yes, as Henrik points out, we need to switch to using a local page. The custom search engine mentioned in comment 26 is not local but certainly an improvement over Google. Please work on a patch, which ideally would add a search engine via a backend API based on a local page, or failing that, uses the hosted litmus-data page.

Comment 28

[Maniac Vlad Florin (:vladmaniac)]

(In reply to Dave Hunt (:davehunt) from comment #27)
> Yes, as Henrik points out, we need to switch to using a local page. The
> custom search engine mentioned in comment 26 is not local but certainly an
> improvement over Google. Please work on a patch, which ideally would add a
> search engine via a backend API based on a local page, or failing that, uses
> the hosted litmus-data page.

Dave, the page is available also in our /data folder under mozmill-tests
This makes it local so I am going forward with the idea from comment 25, comment 26 and comment 27 and come up with a patch

Comment 29

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

(In reply to Maniac Vlad Florin (:vladmaniac) from comment #28)
> Dave, the page is available also in our /data folder under mozmill-tests
> This makes it local so I am going forward with the idea from comment 25,
> comment 26 and comment 27 and come up with a patch

The page is local, but the search engine it installs is not - it is hard-coded to hg.mozilla.org.

Comment 30

[Maniac Vlad Florin (:vladmaniac)]

Attachment: patch v1.0

* Updated the test to comply with code standards
* Removed unnecessary data in the test (linked litmus tests) 
* Added code to make the test use a local search engine

Comment 31

[Alex Lakatos[:AlexLakatos]]

Comment on attachment 647958 [details] [diff] [review]
patch v1.0

>+function testClickAndSearch() {
>+  searchBar.selectedEngine = SEARCH_ENGINE_NAME;
This should be done in the setupModule.

>+function testShortcutAndSearch() {
>+  searchBar.selectedEngine = SEARCH_ENGINE_NAME;
This should be done in the setupModule.

> /**
>- * Map test functions to litmus tests
>+ * Install a local search engine
>  */
>-// testClickAndSearch.meta = {litmusids : [8241]};
>-// testShortcutAndSearch.meta = {litmusids : [8242]};
>+function installSearchEngine() {
>+  controller.open(LOCAL_TEST_PAGE);
>+  controller.waitForPageLoad();
> 
>-setupModule.__force_skip__ = "Bug 761984 - Failure in testFocusAndSearch |" +
>-                             "Disconnect Error: Application unexpectedly closed";
>+  var addEngine = new elementslib.Name(controller.tabs.activeTab, "add");
>+  var md = new modalDialog.modalDialog(controller.window);
>+  
>+  md.start(handleSearchInstall);
>+  controller.click(addEngine);
>+  md.waitForDialog(TIMEOUT_INSTALL_DIALOG);
>+}
This does not need a separate function. Move the code to the setupModule. But first try and do this using back-end code and not the ui.

Comment 32

[Maniac Vlad Florin (:vladmaniac)]

Attachment: patch v1.1

* using backend API to install a search engine 
* opening a local test page to get access to search engine URI
* using a litmus data search engine rather than Google 
* test is modified to comply with our coding standards 
* unnecessary code was killed 
* search.js has a minor update - instantiation of the search service globally to have access of the backend API in tests (see places.js for a working example)

Comment 33

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

Comment on attachment 648303 [details] [diff] [review]
patch v1.1


> /**
>+ *  Instance of the search service to gain access to the search API
>+ *
>+ *  @see http://mxr.mozilla.org/mozilla-central (nsIBrowserSearchService.idl)
>+ */
>+var browserSearchService = Cc["@mozilla.org/browser/search-service;1"]
>+                              .getService(Ci.nsIBrowserSearchService);

This is already used by searchBar, so we should add a method there for adding a search engine via the API.

>+exports.browserSearchService = browserSearchService;

We shouldn't export this.

>+const LOCAL_TEST_FOLDER = collector.addHttpResource('../../../data/');
>+const LOCAL_TEST_PAGE = LOCAL_TEST_FOLDER + 'search/opensearch.html';

The local test page should be the search results (see comments below)

>+  installSearchEngine();

We don't need a separate method here, include this in the setupModule

>+ * Install a local search engine
>  */
>+function installSearchEngine() {
>+  controller.open(LOCAL_TEST_PAGE);
>+  controller.waitForPageLoad();
>+  var engineURL = LOCAL_TEST_PAGE.replace(".html", ".xml");
>+  var type = Components.interfaces.nsISearchEngine.DATA_XML;
>+  
>+  // Adding empty string instead of an icon url
>+  search.browserSearchService.addEngine(engineURL, type, "", false);

This still uses the opensearch.xml, which is hardcoded to point to hg.mozilla.org. Please add a method to searchBar for adding an engine using the addEngineWithDetails API call. See https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIBrowserSearchService#addEngineWithDetails%28%29 for details. I have successfully tested this locally. You will also need to set the current search engine.

Comment 34

[Maniac Vlad Florin (:vladmaniac)]

Attachment: patch v1.2

* we now have a local search engine
* corrected a mistake in search.js module

Comment 35

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

Comment on attachment 648669 [details] [diff] [review]
patch v1.2

>     var domainRegex = /[^\.]+\.([^\.]+)\..+$/gi;
>     var targetDomainName = targetUrl.host.replace(domainRegex, "$1");
>-    var currentDomainName = currentUrl.host.replace(domainRegex, "$1");
>+    var currentDomainName = currentUrl.hostname.replace(domainRegex, "$1");

Could you explain the reason for this change, and why it would not apply to targetDomainName also?

>   /**
>+   * Install search engine (API call)
>+   * 
>+   * @param {string} aName
>+   *         The search engine's name. Must be unique. Must not be null.
>+   *
>+   * @param {string} aIconURL
>+   *        Optional: A URL string pointing to the icon to be used to represent
>+   *        the engine.
>+   *
>+   * @param {string} aAlias
>+   *        Optional: A unique shortcut that can be used to retrieve the
>+   *        search engine.
>+   *
>+   * @param {string} aDescription
>+   *        Optional: a description of the search engine.
>+   *
>+   * @param aMethod
>+   *        The HTTP request method used when submitting a search query.
>+   *        Must be a case insensitive value of either "get" or "post".
>+   *
>+   * @param {string} aURL
>+   *        The URL to which search queries should be sent.
>+   *        Must not be null.
>+   * @param {boolean} selected
>+   *        If true select the newly installed search engine
>+   *        Must be set to either 'true' or 'false'
>+   */

I don't think we need to map all the arguments. I would omit the optional arguments, and may also be tempted to make the request method optional and default to "get" to simplify the method call for our tests.

>+  searchBar.installEngine(ENGINE.name, null, null, null, 
>+                          "GET", ENGINE.url, true);

I think we can just use the engine name and URL constant directly here. There's no need to create an ENGINE object.

Comment 36

[Maniac Vlad Florin (:vladmaniac)]

Attachment: patch v1.3

* Dave - to answer your question: whomever added that check in search.js assumed that all time the host == hostname. It can happen but not always. host may also contain ports or other data, not just the name, for example you can have 

   http://localhost as hostname
   http://localhost:43336 as host 

The other item checked can be both host or hostname

* other change requests are addressed 
* refactor installEngine method accordingly

Comment 37

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Comment on attachment 649246 [details] [diff] [review]
patch v1.3

>     var targetDomainName = targetUrl.host.replace(domainRegex, "$1");
>-    var currentDomainName = currentUrl.host.replace(domainRegex, "$1");
>+    var currentDomainName = currentUrl.hostname.replace(domainRegex, "$1");

I agree with Dave. Lets be consistent here.

>+   * Install search engine (API call)
>+   */
>+  installEngine: function searchBar_installEngine(aName, aURL, aSpec) {

I wish we wouldn't mix up back-end and ui code in the same class. But right now there is nothing else we can do until the big refactoring. So it's fine.

>+    if (selected)
>+      aEngine = this._bss.getEngineByName(aName);
>+      this._bss.currentEngine = aEngine;    

Missing brackets.

>+function testClickAndSearch() {
>   searchBar.focus({type: "click"});
>   searchBar.search({text: "Firefox", action: "returnKey"});
> }
> 
> /**
>  * Use the keyboard shortcut to focus the search bar and start a search
>  */
>-var testShortcutAndSearch = function()
>-{
>+function testShortcutAndSearch() {
>   searchBar.focus({type: "shortcut"});
>   searchBar.search({text: "Mozilla", action: "goButton"});
> }

Right now we have two test functions in this module. Please make sure that we separate them into two modules. This can be a follow-up bug.

>-/**
>- * Map test functions to litmus tests
>- */
>-// testClickAndSearch.meta = {litmusids : [8241]};
>-// testShortcutAndSearch.meta = {litmusids : [8242]};

Please leave this in for existent tests. It would at least help us to find appropriate even outdated litmus tests.

Comment 38

[Maniac Vlad Florin (:vladmaniac)]

(In reply to Henrik Skupin (:whimboo) from comment #37)
> Comment on attachment 649246 [details] [diff] [review]
> patch v1.3
> 
> >     var targetDomainName = targetUrl.host.replace(domainRegex, "$1");
> >-    var currentDomainName = currentUrl.host.replace(domainRegex, "$1");
> >+    var currentDomainName = currentUrl.hostname.replace(domainRegex, "$1");
> 
> I agree with Dave. Lets be consistent here.

Sorry I know I've said we can do this but looking further targetUrl is a URI which does not have a hostname. In this case, URI.host behaves exactly like location.hostname (you can test this easy in the web console). More, we cannot cannot change currentUrl.hostname back to currentUrl.host because of the reason explained in comment 36. So I'm afraid we'll have to live with this

Comment 39

[Maniac Vlad Florin (:vladmaniac)]

Attachment: patch v1.4

* changes addressed except replacing targetUrl.host with targetUrl.hostname which as previously said, does not apply

Comment 40

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Comment on attachment 649638 [details] [diff] [review]
patch v1.4

(In reply to Maniac Vlad Florin (:vladmaniac) from comment #38)
> (In reply to Henrik Skupin (:whimboo) from comment #37)
> > Comment on attachment 649246 [details] [diff] [review]
> > patch v1.3
> > >     var targetUrl = this._bss.currentEngine.getSubmission(searchTerm, null).uri;
> > 
> > >     var targetDomainName = targetUrl.host.replace(domainRegex, "$1");
> > >-    var currentDomainName = currentUrl.host.replace(domainRegex, "$1");
> > >+    var currentDomainName = currentUrl.hostname.replace(domainRegex, "$1");
> > 
> > I agree with Dave. Lets be consistent here.
> 
> Sorry I know I've said we can do this but looking further targetUrl is a URI
> which does not have a hostname. In this case, URI.host behaves exactly like
> location.hostname (you can test this easy in the web console). More, we
> cannot cannot change currentUrl.hostname back to currentUrl.host because of
> the reason explained in comment 36. So I'm afraid we'll have to live with
> this.

No, we do not want to live with this. First the variable is called Url which would also be wrong, but as said we should be consistent. So please check how to get the url from a nsIURI instance and use it so we can use the hostname.

> 
>     this._controller.assert(function () {
>       return currentDomainName === targetDomainName;
>     }, "Current domain name matches target domain name - got '" +
>       currentDomainName + "', expected '" + targetDomainName + "'");
> 
>     // Check if search term is listed in URL
>     this._controller.assert(function () {

At the same time please replace those this._controller.assert() calls so we make use of the assertion module.

Comment 41

[Maniac Vlad Florin (:vladmaniac)]

> 
> No, we do not want to live with this. First the variable is called Url which
> would also be wrong, but as said we should be consistent. So please check
> how to get the url from a nsIURI instance and use it so we can use the
> hostname.

Variable called Url - that is indeed wrong 
Getting the Url is possible, here is the code snippet 

var targetUri = this._bss.currentEngine.getSubmission(searchTerm, null).uri;
var targetUrl = targetUri.QueryInterface(Components.interfaces.nsIURL).spec;

On the other hand, "hostname" is a property from nsIDOMLocation
It is not possible to reach nsIDOMLocation from nsIURI, therefore targetUrl can never 
have "hostname". 

If we still want to be consistent, I can just convert currentUrl to a uri, and then access "host" property for both objects instead of "hostname". Please let me know if this meets your expectations so I can upload a patch

Comment 42

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Please read my comment 40 closely. We do not want to use nsIURI for our checks but the url, so that hostname can be used.

Comment 43

[Maniac Vlad Florin (:vladmaniac)]

Attachment: patch v1.5

Adding this patch based on the clarifications on iRC on both sides

Comment 44

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Comment on attachment 650478 [details] [diff] [review]
patch v1.5

>-    this._controller.assert(function () {
>-      return currentDomainName === targetDomainName;
>-    }, "Current domain name matches target domain name - got '" +
>-      currentDomainName + "', expected '" + targetDomainName + "'");
>+    assert.equal(currentDomainName, targetDomainName, 
>+                 "Current domain name matches target domain name");
[..]
>+    assert.ok(currentUrl.href.toLowerCase().indexOf(searchTerm.toLowerCase()) != -1, 
>+              "Current URL contains the search term '" + 
>+              searchTerm.toLowerCase() + "'");

I'm fairly sure that both should be expect.* calls. Tests should still continue and not be aborted.

Otherwise looks fine now.

Comment 45

[Maniac Vlad Florin (:vladmaniac)]

Attachment: patch v1.6 [checked-in]

fixed to use expect calls

Comment 46

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Comment on attachment 650872 [details] [diff] [review]
patch v1.6 [checked-in]

Looks good. Lets see how it works with todays testrun.

Comment 47

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

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

Comment 48

[Maniac Vlad Florin (:vladmaniac)]

Attachment: update manifest v1.0 [checked-in]

* This patch updates the manifest file correctly. Sorry I missed it in the previous versions

Comment 49

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Comment on attachment 651340 [details] [diff] [review]
update manifest v1.0 [checked-in]

Please come up with a follow-up patch we can use for backporting.

Comment 50

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Follow-up landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/3aa79883fb2c (default)

Comment 51

[Maniac Vlad Florin (:vladmaniac)]

Attachment: backport for aurora, beta, release

* patch for backporting which also includes the updated manifest file

Comment 52

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Comment on attachment 651369 [details] [diff] [review]
backport for aurora, beta, release

r+ only because the code is doing what I shall do. Otherwise I would have given a r- because there are still a dozen of whitespace failures in the patch. Sadly those are too in the version for default which already has been landed. To keep all versions identical I will push the patch with the same whitespace mistakes to the other branches.

We have called out issues like those more than a dozen of times. I want to stop on those. So please get those really fixed on your side.

Comment 53

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Comment on attachment 651369 [details] [diff] [review]
backport for aurora, beta, release

This patch does not apply to the esr branch. Have you tested it?

Comment 54

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

http://hg.mozilla.org/qa/mozmill-tests/rev/f320a0d03162 (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/692010c8f43d (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/dd7f7c2329d3 (release)

Once it has been landed on esr10 we have to update the litmus tests.

Comment 55

[Maniac Vlad Florin (:vladmaniac)]

Attachment: backport for esr

* This should apply cleanly now on esr

Comment 56

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

http://hg.mozilla.org/qa/mozmill-tests/rev/56cb661b2b4d

Comment 57

[Maniac Vlad Florin (:vladmaniac)]

Some majority of the litmus tests were not updated in the first place when this test was skipped, so will remain the same in conclusion. 

There were some updates made for: 
https://litmus.mozilla.org/show_test.cgi?id=15706
https://litmus.mozilla.org/show_test.cgi?id=40804
https://litmus.mozilla.org/show_test.cgi?id=64053
to point to the mozmill subgroup

With this, in-litmus+

Comment 58

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

This only enables the test for the keyboard shortcut but not for the mouse focus one. Litmus updates aren't done yet.

Comment 59

[Maniac Vlad Florin (:vladmaniac)]

(In reply to Henrik Skupin (:whimboo) from comment #58)
> This only enables the test for the keyboard shortcut but not for the mouse
> focus one. Litmus updates aren't done yet.

The mouse focus one is from BTS, the shortcut one is from FFT. They were not updated correctly in the first place, that's why its kinda messy. 

When I checked, the mouse focus one were ok. Did you check and got other results? 
As I said in comment 57, some were not updated when the test was skipped so now remain the same. 

Still, if the mouse focus test appears not up to date to you, please paste me a link.

Comment 60

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Vlad, you should always post all links to litmus tests we would have to update, not only the ones you have updated.

Comment 61

[Maniac Vlad Florin (:vladmaniac)]

(In reply to Henrik Skupin (:whimboo) from comment #60)
> Vlad, you should always post all links to litmus tests we would have to
> update, not only the ones you have updated.

Alright then - are any of these policies documented somewhere? It seems I miss them. If so, mind directing me to the appropriate location of those so that in the future these matter would not block us.

Comment 62

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Why policies? I don't see a reason for one. If we fix a failure across branches I also put all the changeset links in the bug just to let others know and they don't have to query. For Dave and me it will simply help a lot to not have to cross-check ourselves what has not been changed.

Comment 63

[Maniac Vlad Florin (:vladmaniac)]

(In reply to Henrik Skupin (:whimboo) from comment #62)
> Why policies? I don't see a reason for one. If we fix a failure across
> branches I also put all the changeset links in the bug just to let others
> know and they don't have to query. For Dave and me it will simply help a lot
> to not have to cross-check ourselves what has not been changed.

It would be good to have our workflow better documented somewhere so that I don't have to guess all the time which would be the good practice, that's what I meant.

Comment 64

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

Vlad, the Litmus request is still open. Please get the tests updated.