[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: APR hash order ruby test failure

From: Daniel Shahaf <danielsh_at_elego.de>
Date: Thu, 22 Mar 2012 20:39:48 +0200

Joe Swatosh wrote on Thu, Mar 22, 2012 at 07:36:16 -0700:
> On Thu, Mar 22, 2012 at 4:41 AM, Philip Martin
> <philip.martin_at_wandisco.com> wrote:
> > There is another failure in the ruby testsuite:
> >
> > http://ci.apache.org/builders/svn-x64-ubuntu-gcc/builds/4626
> >
> >  1) Failure:
> > test_changelists_get_with_block(SvnClientTest)
> > /var/lib/buildbot/svn-buildslave/svn-x64-ubuntu/build/subversion/bindings/swig/ruby/test/test_client.rb:2296:in `assert_changelists'
> > /var/lib/buildbot/svn-buildslave/svn-x64-ubuntu/build/subversion/bindings/swig/ruby/test/util.rb:204:in `make_context'
> > /var/lib/buildbot/svn-buildslave/svn-x64-ubuntu/build/subversion/bindings/swig/ruby/test/test_client.rb:2288:in `assert_changelists'
> > /var/lib/buildbot/svn-buildslave/svn-x64-ubuntu/build/subversion/bindings/swig/ruby/test/test_client.rb:2349:in `test_changelists_get_with_block':
> > <{nil=>
> >  ["/tmp/d20120322-8616-qtl2ah/wc",
> >   "/tmp/d20120322-8616-qtl2ah/wc/hello1.txt",
> >   "/tmp/d20120322-8616-qtl2ah/wc/hello2.txt"]}> expected but was
> > <{nil=>
> >  ["/tmp/d20120322-8616-qtl2ah/wc",
> >   "/tmp/d20120322-8616-qtl2ah/wc/hello2.txt",
> >   "/tmp/d20120322-8616-qtl2ah/wc/hello1.txt"]}>.
> >
> > The failing code is:
> >
> >    make_context(log) do |ctx|
> >      File.open(path1, "w") {|f| f.print(src)}
> >      File.open(path2, "w") {|f| f.print(src)}
> >      ctx.add(path1)
> >      ctx.add(path2)
> >      ctx.commit(@wc_path)
> >
> >      assert_equal({}, yield(ctx, changelist1))
> >      assert_equal({nil=>[@wc_path,path1,path2].map{|f| File.expand_path(f)}}, yield(ctx, nil))
> >
> > This is likely to be APR's new hash randomisation which means that the
> > notification order of "hello1.txt" and "hello2.txt" is unpredictable.  I
> > think the fix is for the lists to be sorted but I don't know the
> > necessary Ruby.
> >
> > --
> > uberSVN: Apache Subversion Made Easy
> > http://www.uberSVN.com
>
>
> I'm sure you are right. I'll start into this on the weekend....
>
> --
> Joe

How about that? Is it a backwards compatible change?

Index: ruby/svn/client.rb
===================================================================
--- ruby/svn/client.rb (revision 1303600)
+++ ruby/svn/client.rb (working copy)
@@ -19,6 +19,7 @@
 
 require "English"
 require 'uri'
+require 'set'
 require "svn/error"
 require "svn/util"
 require "svn/core"
@@ -692,7 +693,7 @@ module Svn
       end
 
       def changelists(changelists_names, root_path, depth=nil, &block)
- lists_contents = Hash.new{|h,k| h[k]=[]}
+ lists_contents = Hash.new{|h,k| h[k]=Set.new}
         changelists_names = [changelists_names] unless changelists_names.is_a?(Array) or changelists_names.nil?
         block ||= lambda{|path, changelist| lists_contents[changelist] << path }
         Client.get_changelists(root_path, changelists_names, depth, block, self)
Index: ruby/test/test_client.rb
===================================================================
--- ruby/test/test_client.rb (revision 1303600)
+++ ruby/test/test_client.rb (working copy)
@@ -17,6 +17,8 @@
 # under the License.
 # ====================================================================
 
+require 'set'
+
 require "my-assertions"
 require "util"
 
@@ -2293,7 +2295,8 @@ class SvnClientTest < Test::Unit::TestCase
       ctx.commit(@wc_path)
 
       assert_equal({}, yield(ctx, changelist1))
- assert_equal({nil=>[@wc_path,path1,path2].map{|f| File.expand_path(f)}}, yield(ctx, nil))
+ assert_equal({nil=>[@wc_path,path1,path2].map{|f| File.expand_path(f)}.to_set},
+ yield(ctx, nil))
       assert_equal({}, yield(ctx, []))
       assert_equal({}, yield(ctx, [changelist1]))
       assert_equal({}, yield(ctx, [changelist2]))
@@ -2347,7 +2350,7 @@ class SvnClientTest < Test::Unit::TestCase
 
   def test_changelists_get_with_block
     assert_changelists do |ctx, changelist_name|
- changelists = Hash.new{|h,k| h[k]=[]}
+ changelists = Hash.new{|h,k| h[k]=Set.new}
       ctx.changelists(changelist_name, @wc_path) do |path,cl_name|
         changelists[cl_name] << path
       end
Received on 2012-03-22 19:40:39 CET

This is an archived mail posted to the Subversion Dev mailing list.