On Mon, Mar 19, 2012 at 9:09 AM, Hyrum K Wright
<hyrum.wri...@wandisco.com> wrote:
> On Tue, Mar 13, 2012 at 9:52 AM, Joe Swatosh <joe.swat...@gmail.com> wrote:
>> On Mon, Mar 12, 2012 at 6:18 AM, Hyrum K Wright
>> <hyrum.wri...@wandisco.com> wrote:
>>> On Mon, Mar 12, 2012 at 12:11 AM, Joe Swatosh <joe.swat...@gmail.com> wrote:
>>>> On Fri, Mar 9, 2012 at 10:39 PM, Joe Swatosh <joe.swat...@gmail.com> wrote:
>>>>> On Fri, Mar 9, 2012 at 9:46 PM, Joe Swatosh <joe.swat...@gmail.com> wrote:
>>>>>> On Fri, Mar 9, 2012 at 2:44 AM, Philip Martin
>>>>>> <philip.mar...@wandisco.com> wrote:
>>
>>>>
>>>> ****************
>>>>
>>>> Restore failing Ruby bindings tests failing since r1293375.
>>>>
>>>> * subversion/bindings/swig/ruby/test/test_info.rb
>>>> (test_diff): Remove assertions testing implementation details that
>>>> have changed.
>>>>
>>>> ****************
>>>>
>>>>
>>>> Index: subversion/bindings/swig/ruby/test/test_info.rb
>>>> ===================================================================
>>>> --- subversion/bindings/swig/ruby/test/test_info.rb (revision 1294254)
>>>> +++ subversion/bindings/swig/ruby/test/test_info.rb (working copy)
>>>> @@ -217,7 +217,6 @@
>>>> assert_equal([file1, file2, file4].sort, keys[0..-2])
>>>> assert_match(/\A#{file5}/, file5_key)
>>>> assert(info.diffs[file1].has_key?(:modified))
>>>> - assert(info.diffs[file1].has_key?(:property_changed))
>>>> assert(info.diffs[file2].has_key?(:modified))
>>>> assert(info.diffs[file4].has_key?(:added))
>>>> assert(info.diffs[file4].has_key?(:property_changed))
>>>> @@ -230,8 +229,6 @@
>>>> assert_equal(0, info.diffs[file4][:added].deleted_line)
>>>> assert_equal(0, info.diffs[file5_key][:copied].added_line)
>>>> assert_equal(0, info.diffs[file5_key][:copied].deleted_line)
>>>> - assert_equal("Name: #{file1_prop_key}\n - #{file1_prop_value}\n",
>>>> - info.diffs[file1][:property_changed].body)
>>>> assert_equal("Name: #{file4_prop_key}\n + #{file4_prop_value}\n",
>>>> info.diffs[file4][:property_changed].body)
>>>> assert_equal(commit_info.revision, info.revision)
>>>
>>> That would certainly fix the test failures, in that they wouldn't be
>>> detected.
>>>
>>> Are you implying the current (without this patch) ruby tests are
>>> testing implementation details, as well as results, and that's the
>>> reason this change is needed?
>>>
>>> -Hyrum
>>>
>>>
>>
>> Yup that is exactly what I'm implying. You may recall during wc-ng
>> development that there were many failing Ruby bindings tests. There
>> were three broad categories of failures: binding or binding test
>> errors, unintentional changes to how the wc library worked, and tests
>> of wc implementation among the bindings tests. My recollection of that
>> time was that the problems were pretty evenly distributed into those
>> categories. When I asked him about it, my memory of what kou said was
>> that subversion implementation needed testing.
>>
>> WRT this patch, when I sent it, I thought this was an implementation
>> test, but on reflection I am not so sure. I will look into it again
>> when the weekend comes (earliest that my schedule allows). If anyone
>> can resolve this correctly sooner, I encourage them to do so.
>
> I committed the patch in r1302524. Feel free to update it as you feel
> necessary.
>
> -Hyrum
>
I understand needing to get the buildbots green again, however I think
that commit should be reverted. If the problem is in the bindings, I
think this is the patch that is needed:
[[[
Index: subversion/bindings/swig/ruby/svn/info.rb
===================================================================
--- subversion/bindings/swig/ruby/svn/info.rb (revision 1302724)
+++ subversion/bindings/swig/ruby/svn/info.rb (working copy)
@@ -156,7 +156,7 @@
try_diff(node, base_root, path, base_path)
end
- if node.prop_mod? and !node.delete?
+ unless node.delete?
get_prop_diff(node, base_root, path, base_path)
end
]]]
At least this will get the original test passing again. I guess that
deleting a property isn't considered a prop_mod on the node anymore?
I'm only 99% on this patch, as I need more time to make sure it
doesn't cause reporting of empty differences in other situations.
--
Joe
PS I have different patch that provides an implementation of
Dir.mktmpdir if one doesn't exist (like for Ruby 1.8.6), but it is
just a copy of the implementation from Ruby 1.8.7. There is no copy
info in the original file and I want to double check how to properly
attribute it. Thanks.
