Code Review: Bug Regressions and .NET interop

Some more changes. Mostly closing bugs and adding .NET interop tests.
Commit range is 90392fff to a39daeae on
git://github.com/jredville/ironruby.git. I rebased this time, so you
should be able to just grab that range and see only my diffs.

  •     closing Rubyforge 
    

15060http://github.com/jredville/ironruby/commit/90392fff68b62c20b518507ee4e5766a2067290f

  •     [#24589] $PROGRAM_NAME in 'English' not working now 
    

fixedhttp://github.com/jredville/ironruby/commit/ab8a90b2f62c8bd347a7db6a142aba13d6997767

  •     making $0 the same object as $PROGRAM_NAME and adding specs 
    

for it. Closes [#24589] $PROGRAM_NAME in ‘English’ not
workinghttp://github.com/jredville/ironruby/commit/0619c3b38856614dfefeea1d19f539fd0e440284

  •     closing Rubyforge [#15061] tracking: create generic interface 
    

typehttp://github.com/jredville/ironruby/commit/b1cae6307a3a151195442c4224e1c6cc6372c139

  •     closing RubyForge [#15651] NoMethodError expected when calling 
    

the private method ‘initialize’ of a ruby
classhttp://github.com/jredville/ironruby/commit/35745f6f947da80d2b1669de00f8703d118bd05a

  •     added protected method invocation specs. Closes Rubyforge 
    

[#19118] Protected members not available when inheriting from CLR
typehttp://github.com/jredville/ironruby/commit/167197cb78cb81590be9ae2fe48a953306070374

  •     adding regression test for [#19872] IComparableOps.CompareTo 
    

throws argument error when type is
Decimalhttp://github.com/jredville/ironruby/commit/d5e75cf6e129b8f415c9e90dd33ea8f1eaf9df21

  •     added reference support to 
    

csc.bathttp://github.com/jredville/ironruby/commit/15cc5fe83bd36b2557a4fc487e447c3681434a99

  •     closing [#17467] Accessing IronRuby method from 
    

C#http://github.com/jredville/ironruby/commit/6c31b0834dcccb188a331910cc925054a2a97bfc

  •     closing [#19950] Dir.glob doesn't handle 
    

**http://github.com/jredville/ironruby/commit/125638724c04f4743d7807746424d6d04855c643

  •     closing [#20027] Formatting float to string produces 
    

incorrectly
"Infinity"http://github.com/jredville/ironruby/commit/d428c0bcb6b09e1d467cb457a2d540fb9dd4ff21

  •     closing [#20043] creating a generic type when the type is an 
    

interfacehttp://github.com/jredville/ironruby/commit/bf83e3af66c5e97ad15c35571e8299a1ff333657

  •     closing [#20052] Calculating 3.0/2.0 produces 
    

NaNhttp://github.com/jredville/ironruby/commit/40d5b7aaf2915b4a19bf5219cd777a66a6569a52

  •     closing Rubyforge [#20263] Dir.glob doesn't handle missing 
    

foldershttp://github.com/jredville/ironruby/commit/085a6e79e67eda0c04de3473078b4744e208fc18

  •     closing [#20410] 
    

GetKCodeNamehttp://github.com/jredville/ironruby/commit/d52203496f833b8659909fac023af4ca5ab38629

  •     adding test to ensure that File.stat returns a File::Stat 
    

objecthttp://github.com/jredville/ironruby/commit/de0e2a717206eb71ee628fa9941fb64c77565a8c

  •     closing Rubyforge [#20640] Calling File.open with an integer 
    

(File descriptor overload) causes the File.open site to be
unusablehttp://github.com/jredville/ironruby/commit/a0937f07557f26cc44708bfd21f374a4eea07dcf

  •     closing Rubyforge [#20664] Respecting visibility of methods. 
    

Also fixes a compilation error due to multiple IInterface
definitionshttp://github.com/jredville/ironruby/commit/d38765a5d2360dbb4bd5cfc3ba71e8221fab4687

  •     Closing RubyForge [#20665] Can't use an indexer on a WPF 
    

ResourceDictionaryhttp://github.com/jredville/ironruby/commit/c9830e5b0a3616718d4654bfb424aef160ed29ec

  •     removing a tag for 
    

predefinedhttp://github.com/jredville/ironruby/commit/388c1fa61db2fbe32fd375e6c5a396a40628239c

  •     closing Rubyforge [#21943] NullRef exception thrown while 
    

comparing a list containing elements with overriden
==http://github.com/jredville/ironruby/commit/585ab45ddc6fc0beab45fa5c594ba9982aa3a775

  •     closing Rubyforge [#21995] kind_of? not working with when 
    

extend is
usedhttp://github.com/jredville/ironruby/commit/c288dbb7cecefc524d1f24557b57aa54c601d93c

  •     recommiting some changes that were reverted by the merge. 
    

Methinks someone in TFS changed them. This makes describe work with 2
strings for shared behaviors, and it makes csc output #line
pragmashttp://github.com/jredville/ironruby/commit/ac44e0d6d4ae8dbdfd9b92b2125f69e2b53d1276

  •     fixing some more errors from the 
    

mergehttp://github.com/jredville/ironruby/commit/c42e84c21020b6e206f8c84899227eb6e3d2bd7c

  •     Adding class instantiation specs for regular classes with 
    

overloaded
constructorshttp://github.com/jredville/ironruby/commit/eba230a507e88ed6a9833671ce037cdd1ad1a594

  •     adding regression tests for [#22197] calling super throw 
    

Argument
Errorhttp://github.com/jredville/ironruby/commit/9313a99120a518375022ccb4f8a99c5f479f118c

  •     adding StringBuilder specs to get rid of test_basic.rb. Also 
    

added equal_clr_string
matcherhttp://github.com/jredville/ironruby/commit/09d04d656ad9b2621808c23261f228db69660a76

  •     refactor specs to use the equal_clr_string 
    

matcherhttp://github.com/jredville/ironruby/commit/410fabd0582a9e1573118f7d15a057cf10bd44be

  •     basic string specs to remove 
    

test_basic.rbhttp://github.com/jredville/ironruby/commit/044752ab3fc23e41798f84ac45a41f469ed7a4e6

  •     added more string specs to get rid of test_basic. Also adds 
    

field
specshttp://github.com/jredville/ironruby/commit/de239fa9dceb12a4a9bbb0d200ca699fedd8000d

  •     adding some basic event add and remove 
    

specshttp://github.com/jredville/ironruby/commit/bf37c14fe44d1a1ad3567399905c4551da3838b0

  •     added event invocation 
    

specshttp://github.com/jredville/ironruby/commit/a39daeae41da467c03f2c38d0e6b68635fc3adc4

JD

Posted comments on github, but here they are also:

  •      'represents the program name' looks strange, asserting that 
    

the splitted string (?) equals a array with two elements, both
containing the expanded path the string represented. How does that make
sense? Jim explained this one, and now it makes sense. He’s adding a
comment to explain the witchery

  •     Catching NoMethodError to check for a private method is not 
    

enough, you should also ensure that klass.method(:initialize) gives you
back a Method object, and that
klass.private_methods.include?(‘initialize’). Given those three pieces
of info you can better infer that the method exists and it is private.

  •     Make protected_spec less redundant (see comment on 
    

http://github.com/jredville/ironruby/commit/167197cb78cb81590be9ae2fe48a953306070374#comment_18110)

  •     Indent is off on line 10 in Interop/fixtures.generated.cs (and 
    

seems to be for all generated C#).

  •     What's the debug("reference", ref) call doing in csc.rb? (that 
    

name cracks me up)

  •     To answer the TODO in conversion_spec.rb: it should belong in 
    

the tests for using IronRuby through the hosting API. I don’t think we
have any, and maybe they should exist in our .NET interop?
Interop/dlr/…

  •     Is "truncate" the correct terminology in modulo_spec.rb? How 
    

about “rounds”?

  •     There's no spec for the obj.my_event { ... } syntax
    

Otherwise, good job =)

From: Jim D.
Sent: Friday, April 03, 2009 9:45 AM
To: [email protected]; IronRuby External Code R.
Subject: Code Review: Bug Regressions and .NET interop

Some more changes. Mostly closing bugs and adding .NET interop tests.
Commit range is 90392fff to a39daeae on
git://github.com/jredville/ironruby.git. I rebased this time, so you
should be able to just grab that range and see only my diffs.

  •     closing Rubyforge 
    

15060http://github.com/jredville/ironruby/commit/90392fff68b62c20b518507ee4e5766a2067290f

  •     [#24589] $PROGRAM_NAME in 'English' not working now 
    

fixedhttp://github.com/jredville/ironruby/commit/ab8a90b2f62c8bd347a7db6a142aba13d6997767

  •     making $0 the same object as $PROGRAM_NAME and adding specs 
    

for it. Closes [#24589] $PROGRAM_NAME in ‘English’ not
workinghttp://github.com/jredville/ironruby/commit/0619c3b38856614dfefeea1d19f539fd0e440284

  •     closing Rubyforge [#15061] tracking: create generic interface 
    

typehttp://github.com/jredville/ironruby/commit/b1cae6307a3a151195442c4224e1c6cc6372c139

  •     closing RubyForge [#15651] NoMethodError expected when calling 
    

the private method ‘initialize’ of a ruby
classhttp://github.com/jredville/ironruby/commit/35745f6f947da80d2b1669de00f8703d118bd05a

  •     added protected method invocation specs. Closes Rubyforge 
    

[#19118] Protected members not available when inheriting from CLR
typehttp://github.com/jredville/ironruby/commit/167197cb78cb81590be9ae2fe48a953306070374

  •     adding regression test for [#19872] IComparableOps.CompareTo 
    

throws argument error when type is
Decimalhttp://github.com/jredville/ironruby/commit/d5e75cf6e129b8f415c9e90dd33ea8f1eaf9df21

  •     added reference support to 
    

csc.bathttp://github.com/jredville/ironruby/commit/15cc5fe83bd36b2557a4fc487e447c3681434a99

  •     closing [#17467] Accessing IronRuby method from 
    

C#http://github.com/jredville/ironruby/commit/6c31b0834dcccb188a331910cc925054a2a97bfc

  •     closing [#19950] Dir.glob doesn't handle 
    

**http://github.com/jredville/ironruby/commit/125638724c04f4743d7807746424d6d04855c643

  •     closing [#20027] Formatting float to string produces 
    

incorrectly
"Infinity"http://github.com/jredville/ironruby/commit/d428c0bcb6b09e1d467cb457a2d540fb9dd4ff21

  •     closing [#20043] creating a generic type when the type is an 
    

interfacehttp://github.com/jredville/ironruby/commit/bf83e3af66c5e97ad15c35571e8299a1ff333657

  •     closing [#20052] Calculating 3.0/2.0 produces 
    

NaNhttp://github.com/jredville/ironruby/commit/40d5b7aaf2915b4a19bf5219cd777a66a6569a52

  •     closing Rubyforge [#20263] Dir.glob doesn't handle missing 
    

foldershttp://github.com/jredville/ironruby/commit/085a6e79e67eda0c04de3473078b4744e208fc18

  •     closing [#20410] 
    

GetKCodeNamehttp://github.com/jredville/ironruby/commit/d52203496f833b8659909fac023af4ca5ab38629

  •     adding test to ensure that File.stat returns a File::Stat 
    

objecthttp://github.com/jredville/ironruby/commit/de0e2a717206eb71ee628fa9941fb64c77565a8c

  •     closing Rubyforge [#20640] Calling File.open with an integer 
    

(File descriptor overload) causes the File.open site to be
unusablehttp://github.com/jredville/ironruby/commit/a0937f07557f26cc44708bfd21f374a4eea07dcf

  •     closing Rubyforge [#20664] Respecting visibility of methods. 
    

Also fixes a compilation error due to multiple IInterface
definitionshttp://github.com/jredville/ironruby/commit/d38765a5d2360dbb4bd5cfc3ba71e8221fab4687

  •     Closing RubyForge [#20665] Can't use an indexer on a WPF 
    

ResourceDictionaryhttp://github.com/jredville/ironruby/commit/c9830e5b0a3616718d4654bfb424aef160ed29ec

  •     removing a tag for 
    

predefinedhttp://github.com/jredville/ironruby/commit/388c1fa61db2fbe32fd375e6c5a396a40628239c

  •     closing Rubyforge [#21943] NullRef exception thrown while 
    

comparing a list containing elements with overriden
==http://github.com/jredville/ironruby/commit/585ab45ddc6fc0beab45fa5c594ba9982aa3a775

  •     closing Rubyforge [#21995] kind_of? not working with when 
    

extend is
usedhttp://github.com/jredville/ironruby/commit/c288dbb7cecefc524d1f24557b57aa54c601d93c

  •     recommiting some changes that were reverted by the merge. 
    

Methinks someone in TFS changed them. This makes describe work with 2
strings for shared behaviors, and it makes csc output #line
pragmashttp://github.com/jredville/ironruby/commit/ac44e0d6d4ae8dbdfd9b92b2125f69e2b53d1276

  •     fixing some more errors from the 
    

mergehttp://github.com/jredville/ironruby/commit/c42e84c21020b6e206f8c84899227eb6e3d2bd7c

  •     Adding class instantiation specs for regular classes with 
    

overloaded
constructorshttp://github.com/jredville/ironruby/commit/eba230a507e88ed6a9833671ce037cdd1ad1a594

  •     adding regression tests for [#22197] calling super throw 
    

Argument
Errorhttp://github.com/jredville/ironruby/commit/9313a99120a518375022ccb4f8a99c5f479f118c

  •     adding StringBuilder specs to get rid of test_basic.rb. Also 
    

added equal_clr_string
matcherhttp://github.com/jredville/ironruby/commit/09d04d656ad9b2621808c23261f228db69660a76

  •     refactor specs to use the equal_clr_string 
    

matcherhttp://github.com/jredville/ironruby/commit/410fabd0582a9e1573118f7d15a057cf10bd44be

  •     basic string specs to remove 
    

test_basic.rbhttp://github.com/jredville/ironruby/commit/044752ab3fc23e41798f84ac45a41f469ed7a4e6

  •     added more string specs to get rid of test_basic. Also adds 
    

field
specshttp://github.com/jredville/ironruby/commit/de239fa9dceb12a4a9bbb0d200ca699fedd8000d

  •     adding some basic event add and remove 
    

specshttp://github.com/jredville/ironruby/commit/bf37c14fe44d1a1ad3567399905c4551da3838b0

  •     added event invocation 
    

specshttp://github.com/jredville/ironruby/commit/a39daeae41da467c03f2c38d0e6b68635fc3adc4

JD

From: Jimmy S.
Sent: Friday, April 03, 2009 1:47 PM
To: Jim D.; [email protected]; IronRuby External Code
Reviewers
Subject: RE: Code Review: Bug Regressions and .NET interop

Posted comments on github, but here they are also:

  •      'represents the program name' looks strange, asserting that 
    

the splitted string (?) equals a array with two elements, both
containing the expanded path the string represented. How does that make
sense? Jim explained this one, and now it makes sense. He’s adding a
comment to explain the witchery

Comment added

  •     Catching NoMethodError to check for a private method is not 
    

enough, you should also ensure that klass.method(:initialize) gives you
back a Method object, and that
klass.private_methods.include?(‘initialize’). Given those three pieces
of info you can better infer that the method exists and it is private.

Fixed

  •     Make protected_spec less redundant (see comment on 
    

http://github.com/jredville/ironruby/commit/167197cb78cb81590be9ae2fe48a953306070374#comment_18110)

I’m going to punt this one, because I want to extract either a shared
behavior or a matcher out of these specs. I’ll have that in the next
push.

  •     Indent is off on line 10 in Interop/fixtures.generated.cs (and 
    

seems to be for all generated C#).

Yeah, the generated indentation isn’t perfect. I’ll look into it.

  •     What's the debug("reference", ref) call doing in csc.rb? (that 
    

name cracks me up)

Glad you like the name. debug(str, obj) is a debug method that prints
“#{str}: #{obj.to_s}” to stdout if $DEBUG is true

  •     To answer the TODO in conversion_spec.rb: it should belong in 
    

the tests for using IronRuby through the hosting API. I don’t think we
have any, and maybe they should exist in our .NET interop?
Interop/dlr/…

I’ll think about this and make the move if needed.

  •     Is "truncate" the correct terminology in modulo_spec.rb? How 
    

about “rounds”?

Fixed

  •     There's no spec for the obj.my_event { ... } syntax
    

Fixed

Otherwise, good job =)

From: Jim D.
Sent: Friday, April 03, 2009 9:45 AM
To: [email protected]; IronRuby External Code R.
Subject: Code Review: Bug Regressions and .NET interop

Some more changes. Mostly closing bugs and adding .NET interop tests.
Commit range is 90392fff to a39daeae on
git://github.com/jredville/ironruby.git. I rebased this time, so you
should be able to just grab that range and see only my diffs.

  •     closing Rubyforge 
    

15060http://github.com/jredville/ironruby/commit/90392fff68b62c20b518507ee4e5766a2067290f

  •     [#24589] $PROGRAM_NAME in 'English' not working now 
    

fixedhttp://github.com/jredville/ironruby/commit/ab8a90b2f62c8bd347a7db6a142aba13d6997767

  •     making $0 the same object as $PROGRAM_NAME and adding specs 
    

for it. Closes [#24589] $PROGRAM_NAME in ‘English’ not
workinghttp://github.com/jredville/ironruby/commit/0619c3b38856614dfefeea1d19f539fd0e440284

  •     closing Rubyforge [#15061] tracking: create generic interface 
    

typehttp://github.com/jredville/ironruby/commit/b1cae6307a3a151195442c4224e1c6cc6372c139

  •     closing RubyForge [#15651] NoMethodError expected when calling 
    

the private method ‘initialize’ of a ruby
classhttp://github.com/jredville/ironruby/commit/35745f6f947da80d2b1669de00f8703d118bd05a

  •     added protected method invocation specs. Closes Rubyforge 
    

[#19118] Protected members not available when inheriting from CLR
typehttp://github.com/jredville/ironruby/commit/167197cb78cb81590be9ae2fe48a953306070374

  •     adding regression test for [#19872] IComparableOps.CompareTo 
    

throws argument error when type is
Decimalhttp://github.com/jredville/ironruby/commit/d5e75cf6e129b8f415c9e90dd33ea8f1eaf9df21

  •     added reference support to 
    

csc.bathttp://github.com/jredville/ironruby/commit/15cc5fe83bd36b2557a4fc487e447c3681434a99

  •     closing [#17467] Accessing IronRuby method from 
    

C#http://github.com/jredville/ironruby/commit/6c31b0834dcccb188a331910cc925054a2a97bfc

  •     closing [#19950] Dir.glob doesn't handle 
    

**http://github.com/jredville/ironruby/commit/125638724c04f4743d7807746424d6d04855c643

  •     closing [#20027] Formatting float to string produces 
    

incorrectly
"Infinity"http://github.com/jredville/ironruby/commit/d428c0bcb6b09e1d467cb457a2d540fb9dd4ff21

  •     closing [#20043] creating a generic type when the type is an 
    

interfacehttp://github.com/jredville/ironruby/commit/bf83e3af66c5e97ad15c35571e8299a1ff333657

  •     closing [#20052] Calculating 3.0/2.0 produces 
    

NaNhttp://github.com/jredville/ironruby/commit/40d5b7aaf2915b4a19bf5219cd777a66a6569a52

  •     closing Rubyforge [#20263] Dir.glob doesn't handle missing 
    

foldershttp://github.com/jredville/ironruby/commit/085a6e79e67eda0c04de3473078b4744e208fc18

  •     closing [#20410] 
    

GetKCodeNamehttp://github.com/jredville/ironruby/commit/d52203496f833b8659909fac023af4ca5ab38629

  •     adding test to ensure that File.stat returns a File::Stat 
    

objecthttp://github.com/jredville/ironruby/commit/de0e2a717206eb71ee628fa9941fb64c77565a8c

  •     closing Rubyforge [#20640] Calling File.open with an integer 
    

(File descriptor overload) causes the File.open site to be
unusablehttp://github.com/jredville/ironruby/commit/a0937f07557f26cc44708bfd21f374a4eea07dcf

  •     closing Rubyforge [#20664] Respecting visibility of methods. 
    

Also fixes a compilation error due to multiple IInterface
definitionshttp://github.com/jredville/ironruby/commit/d38765a5d2360dbb4bd5cfc3ba71e8221fab4687

  •     Closing RubyForge [#20665] Can't use an indexer on a WPF 
    

ResourceDictionaryhttp://github.com/jredville/ironruby/commit/c9830e5b0a3616718d4654bfb424aef160ed29ec

  •     removing a tag for 
    

predefinedhttp://github.com/jredville/ironruby/commit/388c1fa61db2fbe32fd375e6c5a396a40628239c

  •     closing Rubyforge [#21943] NullRef exception thrown while 
    

comparing a list containing elements with overriden
==http://github.com/jredville/ironruby/commit/585ab45ddc6fc0beab45fa5c594ba9982aa3a775

  •     closing Rubyforge [#21995] kind_of? not working with when 
    

extend is
usedhttp://github.com/jredville/ironruby/commit/c288dbb7cecefc524d1f24557b57aa54c601d93c

  •     recommiting some changes that were reverted by the merge. 
    

Methinks someone in TFS changed them. This makes describe work with 2
strings for shared behaviors, and it makes csc output #line
pragmashttp://github.com/jredville/ironruby/commit/ac44e0d6d4ae8dbdfd9b92b2125f69e2b53d1276

  •     fixing some more errors from the 
    

mergehttp://github.com/jredville/ironruby/commit/c42e84c21020b6e206f8c84899227eb6e3d2bd7c

  •     Adding class instantiation specs for regular classes with 
    

overloaded
constructorshttp://github.com/jredville/ironruby/commit/eba230a507e88ed6a9833671ce037cdd1ad1a594

  •     adding regression tests for [#22197] calling super throw 
    

Argument
Errorhttp://github.com/jredville/ironruby/commit/9313a99120a518375022ccb4f8a99c5f479f118c

  •     adding StringBuilder specs to get rid of test_basic.rb. Also 
    

added equal_clr_string
matcherhttp://github.com/jredville/ironruby/commit/09d04d656ad9b2621808c23261f228db69660a76

  •     refactor specs to use the equal_clr_string 
    

matcherhttp://github.com/jredville/ironruby/commit/410fabd0582a9e1573118f7d15a057cf10bd44be

  •     basic string specs to remove 
    

test_basic.rbhttp://github.com/jredville/ironruby/commit/044752ab3fc23e41798f84ac45a41f469ed7a4e6

  •     added more string specs to get rid of test_basic. Also adds 
    

field
specshttp://github.com/jredville/ironruby/commit/de239fa9dceb12a4a9bbb0d200ca699fedd8000d

  •     adding some basic event add and remove 
    

specshttp://github.com/jredville/ironruby/commit/bf37c14fe44d1a1ad3567399905c4551da3838b0

  •     added event invocation 
    

specshttp://github.com/jredville/ironruby/commit/a39daeae41da467c03f2c38d0e6b68635fc3adc4

JD