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") -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20100113/92fce4b1/attachment.html>
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 Borde Sent: Tuesday, January 12, 2010 4:14 PM To: IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org 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") -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20100113/db55868a/attachment-0001.html>
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 Schementi Sent: Tuesday, January 12, 2010 7:05 PM To: Shri Borde; IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org 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 Borde Sent: Tuesday, January 12, 2010 4:14 PM To: IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org 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") -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20100113/78e363c5/attachment-0001.html>
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 Borde Sent: Wednesday, January 13, 2010 9:42 AM To: Jimmy Schementi; IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org 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 Schementi Sent: Tuesday, January 12, 2010 7:05 PM To: Shri Borde; IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org 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 Borde Sent: Tuesday, January 12, 2010 4:14 PM To: IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org 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") -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20100113/d7e1c3c1/attachment-0001.html>
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 Deville
Sent: Wednesday, January 13, 2010 9:54 AM
To: Shri Borde; Jimmy Schementi; IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
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 Borde
Sent: Wednesday, January 13, 2010 9:42 AM
To: Jimmy Schementi; IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
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 Schementi
Sent: Tuesday, January 12, 2010 7:05 PM
To: Shri Borde; IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
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 Borde
Sent: Tuesday, January 12, 2010 4:14 PM
To: IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
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")
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/ironruby-core/attachments/20100113/8321fc3e/attachment-0001.html>
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 Matousek
Sent: Wednesday, January 13, 2010 10:12 AM
To: Jim Deville; Shri Borde; Jimmy Schementi; IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
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 Deville
Sent: Wednesday, January 13, 2010 9:54 AM
To: Shri Borde; Jimmy Schementi; IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
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 Borde
Sent: Wednesday, January 13, 2010 9:42 AM
To: Jimmy Schementi; IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
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 Schementi
Sent: Tuesday, January 12, 2010 7:05 PM
To: Shri Borde; IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
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 Borde
Sent: Tuesday, January 12, 2010 4:14 PM
To: IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
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")
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/ironruby-core/attachments/20100113/8f98f3f3/attachment-0001.html>