Copilot commented on code in PR #785:
URL: https://github.com/apache/commons-io/pull/785#discussion_r2367837963
##########
src/main/java/org/apache/commons/io/IOUtils.java:
##########
@@ -1645,7 +1645,7 @@ public static long copyLarge(final Reader reader, final
Writer writer, final lon
* @param bufferSize The buffer size of the output stream; must be {@code
> 0}.
* @return a ByteArrayOutputStream containing the read bytes.
*/
- private static UnsynchronizedByteArrayOutputStream copyToOutputStream(
+ static UnsynchronizedByteArrayOutputStream copyToOutputStream(
Review Comment:
Changing the visibility of `copyToOutputStream` from private to
package-private creates an unintended API expansion. This method is only being
exposed for testing purposes, which violates encapsulation principles. Consider
using reflection in the test or creating a dedicated test utility method
instead of exposing internal implementation details.
```suggestion
private static UnsynchronizedByteArrayOutputStream copyToOutputStream(
```
##########
src/test/java/org/apache/commons/io/IOUtilsTest.java:
##########
@@ -1965,4 +1970,23 @@ void testWriteLittleString() throws IOException {
}
}
+ @Test
+ void testToByteArray_ThrowsIOExceptionOnHugeStream() {
+ try (MockedStatic<IOUtils> utils = Mockito.mockStatic(IOUtils.class,
Mockito.CALLS_REAL_METHODS)) {
+ // Prepare the mocks
+ final UnsynchronizedByteArrayOutputStream mockOutputStream =
+ mock(UnsynchronizedByteArrayOutputStream.class);
+ utils.when(() -> IOUtils.copyToOutputStream(
+ Mockito.any(InputStream.class), Mockito.anyLong(),
Mockito.anyInt()))
+ .thenReturn(mockOutputStream);
+
when(mockOutputStream.size()).thenReturn(IOUtils.SOFT_MAX_ARRAY_LENGTH + 1);
+
+ // Test and check
+ final InputStream mockInputStream = mock(InputStream.class);
+ final IOException exception = assertThrows(IOException.class, ()
-> IOUtils.toByteArray(mockInputStream));
+ assertTrue(
+ exception.getMessage().contains(String.format("%,d",
IOUtils.SOFT_MAX_ARRAY_LENGTH)),
+ "Exception message does not contain the maximum length");
+ }
Review Comment:
[nitpick] This test uses complex static mocking that creates tight coupling
between the test and internal implementation details. The test would be more
maintainable and robust if it used a real large InputStream (like a custom
InputStream that returns a predictable large amount of data) instead of mocking
internal methods.
```suggestion
// Create an InputStream that returns more bytes than
SOFT_MAX_ARRAY_LENGTH
final long length = IOUtils.SOFT_MAX_ARRAY_LENGTH + 1L;
InputStream hugeStream = new InputStream() {
private long remaining = length;
@Override
public int read() throws IOException {
if (remaining > 0) {
remaining--;
return 0;
} else {
return -1;
}
}
@Override
public int read(byte[] b, int off, int len) throws IOException {
if (remaining <= 0) {
return -1;
}
int toRead = (int) Math.min(len, Math.min(remaining, 8192));
Arrays.fill(b, off, off + toRead, (byte) 0);
remaining -= toRead;
return toRead;
}
};
final IOException exception = assertThrows(IOException.class, () ->
IOUtils.toByteArray(hugeStream));
assertTrue(
exception.getMessage().contains(String.format("%,d",
IOUtils.SOFT_MAX_ARRAY_LENGTH)),
"Exception message does not contain the maximum length");
```
--
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]
