classloader-leak-prevention icon indicating copy to clipboard operation
classloader-leak-prevention copied to clipboard

Rethrow exception from a failing test instead of analyzing leaks from the failing tests

Open vlsi opened this issue 2 years ago • 2 comments
trafficstars

Currently, JUnitClassloaderRunner performs leak analysis enen in the case the method throws an exception. It makes it harder to analyze the root cause for the failure: https://github.com/mjiderhamn/classloader-leak-prevention/blob/1229f8258d5aaae85f3f2c933f78a49e0272e9e7/classloader-leak-test-framework/src/main/java/se/jiderhamn/classloader/leak/JUnitClassloaderRunner.java#L121-L137

I suggest moving "analyze leaks" from finally to the try section, so it is executed only for the successful test executions.

Then it won't require re-wrapping the exceptions, so they would look better in the test output.

vlsi avatar Mar 13 '23 11:03 vlsi

I will need to take this under consideration, if there are cases when you may want to test that a @LeakPreventor works properly in combination with @Test(expected = ... for example.

mjiderhamn avatar Mar 13 '23 12:03 mjiderhamn

Test(expected= is an anti-pattern as the user can't assert the exception message, cause and so on.

JUnit 4 has assertThrows, so the users should no longer use expected=..: https://github.com/junit-team/junit4/pull/1504

In any case, classloader-leak-prevention changes the exception class anyway, so it makes expected=.. even less useful.

vlsi avatar Mar 13 '23 13:03 vlsi