[Ironruby-core] Review: ActiveRecord tests

Shri Borde Shri.Borde at microsoft.com
Mon Jan 25 14:10:00 EST 2010


From: Jim Deville
Sent: Monday, January 25, 2010 10:22 AM
To: Shri Borde; IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
Subject: RE: Review: ActiveRecord tests

Please replace  @disabled = 0 if @disabled == nil with @disabled ||= 0
[Shri Borde] Sounds good

Why are we undefining methods instead of aliasing them to noop? I'm not pulled too strongly in either direction, but I'd like to know. If we are going to undef, then get rid of the noop definition.
[Shri Borde] Aliasing to noop will still causes setup and teardown of the test to run, and there can be (and are) errors in those methods, resulting in an error associated with the test. Undefing fixes this. Will get rid of noop.

Couldn't all of ensure_single_fault_per_method_name be accomplished by:

faults.map {|e| test_method_name(e)}.uniq
[Shri Borde] The issue is that there can be two different faults for a test, one a failure and one an error. I want to keep just one of these so that we emit just one "disable ClassName, :test_name" line. #uniq does not work because the faults will not compare as equal.

Rest looks good.


From: Shri Borde
Sent: Sunday, January 24, 2010 9:55 PM
To: IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
Subject: Review: ActiveRecord tests

tfpt review /shelveset:ar;sborde

Enables active_record tests in irtests.rb. They require SQLExpress to be installed on the machine.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20100125/20a45774/attachment-0001.html>

More information about the Ironruby-core mailing list