NIFI-12041 Converted Groovy tests in nifi-scripting-bundle/nifi-scripting-processors to Java.

[dan-s1]

Summary

NIFI-12041

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 17

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 the contribution @dan-s1. It looks like the branch does not reflect the latest Groovy package versions, can you review and correct the dependency package names?

[exceptionfactory]

@dan-s1 The automated build on Windows has failed multiple times on the ScriptedReaderTest, attempting to delete a file. It looks like the Java process has not released a lock on files in the temporary directory, causing the problem. You could try disabling the RecordReaderFactory Controller Service after the final assertion, but that may not release the lock. If all else fails, removing that test method is acceptable in this scenario.

Error:  Tests run: 3, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 1.658 s <<< FAILURE! -- in org.apache.nifi.record.script.ScriptedReaderTest
Error:  org.apache.nifi.record.script.ScriptedReaderTest.testRecordReaderGroovyScriptChangeModuleDirectory(Path) -- Time elapsed: 0.564 s <<< ERROR!
java.io.IOException: Failed to delete temp directory C:\Users\RUNNER~1\AppData\Local\Temp\junit7656749993287821052. The following paths could not be deleted (see suppressed exceptions for details): <root>
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.stream.SortedOps$RefSortingSink.end(SortedOps.java:395)
	at java.base/java.util.stream.Sink$ChainedReference.end(Sink.java:258)
	at java.base/java.util.stream.Sink$ChainedReference.end(Sink.java:258)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:510)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	Suppressed: java.nio.file.FileSystemException: C:\Users\RUNNER~1\AppData\Local\Temp\junit7656749993287821052: The process cannot access the file because it is being used by another process
		at java.base/sun.nio.fs.WindowsException.translateToIOException(WindowsException.java:92)
		at java.base/sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:103)
		at java.base/sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:108)
		at java.base/sun.nio.fs.WindowsFileSystemProvider.implDelete(WindowsFileSystemProvider.java:275)
		at java.base/sun.nio.fs.AbstractFileSystemProvider.delete(AbstractFileSystemProvider.java:105)
		at java.base/java.nio.file.Files.delete(Files.java:1152)
		at java.base/java.nio.file.Files.walkFileTree(Files.java:2811)
		at java.base/java.nio.file.Files.walkFileTree(Files.java:2882)
		... 13 more
		Suppressed: java.nio.file.FileSystemException: C:\Users\RUNNER~1\AppData\Local\Temp\junit7656749993287821052: The process cannot access the file because it is being used by another process
			... 21 more

[dan-s1]

@exceptionfactory It seems there is an issue with Junit 5 TempDir on Windows. I found this issue. Though the title indicates the issue is with Java 11, reading further indicates it is also an issue with Java 17 which is what we are running with. It seems this was already an issue in #5791 (which is referenced in the above issue) and the solution there was to disable the test on Windows. Is that what should be done here also?

[exceptionfactory]

@exceptionfactory It seems there is an issue with Junit 5 TempDir on Windows. I found this issue. Though the title indicates the issue is with Java 11, reading further indicates it is also an issue with Java 17 which is what we are running with. It seems this was already an issue in #5791 and the solution there was to disable the test on Windows. Is that what should be done here also?

Disabling the test on Windows is an option, although the fact of failure more likely points to some subtle resource handling issue. Disabling on Windows seems sufficient for this one test method.

[dan-s1]

Another option mentioned in the ticket involves not using the TempDir annotation and instead using

private static Path tempDir;

    @BeforeAll
    public static void before() throws IOException {
        tempDir = Files.createTempDirectory(null);
    }

    @AfterAll
    public static void after() {
        tempDir.toFile().delete();
    }

[dan-s1]

@exceptionfactory I made the changes. That one test was actually using an instance variable and a method variable both annotated with TempDir. I kept the instance variable annotated as it is used by the other tests and I created a class variable for the temp file without the use of TempDir representing the jar file to load. Hopefully that will work on the Windows CI build.

[dan-s1]

@exceptionfactory That worked. Though I realize its not a big deal would it be better just to put the creation and deletion of the temp jar file inside the test instead of having it as a class variable? This would obviate the need for the before() and after() methods and save two imports for BeforeAll and AfterAll.

[exceptionfactory]

@exceptionfactory That worked. Though I realize its not a big deal would it be better just to put the creation and deletion of the temp jar file inside the test instead of having it as a class variable? This would obviate the need for the before() and after() methods and save two imports for BeforeAll and AfterAll.

Thanks for making the changes, glad to see the latest builds worked across the board.

I think the current approach works, otherwise it would require try-finally handling in the test method.

[exceptionfactory]

@dan-s1 I noted one more issue where the anonymous Processor implementations can be replaced with NoOpProcessor.class. Other than that, this should be good to go.

[dan-s1]

@exceptionfactory Thanks for the clarification on those formatting changes. All requested changes have been made.

[exceptionfactory]

Thanks for making the adjustments @dan-s1, the latest version looks good! +1 merging