You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Joe Orton <jo...@redhat.com> on 2013/06/19 11:15:57 UTC

[PATCH] ruby bindings: improve svnserve port handling in test suite

We've seen a few cases where the test suite for the ruby bindings fails 
spuriously.  There are two (separate) issues:

a) there is a race between svnserve failing in bind() because the chosen 
port is already in use, and the test suite checking that the forked 
svnserve process is still running.

b) the set of ports used is not randomized so concurrent runs on the 
same machine will be fighting over the same ports

Is something like the patch below ok?  It does not guarantee success but 
makes failure less likely.

(p.s. congrats to all for the 1.8.0 release ;)

[[[
* subversion/bindings/swig/ruby/test/util.rb
  (SvnTestUtil#setup_default_variables): Randomize port range used
  for svnserve.
  (Svnserve#setup_svnserve): Increase chance of catching an svnserve
  failure.
]]]

Index: subversion/bindings/swig/ruby/test/util.rb
===================================================================
--- subversion/bindings/swig/ruby/test/util.rb	(revision 1494521)
+++ subversion/bindings/swig/ruby/test/util.rb	(working copy)
@@ -73,7 +73,8 @@
     @realm = "sample realm"
 
     @svnserve_host = "127.0.0.1"
-    @svnserve_ports = (64152..64282).collect{|x| x.to_s}
+    sport = (50000 + rand(100) * 100)
+    @svnserve_ports = (sport..sport + 99).collect{|x| x.to_s}
 
     @tmp_path = Dir.mktmpdir
     @wc_path = File.join(@tmp_path, "wc")
@@ -286,6 +287,8 @@
                "--listen-port", port,
                "-d", "--foreground")
         }
+        # wait a while for svnserve to attempt a bind() and possibly fail
+        sleep(1)
         pid, status = Process.waitpid2(@svnserve_pid, Process::WNOHANG)
         if status and status.exited?
           if $DEBUG

Re: [PATCH] ruby bindings: improve svnserve port handling in test suite

Posted by Daniel Shahaf <da...@elego.de>.
Joe Orton wrote on Wed, Jun 19, 2013 at 10:15:57 +0100:
> We've seen a few cases where the test suite for the ruby bindings fails 
> spuriously.  There are two (separate) issues:
> 
> a) there is a race between svnserve failing in bind() because the chosen 
> port is already in use, and the test suite checking that the forked 
> svnserve process is still running.
> 
> b) the set of ports used is not randomized so concurrent runs on the 
> same machine will be fighting over the same ports
> 
> Is something like the patch below ok?  It does not guarantee success but 
> makes failure less likely.
> 
> (p.s. congrats to all for the 1.8.0 release ;)
> 
> [[[
> * subversion/bindings/swig/ruby/test/util.rb
>   (SvnTestUtil#setup_default_variables): Randomize port range used
>   for svnserve.
>   (Svnserve#setup_svnserve): Increase chance of catching an svnserve
>   failure.
> ]]]
> 
> Index: subversion/bindings/swig/ruby/test/util.rb
> ===================================================================
> --- subversion/bindings/swig/ruby/test/util.rb	(revision 1494521)
> +++ subversion/bindings/swig/ruby/test/util.rb	(working copy)
> @@ -73,7 +73,8 @@
>      @realm = "sample realm"
>  
>      @svnserve_host = "127.0.0.1"
> -    @svnserve_ports = (64152..64282).collect{|x| x.to_s}
> +    sport = (50000 + rand(100) * 100)
> +    @svnserve_ports = (sport..sport + 99).collect{|x| x.to_s}
>  

What about:

       @svnserve_ports = (64152..64282).shuffle.collect{|x| x.to_s}

?

>      @tmp_path = Dir.mktmpdir
>      @wc_path = File.join(@tmp_path, "wc")
> @@ -286,6 +287,8 @@
>                 "--listen-port", port,
>                 "-d", "--foreground")
>          }
> +        # wait a while for svnserve to attempt a bind() and possibly fail
> +        sleep(1)

No objections here.

>          pid, status = Process.waitpid2(@svnserve_pid, Process::WNOHANG)
>          if status and status.exited?
>            if $DEBUG