Add and update specs for IO methods#1384
Conversation
Verify that calling IO#binmode on an IO stream disables any active newline conversion (such as universal newline mode) when reading data.
- Add spec for reading a byte with IO#getbyte after calling ungetc on a stream without character conversion.
- Add spec verifying that calling IO#getbyte after ungetc on a stream with character conversion enabled raises IOError ("byte oriented read for character buffered IO").
- Add spec verifying IO#internal_encoding returns nil when external encoding is BINARY. - Add Ruby version-specific specs for setting internal encoding when external encoding is BINARY: Ruby < 3.3 returns the assigned value, while Ruby >= 3.3 ignores the internal encoding and returns nil because binary mode does not support internal character conversion.
Verify that calling IO#pwrite with a negative offset raises Errno::EINVAL.
- Add spec for CRuby Bug #18421 (fixed in 3.0.4) ensuring IO#read_nonblock(0, buffer) clears the output buffer and returns it empty when length is 0.
- Update IOError expectation message check when read_nonblock is called after ungetc with character conversion enabled ("byte oriented read for character buffered IO").
- Add specs verifying that IO#read successfully reads bytes after calling ungetc on streams without character conversion.
- Add specs across various mode combinations verifying that IO#read raises IOError ("byte oriented read for character buffered IO") after ungetc when character conversion is enabled.
- Add spec for IO#readbyte after ungetc on a stream without character conversion.
- Add spec verifying that IO#readbyte raises IOError ("byte oriented read for character buffered IO") after ungetc when character conversion is enabled.
…er conversion
Verify that calling IO#readpartial after ungetc on an IO stream with character conversion enabled raises IOError ("byte oriented read for character buffered IO").
- Add spec verifying IO#reopen raises ArgumentError when passed excess arguments.
- Add specs for reopening an IO with a String path, verifying that the newly opened stream preserves the capabilities and restrictions of the original open mode ('r', 'w', 'r+', 'a').
When ungetc pushes characters into the IO stream's character buffer, calling IO#seek should discard that buffer before seeking to the target offset. This adds a spec for CRuby Bug #20919 fixed in Ruby 3.4.
- Refactor `IO#set_encoding when passed nil, nil` mode blocks ('r', 'rb', 'r+', 'w', 'w+', 'a', 'a+') to reuse shared behaviors `:io_set_encoding_common`, `:io_set_encoding_readonly`, and `:io_set_encoding_write`.
- Add spec for passing "-" as second argument to reset internal_encoding to nil.
- Add validation specs for non-ASCII compatible arguments, nil first argument with non-nil second argument, and invalid newline decorator option.
- Add specs for ASCII-incompatible encodings (handling with binmode, character conversion, write-only streams, and error raising on readable streams).
- Add Ruby 3.2+ spec for newline decorator in binary mode.
- Add Ruby 3.3+ specs for setting internal encoding when second argument is nil.
- Add spec for encoding names containing null bytes.
- Update IOError message expectation when sysread is called after buffered reads.
- Add spec verifying that calling sysread on an IO with a non-empty character buffer raises IOError ("byte oriented read for character buffered IO").
- Update IOError message expectation when sysseek is called after buffered reads.
- Add spec verifying that calling sysseek on an IO with a non-empty character buffer raises IOError ("sysseek for buffered IO").
- Refactor pipe test fixture setup to use before/after blocks for clean tear-down. - Add specs for Ruby < 3.2 verifying that non-blocking syswrite on a full pipe raises Errno::EAGAIN (or Errno::EWOULDBLOCK on Windows) instead of returning partial bytes.
- Add spec for IO#write writing binary data when external encoding is ASCII-8BIT. - Add specs verifying that IO#write performs newline conversion on binary data when no encoding or ASCII-8BIT encoding is specified alongside newline conversion options.
The implementation on Windows does not error in this case
andrykonchin
left a comment
There was a problem hiding this comment.
Will continue review later.
| @io.set_encoding("utf-8:ISO-8859-1", newline: :universal) | ||
| @io.binmode | ||
| @io.read.should == data | ||
| end |
There was a problem hiding this comment.
suggestion: #binmode affects not only the #read method but also some other "reading" methods - each, each_line, each_char, each_codepoint, getc, gets, readchar, readline, readlines.
Some "readable" methods aren't affected - e.g. sysread, pread, read_nonblock, readpartial, readbyte, each_byte).
There was a problem hiding this comment.
Are you asking for additional tests for each of those cases?
There was a problem hiding this comment.
I'm not asking for them as a requirement, but if you have the time and interest, extra tests for those methods would be very appreciated.
| -> do | ||
| @io.getbyte | ||
| end.should.raise(IOError, "byte oriented read for character buffered IO") | ||
| end |
There was a problem hiding this comment.
minor: this appears to be a CRuby-specific implementation detail (or an intentional design constraint) that other Ruby implementations, such as JRuby, TruffleRuby, or Natalie, might handle differently (e.g., by
permitting byte-oriented reading under these conditions).
CRuby already allows mixing byte-oriented and character-oriented read operations on streams with multibyte encodings (such as UTF-16BE). Semantically, calling ungetc does not make this behavior any worse or introduce additional issues.
There was a problem hiding this comment.
My understanding is that this project documents the behavior of CRuby, and this is indeed a CRuby behavior. I've not tried it on other implementations. Is that a requirement now?
| @io.set_encoding(Encoding::BINARY, Encoding::IBM437) | ||
| @io.internal_encoding.should == nil | ||
| end | ||
| end |
There was a problem hiding this comment.
suggestion: similar cases are tested in set_encoding_spec.rb so it may be a better place and for this one.
There was a problem hiding this comment.
Either place works, but is there a rule to help decide which is best?
There was a problem hiding this comment.
My rule of thumb is simply: Where is the most intuitive place for a developer to look for them first?
| -> { | ||
| @file.pwrite("foo", -1) | ||
| }.should.raise(Errno::EINVAL) | ||
| end |
There was a problem hiding this comment.
suggestion: Actually it's interesting what happens on Windows when given a negative value. Yeah, here we test POSIX/Windows system calls but it 's still a part of the CRuby public behavior.
There was a problem hiding this comment.
I had to add the guard here after my first attempt broke the tests on Windows. I don't know what the right behavior should be on Windows, so I just left it out. This may also have different behaviors on different Ruby implementations.
There was a problem hiding this comment.
pwrite raises Errno::EINVAL for offsets of -3 and below, but simply writes to the file at the current position when the offset is -1 or -2. Replacing -1 with -100 should make this test work on Windows as well.
I suspect this special handling of -1 and -2 is unintentional, so it probably doesn't make sense to add test cases for them.
javanthropus
left a comment
There was a problem hiding this comment.
Thanks for taking a look, @andrykonchin.
| -> do | ||
| @io.getbyte | ||
| end.should.raise(IOError, "byte oriented read for character buffered IO") | ||
| end |
There was a problem hiding this comment.
My understanding is that this project documents the behavior of CRuby, and this is indeed a CRuby behavior. I've not tried it on other implementations. Is that a requirement now?
| @io.set_encoding(Encoding::BINARY, Encoding::IBM437) | ||
| @io.internal_encoding.should == nil | ||
| end | ||
| end |
There was a problem hiding this comment.
Either place works, but is there a rule to help decide which is best?
| -> { | ||
| @file.pwrite("foo", -1) | ||
| }.should.raise(Errno::EINVAL) | ||
| end |
There was a problem hiding this comment.
I had to add the guard here after my first attempt broke the tests on Windows. I don't know what the right behavior should be on Windows, so I just left it out. This may also have different behaviors on different Ruby implementations.
|
|
||
| @io.external_encoding.should == nil | ||
| @io.internal_encoding.should == nil | ||
| end |
There was a problem hiding this comment.
Not sure if understand the reason of the changes in the test above.
This PR adds missing specs, updates error message expectations, refactors redundant test fixtures, and adds coverage for edge cases and version-specific behaviors across
core/io.Summary of Changes
IO#binmode: Added spec verifying that newline conversion is disabled when reading in binary mode.IO#getbyte,IO#read,IO#readbyte,IO#readpartial: Added specs for reading afterungetc, including specs verifying that byte-oriented methods raiseIOError("byte oriented read for character buffered IO") when character conversion is enabled.IO#internal_encoding: Added specs forBINARYexternal encoding, including Ruby 3.3+ behavior where internal encoding assignments are ignored.IO#pwrite: Added spec verifyingErrno::EINVALis raised on negative offsets.IO#read_nonblock: Added spec for CRuby Bug #18421 (fixed in 3.0.4) ensuring 0-length reads clear the buffer, and updatedIOErrorexpectation.IO#reopen: Added specs for excess argument validation and mode preservation (r,w,r+,a) when reopening with a String path.IO#seek: Added spec for CRuby Bug #20919 (fixed in 3.4) verifying character buffer is cleared afterungetc.IO#set_encoding: Refactoredset_encoding nil, nilmode blocks to reuse shared behaviors. Added specs for"-"internal encoding, ASCII compatibility validation, newline options, ASCII-incompatible encodings, and Ruby 3.2+/3.3+ version specific behaviors.IO#sysread,IO#sysseek: UpdatedIOErrorexpectation messages for buffered IO and added specs for non-empty character buffers.IO#syswrite: Refactored pipe test setup and added specs for non-blocking write behavior on full pipes (raisingErrno::EAGAIN/Errno::EWOULDBLOCKon Ruby < 3.2).IO#write: Added specs for writing binary data withASCII-8BITexternal encoding and newline conversion.Verification
Ran specs via
mspec core/io/:103 files, 1582 examples, 2459 expectations, 0 failures, 0 errors, 0 tagged