garydgregory commented on code in PR #789:
URL: https://github.com/apache/commons-io/pull/789#discussion_r2387691719


##########
src/test/java/org/apache/commons/io/build/AbstractOriginTest.java:
##########
@@ -133,50 +117,57 @@ private boolean isValid(final RandomAccessFile raf) 
throws IOException {
 
     protected abstract B newOriginRw() throws IOException;
 
-    protected void resetOriginRw() throws IOException {
-        // No-op
-    }
-
-    protected void setOriginRo(final AbstractOrigin<T, B> origin) {
-        this.originRo = origin;
-    }
-
-    protected void setOriginRw(final AbstractOrigin<T, B> origin) {
-        this.originRw = origin;
-    }
-
     @Test
     void testGetByteArray() throws IOException {
-        assertArrayEquals(getFixtureByteArray(), getOriginRo().getByteArray());
+        final AbstractOrigin<T, B> originRo = newOriginRo();
+        assertArrayEquals(getFixtureByteArray(), originRo.getByteArray());
+        assertOpen(originRo);
+        close(originRo);
     }
 
     @Test
     void testGetByteArrayAt_0_0() throws IOException {
-        assertArrayEquals(new byte[] {}, getOriginRo().getByteArray(0, 0));
+        final AbstractOrigin<T, B> originRo = newOriginRo();
+        assertArrayEquals(new byte[]{}, originRo.getByteArray(0, 0));
+        assertOpen(originRo);
+        close(originRo);

Review Comment:
   Hello @ppkarwasz 
   
   The benefit of the previous approach is that the `@AfterEach cleanup()` 
method was _always_ called, no matter what, whether a test passed or failed. In 
this PR, the release of an origin is no longer guaranteed since the 
`@AfterEach` is gone and the new calls to `close(...)` are _not_ in a `finally` 
block. The origin is never released when an assertion fails. 
   
   Am I missing something?
   
   If all the tests now contain a `try-finally` block, it says to me that an 
`@AfterEach` method is warranted. to avoid copy-pasta all over and increasing 
the clutter.
   
   WDYT? Is there a way to have better tests, but with release guarantees that 
don't clutter up the tests?
   
   



-- 
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: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to