887557 - use a dynamic port in dom xpcshell tests so they can be run in parallel

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Comment 1

[Mihnea Dobrescu-Balaur (:mihneadb)]

Attachment: use -1 as httpd port

Comment 2

[Mihnea Dobrescu-Balaur (:mihneadb)]

Attachment: use -1 as httpd port

changing to server.identity.primaryPort

Comment 3

[Mihnea Dobrescu-Balaur (:mihneadb)]

Comment on attachment 768998 [details] [diff] [review]
use -1 as httpd port

Not sure about the *_wrap test, that URL isn't used at all, maybe I should remove it?

Comment 4

[Josh Matthews [:jdm]]

The URL is being used in the _wrap test; we can't set prefs in content processes, and we run test_geolocation_timeout.js in the content process.

Comment 5

[Mihnea Dobrescu-Balaur (:mihneadb)]

I'm not sure I follow. What would be the correct way of fixing that test?

Comment 6

[Josh Matthews [:jdm]]

Start the server in the chrome process in the _wrap test, where you can obtain the dynamic port number. There will be duplicated code, but so it goes.

Comment 7

[(not currently active) Ted Mielczarek]

Comment on attachment 768998 [details] [diff] [review]
use -1 as httpd port

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

::: dom/tests/unit/test_geolocation_provider.js
@@ +74,4 @@
>  
>      var prefs = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch);
> +    prefs.setCharPref("geo.wifi.uri", "http://localhost:" +
> +                      httpserver.identity.primaryPort + "/geo");

I wonder if we should add a convenience method to httpd.js to get a URL out of it with the port in use, like we did for mozhttpd:
http://mozbase.readthedocs.org/en/latest/mozhttpd.html#mozhttpd.MozHttpd.get_url

::: dom/tests/unit/test_geolocation_timeout_wrap.js
@@ +5,5 @@
>    var prefs = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch);
>    prefs.setBoolPref("geo.wifi.scan", false);
> +  prefs.setCharPref("geo.wifi.uri", "http://localhost:4444/geo");
> +  // ^^^ needs the port in the test_geolocation_timeout.js that's being set
> +  // dynamically; it gets re-set in there so it is fine for now

So jdm is saying that you should start a server here, and put the server-starting logic in test_geolocation_timeout.js inside the processType check so that you only start a server when it's not being run as a child process.

Comment 8

[Mihnea Dobrescu-Balaur (:mihneadb)]

Attachment: use a dynamic port for httpd.js

Comment 9

[Josh Matthews [:jdm]]

Comment on attachment 772812 [details] [diff] [review]
use a dynamic port for httpd.js

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

::: dom/tests/unit/test_geolocation_timeout_wrap.js
@@ +7,5 @@
>  function run_test() {
>    var prefs = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch);
>    prefs.setBoolPref("geo.wifi.scan", false);
> +
> +  var httpserver = new HttpServer();

Make this a global.

Comment 10

[Mihnea Dobrescu-Balaur (:mihneadb)]

try run: https://tbpl.mozilla.org/?tree=Try&rev=b9ab7fd28b73

Comment 11

[Mihnea Dobrescu-Balaur (:mihneadb)]

Attachment: use a dynamic port for httpd.js

Comment 12

[Mihnea Dobrescu-Balaur (:mihneadb)]

Comment on attachment 773051 [details] [diff] [review]
use a dynamic port for httpd.js

keeping r+

Comment 13

[Mihnea Dobrescu-Balaur (:mihneadb)]

Isolated (this patch only) try run here: https://tbpl.mozilla.org/?tree=Try&rev=ec7816140100

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d4b4c27078e1

Comment 15

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d4b4c27078e1