Interesting. On the plus side tests that declared the Manager inside the try won't have to worry about calling release().
On the minus side, in actual (non-test) usage, Managers are long-lived objects that are not supposed to be created and destroyed within the scope of a single method call. So declaring them as Closeable might be slightly confusing. I don't consider that potential confusion a major drawback though, as long as we document that Closeable is only implemented to facilitate test cleanup. No strong opinion either way. Remko Sent from my iPhone > On 2016/06/29, at 7:52, Gary Gregory <[email protected]> wrote: > > Would anyone else care to opine? > > G > >> On Tue, Jun 28, 2016 at 1:20 AM, Mikael Ståldal <[email protected]> >> wrote: >> Sounds good. >> >>> On Fri, Jun 24, 2016 at 1:23 AM, Gary Gregory <[email protected]> >>> wrote: >>> I seems that AbstractManager should implement AutoCloseable where close() >>> does the same thing as release() and release() can be deprecated. >>> >>> org.apache.logging.log4j.core.appender.AbstractManager.release() >>> >>> This will let us rewrite things like the recently fixed >>> "OnStartupTriggeringPolicyTest fails on Windows saying the file is used by >>> another process" https://issues.apache.org/jira/browse/LOG4J2-1445: >>> >>> Is: >>> >>> final RollingFileManager manager = >>> RollingFileManager.getFileManager(TARGET_FILE, TARGET_PATTERN, true, false, >>> policy, strategy, null, layout, 8192, true); >>> try { >>> manager.initialize(); >>> assertTrue(Files.exists(target)); >>> assertTrue(Files.size(target) == 0); >>> assertTrue(Files.exists(rolled)); >>> assertTrue(Files.size(rolled) == size); >>> } finally { >>> manager.release(); >>> } >>> >>> Could be: >>> >>> try (final RollingFileManager manager = >>> RollingFileManager.getFileManager(TARGET_FILE, TARGET_PATTERN, true, false, >>> policy, strategy, null, layout, 8192, true)) { >>> manager.initialize(); >>> assertTrue(Files.exists(target)); >>> assertTrue(Files.size(target) == 0); >>> assertTrue(Files.exists(rolled)); >>> assertTrue(Files.size(rolled) == size); >>> } >>> >>> Thoughts? >>> >>> Gary >>> >>> >>> -- >>> E-Mail: [email protected] | [email protected] >>> Java Persistence with Hibernate, Second Edition >>> JUnit in Action, Second Edition >>> Spring Batch in Action >>> Blog: http://garygregory.wordpress.com >>> Home: http://garygregory.com/ >>> Tweet! http://twitter.com/GaryGregory >> >> >> >> -- >> >> >> Mikael Ståldal >> Senior software developer >> >> Magine TV >> [email protected] >> Grev Turegatan 3 | 114 46 Stockholm, Sweden | www.magine.com >> >> Privileged and/or Confidential Information may be contained in this message. >> If you are not the addressee indicated in this message >> (or responsible for delivery of the message to such a person), you may not >> copy or deliver this message to anyone. In such case, >> you should destroy this message and kindly notify the sender by reply email. >> > > > > -- > E-Mail: [email protected] | [email protected] > Java Persistence with Hibernate, Second Edition > JUnit in Action, Second Edition > Spring Batch in Action > Blog: http://garygregory.wordpress.com > Home: http://garygregory.com/ > Tweet! http://twitter.com/GaryGregory
