[sumittlearnbay]

Thank you for taking time to contribute this pull request!
You might have already read the contributor guide, but as a reminder, please make sure to:

• Rebase your changes on the latest main branch and squash your commits
• Add/Update unit tests as needed
• Run a build and make sure all tests pass prior to submission
• Sign-off commits according to the Developer Certificate of Origin

For more details, please check the contributor guide.
Thank you upfront!

[noojung]

I had the same problem on Windows 11.

After looking into the root cause, I found that the following line was written incorrectly in the first place:

when(mocked.delete()).thenReturn(false);

According to the Mockito javadoc, it is recommended to always use doReturn() for spies:
(Related link: https://javadoc.io/doc/org.mockito/mockito-core/latest/org.mockito/org/mockito/Mockito.html#spy(T))

   List list = new LinkedList();
   List spy = spy(list);

   //Impossible: real method is called so spy.get(0) throws IndexOutOfBoundsException (the list is yet empty)
   when(spy.get(0)).thenReturn("foo");

   //You have to use doReturn() for stubbing
   doReturn("foo").when(spy).get(0);

However, even if you update the code to use doReturn(), the test still fails (and now it fails on macOS as well):

//		when(mocked.delete()).thenReturn(false);
		doReturn(false).when(mocked).delete();

This is because, ItemWriter's close() method actually deletes the file using Files.delete().

Therefore, in my opinion, instead of acquiring a lock with RandomAccessFile, mocking Files.delete() would be a better approach.

Here is my sample code:

	@Test
	void testFailedFileDeletionThrowsException() {
		writer.setShouldDeleteIfEmpty(true);
		writer.open(executionContext);

		try (MockedStatic<Files> mocked = Mockito.mockStatic(Files.class, Mockito.CALLS_REAL_METHODS)) {
			mocked.when(() -> Files.delete(any())).thenThrow(new IOException("Failed to delete file"));

			ItemStreamException exception = assertThrows(ItemStreamException.class, writer::close,
					"Expected exception when file deletion fails");

			assertEquals("Failed to delete empty file on close", exception.getMessage(), "Wrong exception message");
			assertNotNull(exception.getCause(), "Exception should have a cause");
		}
	}

[sumittlearnbay]

Thank you for the explanation and suggestion.

I updated both AbstractFileItemWriterTests and StaxEventItemWriterTests to mock Files.delete() using MockedStatic instead of File.delete(), following your recommendation. I verified the updated tests locally and pushed the changes to this PR.

Please let me know if you see anything else that should be adjusted.

[noojung]

Oh, and it's no big deal, but the filename AbstractFileItemWriterTest.java is missing an 's'.

[fmbenhassine]

Hi @sumittlearnbay , thank you for the PR!

When I test the changes, I hit a build failure:

[INFO]
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR]   AbstractFileItemWriterTests.testFailedFileDeletionThrowsException:55 Expected org.springframework.batch.infrastructure.item.ItemStreamException to be thrown, but nothing was thrown.
[ERROR]   StaxEventItemWriterTests.testFailedFileDeletionThrowsException:847 Expected exception when file deletion fails ==> Expected org.springframework.batch.infrastructure.item.ItemStreamException to be thrown, but nothing was thrown.
[INFO]
[ERROR] Tests run: 1532, Failures: 2, Errors: 0, Skipped: 25

Can you please check?

[fmbenhassine]

Oh, and it's no big deal, but the filename AbstractFileItemWriterTest.java is missing an 's'.

Thank you for the follow up, @noojung ! I fixed that in 0a2e1c3.

[noojung]

Hi @sumittlearnbay, are you still working on this one?

If you don't have time to continue, I'd be happy to build on your changes and open a follow-up PR, crediting you as a co-author. Would that work for you?

[sumittlearnbay]

Hi @noojung yes, please continue. Sorry for the delayed response. Stuck in office work

[noojung]

@sumittlearnbay No worries at all, thanks for getting back to me.

I’ll take it from here and open a new PR based on your changes, with you credited as a co-author.

Good luck with everything at work!

[sumittlearnbay]

Hi @noojung try to containeriise if possible or i will take care so consistent behaviour across the platform also it seems it needs java version upgrade .

[noojung]

@sumittlearnbay Thanks for the suggestion.

Do you have any specific concerns about mocking Files.delete() with Mockito here? These tests focus on the writers’ error handling when deletion fails, so explicitly mocking that failure seems appropriate and avoids relying on platform-specific file behavior. This should let us reproduce the failure consistently across platforms without introducing a containerized test environment.

Also, could you clarify which part needs a Java version upgrade?

[noojung]

Regarding the Java version, the README.md states that building Spring Batch requires JDK 22+, and the CI workflow currently uses JDK 25. The java.version property in pom.xml (17) is passed to the compiler as --release, so it only defines the bytecode/API target, not the JDK required to run the build.

Were you referring to the JDK used to run Maven locally? If you ran into a build issue with a specific JDK, could you share the version and the error?

[sumittlearnbay]

@noojung Thanks for the clarification regarding the Java version.

My concern was primarily about achieving consistent test behavior across different operating systems and environments. The original file-deletion tests behaved differently on my Windows environment, so I explored whether containerization or a more deterministic test setup would help.

I agree that explicitly mocking Files.delete() with Mockito is appropriate for testing the writers’ error-handling behavior, provided the mock reliably intercepts the deletion call and verifies the expected exception handling. This would avoid depending on platform-specific filesystem permissions or deletion behavior.

Regarding Java, I initially encountered build issues with my local JDK configuration. I have since switched Maven to JDK 25, and the complete multi-module build succeeds. The targeted AbstractFileItemWriterTests and StaxEventItemWriterTests classes also pass.

Based on these results, I agree that containerization may not be necessary to address this specific issue. The priority should be ensuring that the tests deterministically simulate deletion failures and behave consistently across supported platforms.

Thanks for the clarification.

[noojung]

@sumittlearnbay Thanks for confirming!

I've already opened a new PR to address this issue and credited you as a co-author: #5614

The PR currently uses Mockito to simulate deletion failures consistently across platforms. I'm happy to adjust the approach based on the maintainer's feedback.

[fmbenhassine]

Thank you both!

Closing this in favor of #5614