925346 - Test failure "Geolocation position has been found" in /testGeolocation/testShareLocation.js

Status: RESOLVED FIXED

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

Description

[Cosmin Malutan, [:cosmin-malutan]]

This failed twice today on Mac 10.8.5 (x86) with Nightly it and on Windows 8 (x86) with Nightly de.

I tried to reproduce this on the affected machine (mm-win-8-32-3) without success.


http://mozmill-daily.blargon7.com/#/functional/report/6d6f6a58b02eeffc06eafa8beac909b2

Comment 1

[Andrei Eftimie]

It seems this will fail if we don't have WiFi enabled, see bug 923618
But if this would be the case shouldn't we fail *every* time on OSX?

I'll set it as a blocker since a patch is in review and will probably land soon.
We have 1 instance of it failing on Windows, has to be a different reason for it on Windows.

Assigning this to Andreea since she has the most knowledge about geolocation as the author of the test.

Comment 2

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

Yeah, I don't see a hard connection between both. But if it is really related to WIFI why it hasn't been failed earlier? I strongly believe that's another issue here.

Comment 3

[Andreea Matei [:AndreeaMatei]]

I'll run some testruns on a free OS X machine to see if it reproduces at some point.

Comment 4

[Andreea Matei [:AndreeaMatei]]

Good news: Cosmin and I were able to get it reproduced on a Windows 7 VM, once out of 10 testruns. Now I can fully investigate this and get at the bottom of the issue. But first I need to get the metro work done as it has higher priority.

Comment 5

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

Reproduced manually? If that is the case someone should file a bug for Windows. Also which version of Firefox? I wonder if that is the same as reported lately on bug 888252.

Comment 6

[Andreea Matei [:AndreeaMatei]]

Not manually, in testruns only. It was with Nightly, only there we have the test so far. On CI the failures were on Windows 7 and on OS X only. But with OS X I couldn't reproduced, tried about 30 testruns.

Comment 7

[Andreea Matei [:AndreeaMatei]]

Reassigning to Cosmin who wants to work on this and I have metro as a priority at the moment.

Comment 8

[Cosmin Malutan, [:cosmin-malutan]]

Hi, I had a look in to this.
When it fails the actual error is "Position acquisition timed out". It fails at least once in 100 runs. So I will try to find a fix and return with a patch soon.

Comment 9

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: patch_v1.0

I increased the timeout for geolocation call, and for assertion, and I also updated the assertion fail message, so if it will fail in the future we will know the actual reason we did't get the position.

Reports:
http://mozmill-crowd.blargon7.com/#/functional/report/95c9c51e8c2d9f14917fc749dd61ee15
http://mozmill-crowd.blargon7.com/#/functional/report/95c9c51e8c2d9f14917fc749dd61fbb6
http://mozmill-crowd.blargon7.com/#/functional/report/95c9c51e8c2d9f14917fc749dd61e775

Comment 10

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

Comment on attachment 822215 [details] [diff] [review]
patch_v1.0

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

::: data/geolocation/position.html
@@ +16,5 @@
>        result = document.getElementById("result");
>  
>        if (navigator.geolocation)
>          navigator.geolocation.getCurrentPosition(handlePosition, handleError,
> +                                                 {enableHighAccuracy: false, timeout: 30000});

Don't forget to update this file in the testcase-data repository too.

Comment 11

[Andrei Eftimie]

Comment on attachment 822215 [details] [diff] [review]
patch_v1.0

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

Seems reasonable.

Only 2 small nits to fix:

::: tests/functional/testGeolocation/testShareLocation.js
@@ +62,5 @@
> +  try {
> +    assert.waitFor(function () {
> +      return regExp.test(result.getNode().textContent);
> +    }, "", TIMEOUT_POSITION);
> +  } catch (e) {

nit: new line here

@@ +63,5 @@
> +    assert.waitFor(function () {
> +      return regExp.test(result.getNode().textContent);
> +    }, "", TIMEOUT_POSITION);
> +  } catch (e) {
> +    assert.fail("Geolocation position got : '" + result.getNode().textContent + "'");

"Geolocation position is: "

Comment 12

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: patch_v1.1

I updated the patch.
Thanks

Comment 13

[mario garbi]

Happened again today on Windows with Nightly 27 'de':
http://mozmill-daily.blargon7.com/#/functional/report/95c9c51e8c2d9f14917fc749dd86d06f

Comment 14

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 822246 [details] [diff] [review]
patch_v1.1

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

Great! Lets see if this fixes the issue.

Pushed:
http://hg.mozilla.org/qa/mozmill-tests/rev/67600b9f0842 (default)

This will be merged today along with the test in aurora as well. For beta I will backport the test and this fix in a single patch in bug 758187, so we can close this bug now.