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 B.
Sent: Sunday, January 24, 2010 9:55 PM
To: IronRuby External Code R.
Cc: [email protected]
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 D.
Sent: Monday, January 25, 2010 10:22 AM
To: Shri B.; IronRuby External Code R.
Cc: [email protected]
Subject: RE: Review: ActiveRecord tests

Utr.rb:
Please replace @disabled = 0 if @disabled == nil with @disabled ||= 0
[Shri B.] 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 B.] 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 B.] 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 B.
Sent: Sunday, January 24, 2010 9:55 PM
To: IronRuby External Code R.
Cc: [email protected]
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 B.] 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 B.
Sent: Monday, January 25, 2010 11:10 AM
To: Jim D.; IronRuby External Code R.
Cc: [email protected]
Subject: RE: Review: ActiveRecord tests

Inline…

From: Jim D.
Sent: Monday, January 25, 2010 10:22 AM
To: Shri B.; IronRuby External Code R.
Cc: [email protected]
Subject: RE: Review: ActiveRecord tests

Utr.rb:
Please replace @disabled = 0 if @disabled == nil with @disabled ||= 0
[Shri B.] 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 B.] 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 B.] 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 B.
Sent: Sunday, January 24, 2010 9:55 PM
To: IronRuby External Code R.
Cc: [email protected]
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 B.
Sent: Monday, January 25, 2010 12:21 PM
To: Jim D.; IronRuby External Code R.
Cc: [email protected]
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 D.
Sent: Monday, January 25, 2010 11:18 AM
To: Shri B.; IronRuby External Code R.
Cc: [email protected]
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 B.] 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 B.
Sent: Monday, January 25, 2010 11:10 AM
To: Jim D.; IronRuby External Code R.
Cc: [email protected]
Subject: RE: Review: ActiveRecord tests

Inline…

From: Jim D.
Sent: Monday, January 25, 2010 10:22 AM
To: Shri B.; IronRuby External Code R.
Cc: [email protected]
Subject: RE: Review: ActiveRecord tests

Utr.rb:
Please replace @disabled = 0 if @disabled == nil with @disabled ||= 0
[Shri B.] 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 B.] 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 B.] 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 B.
Sent: Sunday, January 24, 2010 9:55 PM
To: IronRuby External Code R.
Cc: [email protected]
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 D.
Sent: Monday, January 25, 2010 11:18 AM
To: Shri B.; IronRuby External Code R.
Cc: [email protected]
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 B.] 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 B.
Sent: Monday, January 25, 2010 11:10 AM
To: Jim D.; IronRuby External Code R.
Cc: [email protected]
Subject: RE: Review: ActiveRecord tests

Inline…

From: Jim D.
Sent: Monday, January 25, 2010 10:22 AM
To: Shri B.; IronRuby External Code R.
Cc: [email protected]
Subject: RE: Review: ActiveRecord tests

Utr.rb:
Please replace @disabled = 0 if @disabled == nil with @disabled ||= 0
[Shri B.] 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 B.] 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 B.] 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 B.
Sent: Sunday, January 24, 2010 9:55 PM
To: IronRuby External Code R.
Cc: [email protected]
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.