Allow seek when underlying string is frozen #121
Conversation
seek only modifies the StringIO pos, and should work when the underlying String is frozen. Fixes ruby#119
|
Sorry @kou I missed this one because nothing tested I add a test here and fix the JRuby issue (#119). Release is not critical, since this only blocks us upgrading JRuby master. It would be good to resolve #120 before doing the next release (I have started work in #122). |
test/stringio/test_stringio.rb
Outdated
| @@ -483,6 +483,11 @@ def test_seek | |||
| f.close unless f.closed? | |||
| end | |||
|
|
|||
| def test_seek_frozen_string | |||
| f = StringIO.new(-"1234") | |||
| assert_nothing_raised { f.seek(0) } | |||
There was a problem hiding this comment.
In general, we should not use assert_nothing_raised because it doesn't nothing. We should check the read behavior instead. How about the following?
| assert_nothing_raised { f.seek(0) } | |
| f.seek(0) | |
| assert_equal("234", f.read) |
There was a problem hiding this comment.
Good point! I modified the test to check the return value of seek.
I will modify the tests in #122 as well but maybe not until there's some feedback on the ruby-lang bug.
headius
force-pushed
the
seek_frozen_string
branch
from
b946d46 to
ce84d17
Compare
|
@kou Perhaps after this is merged we should expand the test to other StringIO methods that we expect to work with a frozen string, as well as a test for the ones that should error. |
(ruby/stringio#121) Fixes ruby/stringio#119. Adds a test for this expectation. ruby/stringio@3f90fe44c6
|
I agree with you. |
This updates stringio to 3.1.5, which includes all the fixes from ruby/stringio#116 and the regression ruby/stringio#119 fixed by ruby/stringio#121. All CRuby tests and ruby/spec specs for stringio are now green. Tests are from v3.1.5 of the ruby/stringio repo.
(ruby/stringio#121) Fixes ruby/stringio#119. Adds a test for this expectation. ruby/stringio@3f90fe44c6
Fixes #119. Adds a test for this expectation.