Help with bug from Cucumber codebase
Hello, I have been trying to get cucumber working with MacRuby, and I have run into a bug. I have tracked the source of it down to a block of code in cucumber's code base[BLOCK 1]. I have condensed this down to a much smaller example[BLOCK 2]. In ruby 1.9.1 the second block results in 'yellow,bold', but in macruby the second block results in nil. My guess is that there is a bug in the merge implementation or the constructor implementation. I am not familiar enough with ruby to know which, so I will take a look at both. I feel this should be added as a test case, but I am not sure where to put that, since I am new to the project. Any tips? -Scott ----- BLOCK 1 ---------- ALIASES = Hash.new do |h,k| if k.to_s =~ /(.*)_param/ h[$1] + ',bold' end end.merge({ 'missing' => 'yellow', 'pending' => 'yellow', 'failed' => 'red', 'passed' => 'green', 'outline' => 'cyan', 'skipped' => 'cyan', 'comment' => 'grey', 'tag' => 'blue' }) if ENV['CUCUMBER_COLORS'] # Example: export CUCUMBER_COLORS="passed=red:failed=yellow" ENV['CUCUMBER_COLORS'].split(':').each do |pair| a = pair.split('=') ALIASES[a[0]] = a[1] end end ALIASES.each do |method, color| unless method =~ /.*_param/ code = <<-EOF def #{method}(string=nil, &proc) #{ALIASES[method].split(",").join("(") + "(string, &proc" + ")" * ALIASES[method].split(",").length} end # This resets the colour to the non-param colour def #{method}_param(string=nil, &proc) #{ALIASES[method+'_param'].split(",").join("(") + "(string, &proc" + ")" * ALIASES[method+'_param'].split(",").length} + #{ALIASES[method].split(",").join(' + ')} end EOF eval(code) end end -------------- END BLOCK 1 -------------- ------- BLOCK 2 ------------------ ALIASES = Hash.new do |h,k| if k.to_s =~ /(.*)_param/ h[$1] + ',bold' end end.merge({ 'missing' => 'yellow' }) puts ALIASES['missing_param'] --------- END BLOCK 2 ---------
Sorry to reply to my own post but I have been playing with this all day to figure out what is wrong. I have narrowed the problem down to the merge method, but I am not sure how to fix it. Here are some test cases. These all pass in ruby 1.9.1. The first one fails in MacRuby trunk, but the other two pass. h = Hash.new do |h, k| 12 end h = h.merge({1 => 2}) assert_equal(h[1], 2) assert_equal(h[:random], 12) h = Hash.new do |h, k| 12 end h.merge!({1 => 2}) assert_equal(h[1], 2) assert_equal(h[:random], 12) h = Hash.new do |h, k| 12 end h.merge({1 => 2}) assert_equal(h[1], 12) assert_equal(h[:random], 12) On Feb 11, 2009, at 11:53 AM, M. Scott Ford wrote:
Hello,
I have been trying to get cucumber working with MacRuby, and I have run into a bug. I have tracked the source of it down to a block of code in cucumber's code base[BLOCK 1]. I have condensed this down to a much smaller example[BLOCK 2]. In ruby 1.9.1 the second block results in 'yellow,bold', but in macruby the second block results in nil.
My guess is that there is a bug in the merge implementation or the constructor implementation. I am not familiar enough with ruby to know which, so I will take a look at both. I feel this should be added as a test case, but I am not sure where to put that, since I am new to the project. Any tips?
-Scott
----- BLOCK 1 ---------- ALIASES = Hash.new do |h,k| if k.to_s =~ /(.*)_param/ h[$1] + ',bold' end end.merge({ 'missing' => 'yellow', 'pending' => 'yellow', 'failed' => 'red', 'passed' => 'green', 'outline' => 'cyan', 'skipped' => 'cyan', 'comment' => 'grey', 'tag' => 'blue' })
if ENV['CUCUMBER_COLORS'] # Example: export CUCUMBER_COLORS="passed=red:failed=yellow" ENV['CUCUMBER_COLORS'].split(':').each do |pair| a = pair.split('=') ALIASES[a[0]] = a[1] end end
ALIASES.each do |method, color| unless method =~ /.*_param/ code = <<-EOF def #{method}(string=nil, &proc) #{ALIASES[method].split(",").join("(") + "(string, &proc" + ")" * ALIASES[method].split(",").length} end # This resets the colour to the non-param colour def #{method}_param(string=nil, &proc) #{ALIASES[method+'_param'].split(",").join("(") + "(string, &proc" + ")" * ALIASES[method+'_param'].split(",").length} + #{ALIASES[method].split(",").join(' + ')} end EOF eval(code) end end -------------- END BLOCK 1 --------------
------- BLOCK 2 ------------------
ALIASES = Hash.new do |h,k| if k.to_s =~ /(.*)_param/ h[$1] + ',bold' end end.merge({ 'missing' => 'yellow' })
puts ALIASES['missing_param']
--------- END BLOCK 2 --------- _______________________________________________ MacRuby-devel mailing list MacRuby-devel@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo.cgi/macruby-devel
And the fix. Change rb_hash_merge to use rb_obj_dup instead of rb_hash_dup. On Feb 11, 2009, at 2:48 PM, M. Scott Ford wrote:
Sorry to reply to my own post but I have been playing with this all day to figure out what is wrong. I have narrowed the problem down to the merge method, but I am not sure how to fix it. Here are some test cases. These all pass in ruby 1.9.1. The first one fails in MacRuby trunk, but the other two pass.
h = Hash.new do |h, k| 12 end h = h.merge({1 => 2}) assert_equal(h[1], 2) assert_equal(h[:random], 12)
h = Hash.new do |h, k| 12 end h.merge!({1 => 2}) assert_equal(h[1], 2) assert_equal(h[:random], 12)
h = Hash.new do |h, k| 12 end h.merge({1 => 2}) assert_equal(h[1], 12) assert_equal(h[:random], 12)
On Feb 11, 2009, at 11:53 AM, M. Scott Ford wrote:
Hello,
I have been trying to get cucumber working with MacRuby, and I have run into a bug. I have tracked the source of it down to a block of code in cucumber's code base[BLOCK 1]. I have condensed this down to a much smaller example[BLOCK 2]. In ruby 1.9.1 the second block results in 'yellow,bold', but in macruby the second block results in nil.
My guess is that there is a bug in the merge implementation or the constructor implementation. I am not familiar enough with ruby to know which, so I will take a look at both. I feel this should be added as a test case, but I am not sure where to put that, since I am new to the project. Any tips?
-Scott
----- BLOCK 1 ---------- ALIASES = Hash.new do |h,k| if k.to_s =~ /(.*)_param/ h[$1] + ',bold' end end.merge({ 'missing' => 'yellow', 'pending' => 'yellow', 'failed' => 'red', 'passed' => 'green', 'outline' => 'cyan', 'skipped' => 'cyan', 'comment' => 'grey', 'tag' => 'blue' })
if ENV['CUCUMBER_COLORS'] # Example: export CUCUMBER_COLORS="passed=red:failed=yellow" ENV['CUCUMBER_COLORS'].split(':').each do |pair| a = pair.split('=') ALIASES[a[0]] = a[1] end end
ALIASES.each do |method, color| unless method =~ /.*_param/ code = <<-EOF def #{method}(string=nil, &proc) #{ALIASES[method].split(",").join("(") + "(string, &proc" + ")" * ALIASES[method].split(",").length} end # This resets the colour to the non-param colour def #{method}_param(string=nil, &proc) #{ALIASES[method+'_param'].split(",").join("(") + "(string, &proc" + ")" * ALIASES[method +'_param'].split(",").length} + #{ALIASES[method].split(",").join(' + ')} end EOF eval(code) end end -------------- END BLOCK 1 --------------
------- BLOCK 2 ------------------
ALIASES = Hash.new do |h,k| if k.to_s =~ /(.*)_param/ h[$1] + ',bold' end end.merge({ 'missing' => 'yellow' })
puts ALIASES['missing_param']
--------- END BLOCK 2 --------- _______________________________________________ MacRuby-devel mailing list MacRuby-devel@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo.cgi/macruby-devel
_______________________________________________ MacRuby-devel mailing list MacRuby-devel@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo.cgi/macruby-devel
Thanks for the detective work Scott! Could you create a test case in test/ruby/test_hash.rb and send us a patch? I would then merge it. Laurent On Feb 11, 2009, at 12:11 PM, M. Scott Ford wrote:
And the fix.
Change rb_hash_merge to use rb_obj_dup instead of rb_hash_dup.
On Feb 11, 2009, at 2:48 PM, M. Scott Ford wrote:
Sorry to reply to my own post but I have been playing with this all day to figure out what is wrong. I have narrowed the problem down to the merge method, but I am not sure how to fix it. Here are some test cases. These all pass in ruby 1.9.1. The first one fails in MacRuby trunk, but the other two pass.
h = Hash.new do |h, k| 12 end h = h.merge({1 => 2}) assert_equal(h[1], 2) assert_equal(h[:random], 12)
h = Hash.new do |h, k| 12 end h.merge!({1 => 2}) assert_equal(h[1], 2) assert_equal(h[:random], 12)
h = Hash.new do |h, k| 12 end h.merge({1 => 2}) assert_equal(h[1], 12) assert_equal(h[:random], 12)
On Feb 11, 2009, at 11:53 AM, M. Scott Ford wrote:
Hello,
I have been trying to get cucumber working with MacRuby, and I have run into a bug. I have tracked the source of it down to a block of code in cucumber's code base[BLOCK 1]. I have condensed this down to a much smaller example[BLOCK 2]. In ruby 1.9.1 the second block results in 'yellow,bold', but in macruby the second block results in nil.
My guess is that there is a bug in the merge implementation or the constructor implementation. I am not familiar enough with ruby to know which, so I will take a look at both. I feel this should be added as a test case, but I am not sure where to put that, since I am new to the project. Any tips?
-Scott
----- BLOCK 1 ---------- ALIASES = Hash.new do |h,k| if k.to_s =~ /(.*)_param/ h[$1] + ',bold' end end.merge({ 'missing' => 'yellow', 'pending' => 'yellow', 'failed' => 'red', 'passed' => 'green', 'outline' => 'cyan', 'skipped' => 'cyan', 'comment' => 'grey', 'tag' => 'blue' })
if ENV['CUCUMBER_COLORS'] # Example: export CUCUMBER_COLORS="passed=red:failed=yellow" ENV['CUCUMBER_COLORS'].split(':').each do |pair| a = pair.split('=') ALIASES[a[0]] = a[1] end end
ALIASES.each do |method, color| unless method =~ /.*_param/ code = <<-EOF def #{method}(string=nil, &proc) #{ALIASES[method].split(",").join("(") + "(string, &proc" + ")" * ALIASES[method].split(",").length} end # This resets the colour to the non-param colour def #{method}_param(string=nil, &proc) #{ALIASES[method+'_param'].split(",").join("(") + "(string, &proc" + ")" * ALIASES[method +'_param'].split(",").length} + #{ALIASES[method].split(",").join(' + ')} end EOF eval(code) end end -------------- END BLOCK 1 --------------
------- BLOCK 2 ------------------
ALIASES = Hash.new do |h,k| if k.to_s =~ /(.*)_param/ h[$1] + ',bold' end end.merge({ 'missing' => 'yellow' })
puts ALIASES['missing_param']
--------- END BLOCK 2 --------- _______________________________________________ MacRuby-devel mailing list MacRuby-devel@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo.cgi/macruby-devel
_______________________________________________ MacRuby-devel mailing list MacRuby-devel@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo.cgi/macruby-devel
_______________________________________________ MacRuby-devel mailing list MacRuby-devel@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo.cgi/macruby-devel
Hi, If this is a problem with the Objective-C Hash implementation, I would be inclined to say that the test case should go in test-macruby/cases/ rubyspec/hash_test.rb. Because we might need to move this into the rubyspec project, if it's not already in there, once we start on integrating rubyspec. If so, please take a look at, for instance, this test case as an example: http://www.macruby.org/trac/browser/MacRuby/trunk/test-macruby/cases/hotcoco... - Eloy On 12 feb 2009, at 04:21, Laurent Sansonetti wrote:
Thanks for the detective work Scott! Could you create a test case in test/ruby/test_hash.rb and send us a patch? I would then merge it.
Laurent
On Feb 11, 2009, at 12:11 PM, M. Scott Ford wrote:
And the fix.
Change rb_hash_merge to use rb_obj_dup instead of rb_hash_dup.
On Feb 11, 2009, at 2:48 PM, M. Scott Ford wrote:
Sorry to reply to my own post but I have been playing with this all day to figure out what is wrong. I have narrowed the problem down to the merge method, but I am not sure how to fix it. Here are some test cases. These all pass in ruby 1.9.1. The first one fails in MacRuby trunk, but the other two pass.
h = Hash.new do |h, k| 12 end h = h.merge({1 => 2}) assert_equal(h[1], 2) assert_equal(h[:random], 12)
h = Hash.new do |h, k| 12 end h.merge!({1 => 2}) assert_equal(h[1], 2) assert_equal(h[:random], 12)
h = Hash.new do |h, k| 12 end h.merge({1 => 2}) assert_equal(h[1], 12) assert_equal(h[:random], 12)
On Feb 11, 2009, at 11:53 AM, M. Scott Ford wrote:
Hello,
I have been trying to get cucumber working with MacRuby, and I have run into a bug. I have tracked the source of it down to a block of code in cucumber's code base[BLOCK 1]. I have condensed this down to a much smaller example[BLOCK 2]. In ruby 1.9.1 the second block results in 'yellow,bold', but in macruby the second block results in nil.
My guess is that there is a bug in the merge implementation or the constructor implementation. I am not familiar enough with ruby to know which, so I will take a look at both. I feel this should be added as a test case, but I am not sure where to put that, since I am new to the project. Any tips?
-Scott
----- BLOCK 1 ---------- ALIASES = Hash.new do |h,k| if k.to_s =~ /(.*)_param/ h[$1] + ',bold' end end.merge({ 'missing' => 'yellow', 'pending' => 'yellow', 'failed' => 'red', 'passed' => 'green', 'outline' => 'cyan', 'skipped' => 'cyan', 'comment' => 'grey', 'tag' => 'blue' })
if ENV['CUCUMBER_COLORS'] # Example: export CUCUMBER_COLORS="passed=red:failed=yellow" ENV['CUCUMBER_COLORS'].split(':').each do |pair| a = pair.split('=') ALIASES[a[0]] = a[1] end end
ALIASES.each do |method, color| unless method =~ /.*_param/ code = <<-EOF def #{method}(string=nil, &proc) #{ALIASES[method].split(",").join("(") + "(string, &proc" + ")" * ALIASES[method].split(",").length} end # This resets the colour to the non-param colour def #{method}_param(string=nil, &proc) #{ALIASES[method+'_param'].split(",").join("(") + "(string, &proc" + ")" * ALIASES[method +'_param'].split(",").length} + #{ALIASES[method].split(",").join(' + ')} end EOF eval(code) end end -------------- END BLOCK 1 --------------
------- BLOCK 2 ------------------
ALIASES = Hash.new do |h,k| if k.to_s =~ /(.*)_param/ h[$1] + ',bold' end end.merge({ 'missing' => 'yellow' })
puts ALIASES['missing_param']
--------- END BLOCK 2 --------- _______________________________________________ MacRuby-devel mailing list MacRuby-devel@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo.cgi/macruby-devel
_______________________________________________ MacRuby-devel mailing list MacRuby-devel@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo.cgi/macruby-devel
_______________________________________________ MacRuby-devel mailing list MacRuby-devel@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo.cgi/macruby-devel
_______________________________________________ MacRuby-devel mailing list MacRuby-devel@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo.cgi/macruby-devel
Eloy, I did not see your email before I submitted the patch. I glanced at the hash tests in the RubySpec project and there is not much there, so moving this test into RubySpec sounds like a good idea. Is that something that I should take care of now? -Scott On Feb 12, 2009, at 8:53 AM, Eloy Duran wrote:
Hi,
If this is a problem with the Objective-C Hash implementation, I would be inclined to say that the test case should go in test- macruby/cases/rubyspec/hash_test.rb. Because we might need to move this into the rubyspec project, if it's not already in there, once we start on integrating rubyspec.
If so, please take a look at, for instance, this test case as an example: http://www.macruby.org/trac/browser/MacRuby/trunk/test-macruby/cases/hotcoco...
- Eloy
Hi Scott, If you would like to re write your patch in a more spec style as the one I pointed to that would be great! We can then easily migrate it to rubyspec later on. Thanks, - Eloy On 12 feb 2009, at 15:09, M. Scott Ford wrote:
Eloy,
I did not see your email before I submitted the patch.
I glanced at the hash tests in the RubySpec project and there is not much there, so moving this test into RubySpec sounds like a good idea. Is that something that I should take care of now?
-Scott
On Feb 12, 2009, at 8:53 AM, Eloy Duran wrote:
Hi,
If this is a problem with the Objective-C Hash implementation, I would be inclined to say that the test case should go in test- macruby/cases/rubyspec/hash_test.rb. Because we might need to move this into the rubyspec project, if it's not already in there, once we start on integrating rubyspec.
If so, please take a look at, for instance, this test case as an example: http://www.macruby.org/trac/browser/MacRuby/trunk/test-macruby/cases/hotcoco...
- Eloy
_______________________________________________ MacRuby-devel mailing list MacRuby-devel@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo.cgi/macruby-devel
Eloy, Here is an updated patch that uses spec style tests. -Scott On Feb 12, 2009, at 9:16 AM, Eloy Duran wrote:
Hi Scott,
If you would like to re write your patch in a more spec style as the one I pointed to that would be great! We can then easily migrate it to rubyspec later on.
Thanks, - Eloy
Hi Scott, As you can see in this commit: http://github.com/masterkain/macruby/commit/0cb18321229b35a61e9af46e068b3d55... I have fixed the problem in a different way. Because if I understood your intention right, then the problem was in #dup. Please let me know if this fixes your issue, or that I might have missed another part of your tests. - Eloy On 12 feb 2009, at 16:29, M. Scott Ford wrote:
Eloy,
Here is an updated patch that uses spec style tests.
-Scott <hash_merge.patch>
On Feb 12, 2009, at 9:16 AM, Eloy Duran wrote:
Hi Scott,
If you would like to re write your patch in a more spec style as the one I pointed to that would be great! We can then easily migrate it to rubyspec later on.
Thanks, - Eloy
_______________________________________________ MacRuby-devel mailing list MacRuby-devel@lists.macosforge.org http://lists.macosforge.org/mailman/listinfo.cgi/macruby-devel
Eloy, I applied your change to dup, rolled out my change, and then the tests that I wrote still pass. -Scott On Feb 12, 2009, at 2:02 PM, Eloy Duran wrote:
Hi Scott,
As you can see in this commit: http://github.com/masterkain/macruby/commit/0cb18321229b35a61e9af46e068b3d55... I have fixed the problem in a different way. Because if I understood your intention right, then the problem was in #dup. Please let me know if this fixes your issue, or that I might have missed another part of your tests.
- Eloy
participants (3)
-
Eloy Duran
-
Laurent Sansonetti
-
M. Scott Ford