NIFI-13996 Improved removal of tmp file creation during build

[dan-s1]

Summary

NIFI-13996

This PR aims to ensure the build does not leave around artifacts in a systems tmp directory. Two strategies were employed.

1. Where the unit test generated temporary files but did not clean them up, I added the use of Junit 5 TempDir annotation to allow Junit to handle the creation and deletion of temporary files .
2. Where the code called by the unit test generated temporary files, I added the use of the Junit 5 AfterAll annotation to allow for globbing files using file patterns with java.nio.file.Files.newDirectoryStream and then deleting them.

The one directory I could not delete was the poifiles directory created by all the Excel unit tests found in the nifi-poi-bundle. When trying to delete that directory after each of those unit tests, it seemed to clobber the poifiles directory for each of the subsequent tests causing the unit tests to fail due to files not existing which thereby failed the build.

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

• Apache NiFi Jira issue created

Pull Request Tracking

• Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
• Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

• Pull Request based on current revision of the main branch
• Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

• Build completed using mvn clean install -P contrib-check
  • JDK 21

Licensing

• New dependencies are compatible with the Apache License 2.0 according to the License Policy
• New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

• Documentation formatting appears as expected in rendered files

[exceptionfactory]

Thanks for working on improving the test behavior @dan-s1.

The failures on Windows indicate a process still using some files. This points to resources not closed, which appears to be a latent issue in some tests.

I plan to take a closer look soon, but Derby is particularly susceptible to this issue, so I recommend reviewing the behavior of other tests that use Derby.

[exceptionfactory]

It seems like this could be better as an AfterEach method. Ideally, the Handler class would have the temporary directory as a parameter, but given the usage of createTempDirectory(), this style of expecting the system temporary seems reasonable.

[dan-s1]

Thanks for working on improving the test behavior @dan-s1.

The failures on Windows indicate a process still using some files. This points to resources not closed, which appears to be a latent issue in some tests.

I plan to take a closer look soon, but Derby is particularly susceptible to this issue, so I recommend reviewing the behavior of other tests that use Derby.

@exceptionfactory Looking closer at the logs closer I noticed the following
Caused by: java.io.IOException: Failed to delete temp directory C:\Users\RUNNER~1\AppData\Local\Temp\junit-6685 264502979446340. The following paths could not be deleted (see suppressed exceptions for details)

I then looked at the suppressed exception which was
Suppressed: java.nio.file.DirectoryNotEmptyException: C:\Users\RUNNER~1\AppData\Local\Temp\junit-66852645029 79446340^M

Some Google searching revealed

When you encounter a java.nio.file.DirectoryNotEmptyException while using JUnit 5's @ TempDir annotation, it typically means that the temporary directory created for your test still contains files when JUnit tries to delete it.

I therefore added code to manually delete the directories created under the Junit 5 tempdir.

[dan-s1]

@exceptionfactory It seems that did not work as there is still that exception and the one you mentioned
java.nio.file.FileSystemException: C:\Users\RUNNER~1\AppData\Local\Temp\junit-4698073952417928585\TestPutSQL-cd3eb97e-97f8-4e02-acb5-1ce237ecc773\db\db.lck: The process cannot access the file because it is being used by another process

I understand now that is the main issue which is preventing the deletion which then prevents removing any top level directory.

I am not sure why this would make a difference but I see many of the database test classes placing the Derby database under target (in the Maven build).

[exceptionfactory]

@exceptionfactory It seems that did not work as there is still that exception and the one you mentioned java.nio.file.FileSystemException: C:\Users\RUNNER~1\AppData\Local\Temp\junit-4698073952417928585\TestPutSQL-cd3eb97e-97f8-4e02-acb5-1ce237ecc773\db\db.lck: The process cannot access the file because it is being used by another process

I understand now that is the main issue which is preventing the deletion which then prevents removing any top level directory.

Yes, this is where working with the lifecycle of the Derby database is essential to make things work as expected, versus just attempting to delete the directory contents.

[dan-s1]

What are the Unix based operating systems doing different than Windows for this as they do not seem to exhibit the same issues?

[exceptionfactory]

What are the Unix based operating systems doing different than Windows for this as they do not seem to exhibit the same issues?

This reflects OS differences in file locking.

• https://bugs.openjdk.org/browse/JDK-8208355
• https://www.howtogeek.com/137096/6-ways-the-linux-file-system-is-different-from-the-windows-file-system/#you-can-delete-or-modify-open-files

[dan-s1]

@exceptionfactory Instead of Using JUnit @ TempDir, I ended up just writing to the system temp dir and then using Junit 5 @ AfterAll to delete the created directory. That seemed to be ok even on Windows. At you earliest convenience can you please review this PR again? Thanks!

[exceptionfactory]

Thanks for the updates @dan-s1. Reviewing the changes locally, everything looked good except for the TestPutSQL directory. I made some adjustments to that class using the FileUtils.deleteDirectory method for more reliable directory removal, since it remained after the test completed. This worked locally on Linux, so I will review the GitHub workflow build for Windows to confirm it works as expected. It may be necessary to do some additional Derby shutdown calls given Windows behavior, but if the builds pass, I will proceed with merging.

I also noticed the poifiles directory as you mentioned in the description. It sounds like we may need to revisit this as a runtime setting, but that can be addressed separately.

[exceptionfactory]

All builds completed on the latest commit. Thanks again for this improvement @dan-s1, it is very helpful to avoid leaving temporary files around after a build. +1 merging