sarankumarbaskar commented on code in PR #860:
URL: https://github.com/apache/commons-io/pull/860#discussion_r3435006869
##########
src/test/java/org/apache/commons/io/input/BoundedReaderTest.java:
##########
@@ -242,4 +243,42 @@ void testSkipTest() throws IOException {
assertEquals(-1, mr.read());
}
}
+
+ @Test
+ void testSkipDoesNotExceedBound() throws IOException {
+ try (BoundedReader mr = new BoundedReader(new
StringReader("01234567890"), 3)) {
+ assertEquals(3, mr.skip(100));
+ assertEquals(-1, mr.read());
+ }
+ }
+
+ @Test
+ void testSkipDoesNotOverflowCharsRead() throws IOException {
+ try (BoundedReader mr = new BoundedReader(new
StringReader("01234567890"), 3)) {
+ assertEquals(3, mr.skip(Long.MAX_VALUE));
+ assertEquals(-1, mr.read());
+ }
+ }
+
+ @Test
+ void testSkipUsesActualSkippedCount() throws IOException {
+ try (BoundedReader mr = new BoundedReader(new FilterReader(new
StringReader("01234567890")) {
+ @Override
+ public long skip(final long n) throws IOException {
+ return super.skip(Math.min(n, 2));
+ }
+ }, 5)) {
+ assertEquals(2, mr.skip(5));
+ assertEquals('2', mr.read());
+ }
+ }
+
+ @Test
+ void testSkipRespectsReadAheadLimit() throws IOException {
+ try (BoundedReader mr = new BoundedReader(new
StringReader("01234567890"), 10)) {
+ mr.mark(3);
Review Comment:
@garydgregory Thanks, I see your point. Before changing this further, I
want to clarify the intended behavior.
Should BoundedReader.skip(long) respect only maxCharsFromTargetReader, or
should it also honor the active mark/readAheadLimit the same way read()
currently does?
I can keep this PR limited to the clear issue — capping skip() to
maxCharsFromTargetReader and updating charsRead with the actual skipped count —
unless you think skip() should also mirror the readAheadLimit check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
