Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ensure_open_and_* methods to StringIO. #2246

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 12 additions & 3 deletions kernel/common/io19.rb
Expand Up @@ -280,7 +280,14 @@ def run
saved_pos = 0
end

@from.seek @offset, IO::SEEK_CUR if @offset
if @offset
if @from.respond_to?(:descriptor) && @from.descriptor != -1
@from.seek @offset, IO::SEEK_CUR
else
raise ArgumentError, "cannot specify src_offset for non-IO"
end
end


size = @length ? @length : 16384
bytes = 0
Expand All @@ -296,7 +303,9 @@ def run
# done reading
end

@to.flush
# Should only flush if @from and @to are both descended from IO
# in other words, MRI does not flush copy_stream from/to a StringIO
@to.flush if @from.kind_of?(IO) && @to.kind_of?(IO)
return bytes
ensure
if @from_io
Expand All @@ -305,7 +314,7 @@ def run
@from.close
end

@to.close unless @to_io
@to.close unless @to_io || @to.closed?
end
end

Expand Down
2 changes: 2 additions & 0 deletions lib/19/stringio.rb
Expand Up @@ -66,13 +66,15 @@ def check_readable
raise IOError, "not opened for reading" unless @readable
end

alias_method :ensure_open_and_readable, :check_readable
private :check_readable

def check_writable
raise IOError, "not opened for writing" unless @writable
raise IOError, "unable to modify data" if @__data__.string.frozen?
end

alias_method :ensure_open_and_writable, :check_writable
private :check_writable

def set_encoding(external, internal=nil, options=nil)
Expand Down
36 changes: 36 additions & 0 deletions spec/ruby/core/io/copy_stream_spec.rb
@@ -1,5 +1,6 @@
require File.expand_path('../../../spec_helper', __FILE__)
require File.expand_path('../fixtures/classes', __FILE__)
require 'stringio'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to include specs that depend on stuff in lib/ in spec/ruby/core. If we need specs for StringIO behavior here, they should go in spec/library/stringio

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, makes sense, although this is more about copy_stream's behaviour with an IO-ish object like stringio.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I get that this isn't really easy / straightforward in that sense, just that lib/ dependencies inside core specs aren't something we want, especially with goals like gemifying lib etc. that can get very hairy in the future. So we are strict about that.


ruby_version_is "1.9" do
describe :io_copy_stream_to_file, :shared => true do
Expand Down Expand Up @@ -46,6 +47,7 @@
describe :io_copy_stream_to_io, :shared => true do
it "copies the entire IO contents to the IO" do
IO.copy_stream(@object.from, @to_io)
@to_io.flush if @object.flush?
@to_name.should have_data(@content)
IO.copy_stream(@from_bigfile, @to_io)
@to_name.should have_data(@content + @content_bigfile)
Expand All @@ -59,6 +61,7 @@
it "starts writing at the destination IO's current position" do
@to_io.write("prelude ")
IO.copy_stream(@object.from, @to_io)
@to_io.flush if @object.flush?
@to_name.should have_data("prelude " + @content)
end

Expand All @@ -80,6 +83,7 @@

it "copies only length bytes when specified" do
IO.copy_stream(@object.from, @to_io, 8).should == 8
@to_io.flush if @object.flush?
@to_name.should have_data("Line one")
end
end
Expand Down Expand Up @@ -235,5 +239,37 @@
it_behaves_like :io_copy_stream_to_io, nil, IOSpecs::CopyStream
end
end

describe "from a string IO" do
before :each do
@from_io = StringIO.new(@content)
IOSpecs::CopyStream.from = @from_io
IOSpecs::CopyStream.flush = true
end

describe "to a file name" do
it_behaves_like :io_copy_stream_to_file, nil, IOSpecs::CopyStream

it "raises an error when an offset is specified" do
lambda { IO.copy_stream(@from_io, @to_name, 8, 4) }.should raise_error(ArgumentError)
end
end

describe "to an IO" do
before :each do
@to_io = new_io @to_name, "w:utf-8"
end

after :each do
@to_io.close
end

it_behaves_like :io_copy_stream_to_io, nil, IOSpecs::CopyStream

it "raises an error when an offset is specified" do
lambda { IO.copy_stream(@from_io, @to_io, 8, 4) }.should raise_error(ArgumentError)
end
end
end
end
end
10 changes: 9 additions & 1 deletion spec/ruby/core/io/fixtures/classes.rb
Expand Up @@ -121,7 +121,7 @@ def self.closed_io
io.close
io
end

# Creates a pipe-based IO fixture containing the specified
# contents
def self.pipe_fixture(content)
Expand All @@ -146,5 +146,13 @@ def self.from=(from)
def self.from
@from
end

def self.flush=(flush)
@flush = flush
end

def self.flush?
@flush
end
end
end