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/633db5fb/attachment.html>
Utr.rb: Please replace @disabled = 0 if @disabled == nil with @disabled ||= 0 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. Generate_test-unit_tags.rb: Couldn''t all of ensure_single_fault_per_method_name be accomplished by: faults.map {|e| test_method_name(e)}.uniq Rest looks good. JD 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/c85b1097/attachment.html>
Inline... 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 Utr.rb: 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. Generate_test-unit_tags.rb: 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. JD 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>
>Generate_test-unit_tags.rb: >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.I thought include used the same equal call as uniq, or is there something else going on here? JD From: Shri Borde Sent: Monday, January 25, 2010 11:10 AM To: Jim Deville; IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org Subject: RE: Review: ActiveRecord tests Inline... 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 Utr.rb: 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. Generate_test-unit_tags.rb: 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. JD 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/35b2d7c1/attachment.html>
"faults.map {|e| test_method_name(e)}.uniq" will give you a unique set of method names. I need an array of faults with a unique set of method names. If uniq took a block like sort does, that would work. From: Jim Deville Sent: Monday, January 25, 2010 11:18 AM To: Shri Borde; IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org Subject: RE: Review: ActiveRecord tests>Generate_test-unit_tags.rb: >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.I thought include used the same equal call as uniq, or is there something else going on here? JD From: Shri Borde Sent: Monday, January 25, 2010 11:10 AM To: Jim Deville; IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org Subject: RE: Review: ActiveRecord tests Inline... 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 Utr.rb: 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. Generate_test-unit_tags.rb: 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. JD 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/e297a995/attachment.html>
My bad, I see what''s going on. You are rejecting faults based on the condition, not the string method names. Looks good. JD From: Shri Borde Sent: Monday, January 25, 2010 12:21 PM To: Jim Deville; IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org Subject: RE: Review: ActiveRecord tests "faults.map {|e| test_method_name(e)}.uniq" will give you a unique set of method names. I need an array of faults with a unique set of method names. If uniq took a block like sort does, that would work. From: Jim Deville Sent: Monday, January 25, 2010 11:18 AM To: Shri Borde; IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org Subject: RE: Review: ActiveRecord tests>Generate_test-unit_tags.rb: >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.I thought include used the same equal call as uniq, or is there something else going on here? JD From: Shri Borde Sent: Monday, January 25, 2010 11:10 AM To: Jim Deville; IronRuby External Code Reviewers Cc: ironruby-core at rubyforge.org Subject: RE: Review: ActiveRecord tests Inline... 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 Utr.rb: 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. Generate_test-unit_tags.rb: 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. JD 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/113f80ec/attachment-0001.html>