On Mon, 28 Jul 2025 09:50:26 GMT, Volkan Yazici <vyaz...@openjdk.org> wrote:

>> test/jdk/java/net/httpclient/FileChannelPublisherTest.java line 300:
>> 
>>> 298:             int fileChannelOffset,
>>> 299:             int fileChannelLength,
>>> 300:             @TempDir(cleanup = CleanupMode.ON_SUCCESS) Path tempDir) 
>>> throws Exception {
>> 
>> It's not very common for us to use JUnit's `@TempDir` in our tests. I'm not 
>> completely sure where it creates those temporary directories. Instead, it 
>> might be better to use `Files.createTemporaryDirectory(Path.of("."), ...)` 
>> in these tests to allow for those temporary directories to be created under 
>> the jtreg's scratch directories.
>
>> I'm not completely sure where it creates those temporary directories.
> 
> JUnit `@TempDir` uses the JTreg scratch directory – [JTreg 
> 7.5.2](https://github.com/openjdk/jtreg/blob/master/CHANGELOG.md#752) bundles 
> [CODETOOLS-7903953](https://bugs.openjdk.org/browse/CODETOOLS-7903953) 
> addressing this issue. Shall I still replace these with `Files::cTD`? Or 
> `jdk.test.lib.Utils::createTempFile`?

Can you verify how it behaves with the `-retain` option of jtreg? I think the 
use of `cleanup` attribute here can conflict with the `-retain` option that can 
be used to launch `jtreg`. If it is possible to instruct JUnit to not do any 
cleanup at all, then I think using `@TempDir` here is OK (since as you note, it 
will end up using the jtreg scratch directory). If JUnit cannot be instructed 
not to do any cleanup, then using `@TempDir` here would add another layer of 
potential issue if jtreg was launched with `-retain:all` (for example).

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/26155#discussion_r2235669314

Reply via email to