889034 - use a dynamic port in places/ xpcshell tests so they can be run in parallel

Status: RESOLVED FIXED

(Toolkit :: Places, defect)

Comment 1

[Mihnea Dobrescu-Balaur (:mihneadb)]

Attachment: use -1 as httpd port

Comment 2

[Marco Bonardo [:mak]]

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

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

::: toolkit/components/places/tests/network/test_history_redirects.js
@@ +28,5 @@
> +});
> +
> +XPCOMUtils.defineLazyGetter(this, "FOUND_URL", function() {
> +  return "http://localhost:" + PORT + FOUND_PATH;
> +});

these don't need to be lazy getters, why not just define const?

const PORT = HTTPSVR.identity.primaryPort;
const PERMA_REDIR_URL = "http://localhost:" + PORT + PERMA_REDIR_PATH;

and so on...

@@ +77,5 @@
>  
>    var chan = NetUtil.ioService
> +                    .newChannelFromURI(uri("http://localhost:" +
> +                                           HTTPSVR.identity.primaryPort +
> +                                           "/permaredir"));

this is just .newChannelFromURI(uri(PERMA_REDIR_URL));

Comment 3

[Mihnea Dobrescu-Balaur (:mihneadb)]

(In reply to Marco Bonardo [:mak] from comment #2)
> Comment on attachment 769796 [details] [diff] [review]
> use -1 as httpd port
> 
> Review of attachment 769796 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/components/places/tests/network/test_history_redirects.js
> @@ +28,5 @@
> > +});
> > +
> > +XPCOMUtils.defineLazyGetter(this, "FOUND_URL", function() {
> > +  return "http://localhost:" + PORT + FOUND_PATH;
> > +});
> 
> these don't need to be lazy getters, why not just define const?
> 
> const PORT = HTTPSVR.identity.primaryPort;
> const PERMA_REDIR_URL = "http://localhost:" + PORT + PERMA_REDIR_PATH;
> 
> and so on...

They have to be lazy getters because the server is not started at that time. If you want I could take out the start call from the run_test function and put it at the top of the file and then I can do what you suggested. Is that ok?

> 
> @@ +77,5 @@
> >  
> >    var chan = NetUtil.ioService
> > +                    .newChannelFromURI(uri("http://localhost:" +
> > +                                           HTTPSVR.identity.primaryPort +
> > +                                           "/permaredir"));
> 
> this is just .newChannelFromURI(uri(PERMA_REDIR_URL));

Right, thanks!

Comment 4

[Marco Bonardo [:mak]]

(In reply to Mihnea Dobrescu-Balaur (:mihneadb) from comment #3)
> (In reply to Marco Bonardo [:mak] from comment #2)
> > Comment on attachment 769796 [details] [diff] [review]
> > use -1 as httpd port
> > 
> > Review of attachment 769796 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: toolkit/components/places/tests/network/test_history_redirects.js
> > @@ +28,5 @@
> > > +});
> > > +
> > > +XPCOMUtils.defineLazyGetter(this, "FOUND_URL", function() {
> > > +  return "http://localhost:" + PORT + FOUND_PATH;
> > > +});
> > 
> > these don't need to be lazy getters, why not just define const?
> > 
> > const PORT = HTTPSVR.identity.primaryPort;
> > const PERMA_REDIR_URL = "http://localhost:" + PORT + PERMA_REDIR_PATH;
> > 
> > and so on...
> 
> They have to be lazy getters because the server is not started at that time.
> If you want I could take out the start call from the run_test function and
> put it at the top of the file and then I can do what you suggested. Is that
> ok?

well, we start the server immediately regardless, so I think it's ok if you do that.

Comment 5

[Mihnea Dobrescu-Balaur (:mihneadb)]

Attachment: use a dynamic port

Comment 6

[Mihnea Dobrescu-Balaur (:mihneadb)]

Comment on attachment 773702 [details] [diff] [review]
use a dynamic port

keeping prev r+

Comment 7

[Mihnea Dobrescu-Balaur (:mihneadb)]

here's the try run https://tbpl.mozilla.org/?tree=Try&rev=93ea699403ed

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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

Comment 9

[Ryan VanderMeulen [:RyanVM]]

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