Forum: IronRuby Code Review: Fix for File.print misbehavior

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
Shay F. (Guest)
on 2009-05-15 19:20
Commit Description:
Using the File.print method after the global input record separator had
been changed generated a different output than the MRI.

For example, consider the next code block:
$\ = ">>"
file =  File.open("a.txt","w")
file.print "One ", 1, ", Two ", 2
file.close

Output file on MRI: One 1, Two 2>>
Output file on IronRuby: One >>1>>, Two >>2>>

This commit fixes this behavior.

Commit site:
http://github.com/shayfriedman/ironruby/commit/0cc...

Files changed:
Merlin/Main/Languages/Ruby/Libraries.LCA_RESTRICTED/Builtins/IoOps.cs

Thanks,
Shay.
Shri B. (Guest)
on 2009-05-15 22:48
(Received via mailing list)
Thanks for the first patch, Shay! Couple of questions and comments.

Have you signed the contributor agreement mentioned at the top of
http://wiki.github.com/ironruby/ironruby/contributing?

Please see
http://wiki.github.com/ironruby/ironruby/modifying... for
coding conventions. Opening braces should go on the same line as the
"if" statement.

You should make sure that there is a RubySpec spec for your fixes. In
this case, there should be a spec in
Merlin\External.LCA_RESTRICTED\Languages\IronRuby\mspec\rubyspec\core\io\print_spec.rb.
It does look like the example "writes each obj.to_s to the stream and
appends $\\ (if any) given multiple objects" corresponds to your fix. If
there is an existing spec, you should delete the corresponding tag from
Merlin\External.LCA_RESTRICTED\Languages\IronRuby\mspec\ironruby-tags\core\io\print_tags.txt.
Please delete the print_tags.txt file since there are not more tags left
in that file.

You did run irtests.bat, right? There are currently 5 known failures
from run.bat and 19 from the RubySpec specs (all of which should go away
with the next push from Microsoft TFS to GIT).

Thanks again!

Shri
Shay F. (Guest)
on 2009-05-16 09:50
Hi Shri,

Regarding the contributor agreement - yes, I've signed that (Jim D.
helped me with that).

Regarding the coding conventions - I didn't notice the "modifying the
sources" page, sorrry for that... I would pay attention to that next
time.

About RubySpec - "writes each obj.to_s to the stream and
appends $\\ (if any) given multiple objects" really seems like the
corresponding test to my fix. What are the tags for?
The test mentioned on print_tags.txt should also pass after this commit.

About irtests - I ran it. I saw the 5 failures from run.bat but I
couldn't find the place where the RubySpec specs failures were
presented. What am I doing wrong?

Thanks!
Shay.
Shri B. (Guest)
on 2009-05-16 10:32
(Received via mailing list)
"writes each obj.to_s to the stream and appends $\\ (if any) given
multiple objects" does not have a tag for it. If it did, it would be in
Merlin\External.LCA_RESTRICTED\Languages\IronRuby\mspec\ironruby-tags\core\io\print_tags.txt,
but that file only has a tag for "writes $_.to_s followed by $\ (if any)
to the stream if no arguments given". I incorrectly thought that these
were the same test name, but clearly they are not.

It's a mystery how "writes each obj.to_s to the stream and appends $\\
(if any) given multiple objects" works, but your example below fails.
Could you could step into IoOps.Print for this test without your fix and
see how it works? Else, I can try it next week.

Shri
Shay F. (Guest)
on 2009-05-16 11:45
I couldn't follow the code exactly but what I did find, was that this
test never reached the code inside IOOps.Print.
The behavior is really strange, I tried to print the value on the screen
and the value were really incorrect (the $\ delimiter is added after
each argument) but the $stdout.print call inside the lambda expression
got it correctly...

I did that:

  it "writes each obj.to_s to the stream and appends $\\ (if any) given
multiple

objects" do
    o, o2 = Object.new, Object.new
    def o.to_s(); 'o'; end
    def o2.to_s(); 'o2'; end

    puts $stdout.print(o, o2)

    lambda { $stdout.print(o, o2) }.should
output("#{o.to_s}#{o2.to_s}#{$\}")
  end


And it wrote "o->o2->nil" on the screen... But when I changed the output
it to "123" the test failed and told me:
Expected:
  $stdout: "123"
      got:
  $stdout: "oo2->"

So I'm really confused... like I said, my breakpoint inside the print
method never gets hit so I can't really tell which "print" method is
invoked...

Shay.
Shri B. (Guest)
on 2009-05-17 02:10
(Received via mailing list)
The problem is because of mspec itself. In
External.LCA_RESTRICTED\Languages\IronRuby\mspec\mspec\lib\mspec\matchers\output.rb,
the code sets $stdout to a helper class (IOStub) so that it can capture
the output. However, what this means is that calls to $stdout.print that
the test makes now go to IOStub#print rather than to the real
implementation of IO#print, and this masks the bug.

A better solution would be to use StringIO in matchers\output.rb rather
than IOStub. If you are included to pursue this (which would be great),
you could try the change and/or send email to
http://groups.google.com/group/rubyspec to discuss the change. However,
since this is a preexisting issue, you could leave it for another day if
you want.

So consider your change code-reviewed, and you can chose separately to
pursue (or not) the issue with matchers\output.rb.
This topic is locked and can not be replied to.