# Review: ActionPack fix

**URL:** <https://www.ruby-forum.com/t/review-actionpack-fix/181516>\
**Category:** IronRuby\
**Created:** [January 13, 2010, 1:14am UTC](https://www.ruby-forum.com/t/review-actionpack-fix/181516 "2010-01-13T01:14:28Z")\
**Posts on this page:** 6\
**Page:** 1

<div class="post-metadata">

**Author:** ![Shri\_B](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/s/898d66/32.png) [@Shri\_B](https://www.ruby-forum.com/u/Shri_B)\
**Post date:** [January 13, 2010, 1:14am UTC](https://www.ruby-forum.com/t/review-actionpack-fix/181516/1 "2010-01-13T01:14:28Z")

</div>

tfpt review /shelveset:rails;sborde

Fixes a bug in Proc equality comparison which was causing the ActionPack  
caching tests to fail  
Splits up the Rails test harness files into tests that fail with MRI as  
well (which are often issues with the test)  
Changes the tests to load specific versions of Rails and other gems.  
Without that, you get load errors because of incorrect versions being  
loaded  
Changed the default of irtests to run the Rails tests. They can be  
skipped by using -m (for “minimum”)

---

<div class="post-metadata">

**Author:** ![Shri\_B](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/s/898d66/32.png) [@Shri\_B](https://www.ruby-forum.com/u/Shri_B)\
**Post date:** [January 13, 2010, 4:06am UTC](https://www.ruby-forum.com/t/review-actionpack-fix/181516/2 "2010-01-13T04:06:33Z")

</div>

Is ActiveRecord supposed to be commented out (irtests.rb#46)? Also, we  
should upgrade to testing the latest version of Rails (2.3.5), but it  
doesn’t have to be now.

Looks good otherwise,  
~js

From: Shri B.  
Sent: Tuesday, January 12, 2010 4:14 PM  
To: IronRuby External Code R.  
Cc: removed\_email\_address@domain.invalid  
Subject: Review: ActionPack fix

tfpt review /shelveset:rails;sborde

Fixes a bug in Proc equality comparison which was causing the ActionPack  
caching tests to fail  
Splits up the Rails test harness files into tests that fail with MRI as  
well (which are often issues with the test)  
Changes the tests to load specific versions of Rails and other gems.  
Without that, you get load errors because of incorrect versions being  
loaded  
Changed the default of irtests to run the Rails tests. They can be  
skipped by using -m (for “minimum”)

---

<div class="post-metadata">

**Author:** ![Shri\_B](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/s/898d66/32.png) [@Shri\_B](https://www.ruby-forum.com/u/Shri_B)\
**Post date:** [January 13, 2010, 6:42pm UTC](https://www.ruby-forum.com/t/review-actionpack-fix/181516/3 "2010-01-13T18:42:12Z")

</div>

The ActiveRecord tests in irtests.rb has never been really enabled for  
real (it was enabled with --all but no one used --all). The tests do  
work with a little bit of manual setup (downloading ironruby-dbi,  
creating the two required test databases, etc), but they should be  
enabled in irtests only when they are fully automated.

From: Jimmy S.  
Sent: Tuesday, January 12, 2010 7:05 PM  
To: Shri B.; IronRuby External Code R.  
Cc: removed\_email\_address@domain.invalid  
Subject: RE: Review: ActionPack fix

Is ActiveRecord supposed to be commented out (irtests.rb#46)? Also, we  
should upgrade to testing the latest version of Rails (2.3.5), but it  
doesn’t have to be now.

Looks good otherwise,  
~js

From: Shri B.  
Sent: Tuesday, January 12, 2010 4:14 PM  
To: IronRuby External Code R.  
Cc: removed\_email\_address@domain.invalid  
Subject: Review: ActionPack fix

tfpt review /shelveset:rails;sborde

Fixes a bug in Proc equality comparison which was causing the ActionPack  
caching tests to fail  
Splits up the Rails test harness files into tests that fail with MRI as  
well (which are often issues with the test)  
Changes the tests to load specific versions of Rails and other gems.  
Without that, you get load errors because of incorrect versions being  
loaded  
Changed the default of irtests to run the Rails tests. They can be  
skipped by using -m (for “minimum”)

---

<div class="post-metadata">

**Author:** ![Shri\_B](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/s/898d66/32.png) [@Shri\_B](https://www.ruby-forum.com/u/Shri_B)\
**Post date:** [January 13, 2010, 6:55pm UTC](https://www.ruby-forum.com/t/review-actionpack-fix/181516/4 "2010-01-13T18:55:05Z")

</div>

In generate\_test-unit\_tags.rb line 39:  
Please change

if fault == testcase\_faults.last then comma\_separator = “”  
else comma\_separator = “,”  
end

to

if fault == testcase\_faults.last  
comma\_separator = “”  
else  
comma\_separator = “,”  
end

In utr it looks like you’ve added ~120 blank lines after line 70, can  
you double check please? I’d also like a better way than checking the  
defined?-ness of RUBY\_ENGINE to disable MRI failures (since 1.9 has  
RUBY\_ENGINE), but I don’t feel too strongly about that.

JD

From: Shri B.  
Sent: Wednesday, January 13, 2010 9:42 AM  
To: Jimmy S.; IronRuby External Code R.  
Cc: removed\_email\_address@domain.invalid  
Subject: RE: Review: ActionPack fix

The ActiveRecord tests in irtests.rb has never been really enabled for  
real (it was enabled with --all but no one used --all). The tests do  
work with a little bit of manual setup (downloading ironruby-dbi,  
creating the two required test databases, etc), but they should be  
enabled in irtests only when they are fully automated.

From: Jimmy S.  
Sent: Tuesday, January 12, 2010 7:05 PM  
To: Shri B.; IronRuby External Code R.  
Cc: removed\_email\_address@domain.invalid  
Subject: RE: Review: ActionPack fix

Is ActiveRecord supposed to be commented out (irtests.rb#46)? Also, we  
should upgrade to testing the latest version of Rails (2.3.5), but it  
doesn’t have to be now.

Looks good otherwise,  
~js

From: Shri B.  
Sent: Tuesday, January 12, 2010 4:14 PM  
To: IronRuby External Code R.  
Cc: removed\_email\_address@domain.invalid  
Subject: Review: ActionPack fix

tfpt review /shelveset:rails;sborde

Fixes a bug in Proc equality comparison which was causing the ActionPack  
caching tests to fail  
Splits up the Rails test harness files into tests that fail with MRI as  
well (which are often issues with the test)  
Changes the tests to load specific versions of Rails and other gems.  
Without that, you get load errors because of incorrect versions being  
loaded  
Changed the default of irtests to run the Rails tests. They can be  
skipped by using -m (for “minimum”)

---

<div class="post-metadata">

**Author:** ![Shri\_B](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/s/898d66/32.png) [@Shri\_B](https://www.ruby-forum.com/u/Shri_B)\
**Post date:** [January 13, 2010, 7:12pm UTC](https://www.ruby-forum.com/t/review-actionpack-fix/181516/5 "2010-01-13T19:12:04Z")

</div>

It looks like Proc equality is simply a reference equality  
ReferenceEquals(self, other)with an exception of an empty block for  
which the scopes seem to be compared. Or do you have a counterexample?

def foo  
a = Proc.new { |x| }  
b = lambda &a  
c = Proc.new &a  
d = Proc.new { |x| }

return a,b,c,d  
end

x = foo  
y = foo

p x[0] == y[0]  
p x[1] == y[1]  
p x[2] == y[2]

p x[0] == x[1]  
p x[0] == x[2]  
p x[0] == x[3]

p x[0].object\_id  
p x[1].object\_id  
p x[2].object\_id  
p x[3].object\_id

a = Proc.new {}  
b = Proc.new {}  
def bar  
Proc.new {}  
end

p a == b, a.object\_id == b.object\_id, a == bar, bar == bar

Ruby 1.8:  
false  
false  
false  
false  
true  
false  
29077332  
29077320  
29077332  
29077308  
true  
false  
false  
false

Tomas

From: Jim D.  
Sent: Wednesday, January 13, 2010 9:54 AM  
To: Shri B.; Jimmy S.; IronRuby External Code R.  
Cc: removed\_email\_address@domain.invalid  
Subject: RE: Review: ActionPack fix

In generate\_test-unit\_tags.rb line 39:  
Please change

if fault == testcase\_faults.last then comma\_separator = “”  
else comma\_separator = “,”  
end

to

if fault == testcase\_faults.last  
comma\_separator = “”  
else  
comma\_separator = “,”  
end

In utr it looks like you’ve added ~120 blank lines after line 70, can  
you double check please? I’d also like a better way than checking the  
defined?-ness of RUBY\_ENGINE to disable MRI failures (since 1.9 has  
RUBY\_ENGINE), but I don’t feel too strongly about that.

JD

From: Shri B.  
Sent: Wednesday, January 13, 2010 9:42 AM  
To: Jimmy S.; IronRuby External Code R.  
Cc: removed\_email\_address@domain.invalid  
Subject: RE: Review: ActionPack fix

The ActiveRecord tests in irtests.rb has never been really enabled for  
real (it was enabled with --all but no one used --all). The tests do  
work with a little bit of manual setup (downloading ironruby-dbi,  
creating the two required test databases, etc), but they should be  
enabled in irtests only when they are fully automated.

From: Jimmy S.  
Sent: Tuesday, January 12, 2010 7:05 PM  
To: Shri B.; IronRuby External Code R.  
Cc: removed\_email\_address@domain.invalid  
Subject: RE: Review: ActionPack fix

Is ActiveRecord supposed to be commented out (irtests.rb#46)? Also, we  
should upgrade to testing the latest version of Rails (2.3.5), but it  
doesn’t have to be now.

Looks good otherwise,  
~js

From: Shri B.  
Sent: Tuesday, January 12, 2010 4:14 PM  
To: IronRuby External Code R.  
Cc: removed\_email\_address@domain.invalid  
Subject: Review: ActionPack fix

tfpt review /shelveset:rails;sborde

Fixes a bug in Proc equality comparison which was causing the ActionPack  
caching tests to fail  
Splits up the Rails test harness files into tests that fail with MRI as  
well (which are often issues with the test)  
Changes the tests to load specific versions of Rails and other gems.  
Without that, you get load errors because of incorrect versions being  
loaded  
Changed the default of irtests to run the Rails tests. They can be  
skipped by using -m (for “minimum”)

---

<div class="post-metadata">

**Author:** ![Shri\_B](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/s/898d66/32.png) [@Shri\_B](https://www.ruby-forum.com/u/Shri_B)\
**Post date:** [January 13, 2010, 7:21pm UTC](https://www.ruby-forum.com/t/review-actionpack-fix/181516/6 "2010-01-13T19:21:26Z")

</div>

The examples in  
Merlin\External.LCA\_RESTRICTED\Languages\IronRuby\mspec\rubyspec\core\proc\shared\equal.rb  
would not seem to work with ReferenceEquals

From: Tomas M.  
Sent: Wednesday, January 13, 2010 10:12 AM  
To: Jim D.; Shri B.; Jimmy S.; IronRuby External Code  
Reviewers  
Cc: removed\_email\_address@domain.invalid  
Subject: RE: Review: ActionPack fix

It looks like Proc equality is simply a reference equality  
ReferenceEquals(self, other)with an exception of an empty block for  
which the scopes seem to be compared. Or do you have a counterexample?

def foo  
a = Proc.new { |x| }  
b = lambda &a  
c = Proc.new &a  
d = Proc.new { |x| }

return a,b,c,d  
end

x = foo  
y = foo

p x[0] == y[0]  
p x[1] == y[1]  
p x[2] == y[2]

p x[0] == x[1]  
p x[0] == x[2]  
p x[0] == x[3]

p x[0].object\_id  
p x[1].object\_id  
p x[2].object\_id  
p x[3].object\_id

a = Proc.new {}  
b = Proc.new {}  
def bar  
Proc.new {}  
end

p a == b, a.object\_id == b.object\_id, a == bar, bar == bar

Ruby 1.8:  
false  
false  
false  
false  
true  
false  
29077332  
29077320  
29077332  
29077308  
true  
false  
false  
false

Tomas

From: Jim D.  
Sent: Wednesday, January 13, 2010 9:54 AM  
To: Shri B.; Jimmy S.; IronRuby External Code R.  
Cc: removed\_email\_address@domain.invalid  
Subject: RE: Review: ActionPack fix

In generate\_test-unit\_tags.rb line 39:  
Please change

if fault == testcase\_faults.last then comma\_separator = “”  
else comma\_separator = “,”  
end

to

if fault == testcase\_faults.last  
comma\_separator = “”  
else  
comma\_separator = “,”  
end

In utr it looks like you’ve added ~120 blank lines after line 70, can  
you double check please? I’d also like a better way than checking the  
defined?-ness of RUBY\_ENGINE to disable MRI failures (since 1.9 has  
RUBY\_ENGINE), but I don’t feel too strongly about that.

JD

From: Shri B.  
Sent: Wednesday, January 13, 2010 9:42 AM  
To: Jimmy S.; IronRuby External Code R.  
Cc: removed\_email\_address@domain.invalid  
Subject: RE: Review: ActionPack fix

The ActiveRecord tests in irtests.rb has never been really enabled for  
real (it was enabled with --all but no one used --all). The tests do  
work with a little bit of manual setup (downloading ironruby-dbi,  
creating the two required test databases, etc), but they should be  
enabled in irtests only when they are fully automated.

From: Jimmy S.  
Sent: Tuesday, January 12, 2010 7:05 PM  
To: Shri B.; IronRuby External Code R.  
Cc: removed\_email\_address@domain.invalid  
Subject: RE: Review: ActionPack fix

Is ActiveRecord supposed to be commented out (irtests.rb#46)? Also, we  
should upgrade to testing the latest version of Rails (2.3.5), but it  
doesn’t have to be now.

Looks good otherwise,  
~js

From: Shri B.  
Sent: Tuesday, January 12, 2010 4:14 PM  
To: IronRuby External Code R.  
Cc: removed\_email\_address@domain.invalid  
Subject: Review: ActionPack fix

tfpt review /shelveset:rails;sborde

Fixes a bug in Proc equality comparison which was causing the ActionPack  
caching tests to fail  
Splits up the Rails test harness files into tests that fail with MRI as  
well (which are often issues with the test)  
Changes the tests to load specific versions of Rails and other gems.  
Without that, you get load errors because of incorrect versions being  
loaded  
Changed the default of irtests to run the Rails tests. They can be  
skipped by using -m (for “minimum”)