Forum: IronRuby Review: ActiveRecord tests

tfpt review /shelveset:ar;sborde

Enables active_record tests in irtests.rb. They require SQLExpress to be 
installed on the machine.

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@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.

Inline...

From: Jim Deville
Sent: Monday, January 25, 2010 10:22 AM
To: Shri Borde; IronRuby External Code Reviewers
Cc: ironruby-core@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@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.

>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@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@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@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.

"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@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@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@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@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.

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@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@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@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@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@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.