On Sat, 23 Nov 2024 15:47:53 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:

> Please review this PR which  adds a utility API in the test libraries to 
> assert whether a file is currently open.
> 
> Several OpenJDK  tests currently rely on approximations to check this, 
> including deletion (fails only on Windows), or checking `/proc/<pid>/fd` 
> (Works only on Linux). These approximations are blunt instruments, it would 
> be better to have an exact cross platform solution for this need to check if 
> a file is open or closed.
> 
> Initial support includes:
> 
> * `FileInputStream`, `FileOutputStream` and `RandomAccessFile` from `java.io`
> * `FileChannelImpl` from `java.nio`, meaning we can check synchronous NIO 
> APIs such as `Files.newInputStream`, `Files.newOutputStream`, 
> `Files.newByteChannel` and `FileChannel.open`
> 
> This allows testing of many (most?) JDK APIs such as `ZipFile`, `JarFile`, 
> `JarURLConnection`, `FileURLConnection` etc.
> 
> Tests use the following configuration to enable  instrumentation of file 
> access APIs:
> 
> 
> * @library /test/lib
> * @run driver jdk.test.lib.util.OpenFilesAgent
> * @run junit/othervm -javaagent:OpenFilesAgent.jar OpenFilesTest
> 
> 
> Also provided is an assertion API to verify whether files are open or not:
> 
> 
> Path file = Files.createFile(Path.of("file.txt"));
> assertClosed(file);
> try (var is = Files.newInputStream(file)) {
>     assertOpen(file);
> }
> assertClosed(file);
> ``` 
> 
> A failed `assertClosed` shows the stack trace for whatever action opened the 
> file:
> 
> 
> Caused by: java.lang.Throwable: Opening stack trace of 
> jdk/build/macosx-x86_64-server-release/JTwork/scratch/file.txt
>       at jdk.test.lib.util.OpenFiles.openFile(OpenFiles.java:59)
>       at java.base/java.nio.file.Files.newByteChannel(Files.java:357)
>       at java.base/java.nio.file.Files.newByteChannel(Files.java:399)
>       at 
> java.base/java.nio.file.spi.FileSystemProvider.newInputStream(FileSystemProvider.java:371)
>       at java.base/java.nio.file.Files.newInputStream(Files.java:154)
>       at OpenFilesTest.sample(OpenFilesTest.java:81)
> 
> 
> See the library implementation test `OpenFilesTest` for samples.
> 
> Verification:
> With `OpenFilesTest` temporarily in tier1, GHA completed successfully across 
> platforms.

To the motivation, please list the tests that would be modified to use this 
test feature.
To the feature itself, it seems pretty complicated and invasive, please expand 
on the necessity.

Note that the initial comment in any PR is copied in every email sent on the PR 
and makes it harder to get to the point in subsequent emails. Additional 
details can/should be included in separate comments or in the Jira issue.

To have a clean PR, it would have been a benefit to reviewers to have squashed 
the work in progress commits before opening the PR.

Thanks for developing this approach to a difficult test problem.

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

PR Comment: https://git.openjdk.org/jdk/pull/22343#issuecomment-2498560121

Reply via email to