refactor(tests): replaced redundant setups, simplified exception handling, and optimized code readability. (Stirling-Tools#4710)

# Description of Changes

This pull request primarily refactors and improves the test code across
several modules, focusing on modernization, simplification, and
consistency of assertions and test setup. The changes include formatting
updates and improvements to utility methods. These updates help make the
tests easier to maintain and read, and ensure they use current best
practices.

**Test code modernization and assertion improvements:**

* Replaced legacy assertion methods such as `assertTrue(x instanceof Y)`
with more specific `assertInstanceOf` assertions in multiple test files,
improving clarity and type safety.
* Updated exception assertion checks to use `assertInstanceOf` for error
types instead of `assertTrue`, ensuring more precise test validation.
* Refactored test setup in `ResourceMonitorTest` to use `final` for
`AtomicReference` fields, clarifying intent and thread safety.
* Changed some test method signatures to remove unnecessary `throws
Exception` clauses, simplifying the test code.

**Test code simplification and cleanup:**

* Removed unused mock fields and simplified array initializations in
`AutoJobPostMappingIntegrationTest`, streamlining test setup and
reducing clutter.
* Updated YAML string initialization in
`ApplicationPropertiesDynamicYamlPropertySourceTest` to use Java text
blocks for improved readability.
* Improved null handling in assertions for collection validity checks.
* Updated byte array encoding to use `StandardCharsets.UTF_8` for
reliability and clarity.

**PDF document factory test refactoring:**

* Refactored `CustomPDFDocumentFactoryTest` to move helper methods for
inflating PDFs and writing temp files to the top of the class, and
restructured parameterized tests for better organization and
maintainability.



<!--
Please provide a summary of the changes, including:

- What was changed
- Why the change was made
- Any challenges encountered

Closes #(issue_number)
-->

---

## Checklist

### General

- [ ] I have read the [Contribution
Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md)
- [ ] I have read the [Stirling-PDF Developer
Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/DeveloperGuide.md)
(if applicable)
- [ ] I have read the [How to add new languages to
Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md)
(if applicable)
- [ ] I have performed a self-review of my own code
- [ ] My changes generate no new warnings

### Documentation

- [ ] I have updated relevant docs on [Stirling-PDF's doc
repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/)
(if functionality has heavily changed)
- [ ] I have read the section [Add New Translation
Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags)
(for new translation tags only)

### UI Changes (if applicable)

- [ ] Screenshots or videos demonstrating the UI changes are attached
(e.g., as comments or direct attachments in the PR)

### Testing (if applicable)

- [ ] I have tested my changes locally. Refer to the [Testing
Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/DeveloperGuide.md#6-testing)
for more details.

---------

Signed-off-by: Balázs Szücs <[email protected]>

Author: balazs-szucs

app/common/src/test/java/stirling/software/common/annotations/AutoJobPostMappingIntegrationTest.java

@@ -31,8 +31,6 @@
 import stirling.software.common.model.api.PDFFile;
 import stirling.software.common.service.FileStorage;
 import stirling.software.common.service.JobExecutorService;
-import stirling.software.common.service.JobQueue;
-import stirling.software.common.service.ResourceMonitor;
 
 @ExtendWith(MockitoExtension.class)
 class AutoJobPostMappingIntegrationTest {
@@ -45,10 +43,6 @@ class AutoJobPostMappingIntegrationTest {
 
     @Mock private FileStorage fileStorage;
 
-    @Mock private ResourceMonitor resourceMonitor;
-
-    @Mock private JobQueue jobQueue;
-
     @BeforeEach
     void setUp() {
         autoJobAspect = new AutoJobAspect(jobExecutorService, request, fileStorage);
@@ -73,7 +67,7 @@ void shouldExecuteWithCustomParameters() throws Throwable {
         // Given
         PDFFile pdfFile = new PDFFile();
         pdfFile.setFileId("test-file-id");
-        Object[] args = new Object[] {pdfFile};
+        Object[] args = {pdfFile};
 
         when(joinPoint.getArgs()).thenReturn(args);
         when(request.getParameter("async")).thenReturn("true");
@@ -153,7 +147,7 @@ void shouldHandlePDFFileWithAsyncRequests() throws Throwable {
         // Given
         PDFFile pdfFile = new PDFFile();
         pdfFile.setFileInput(mock(MultipartFile.class));
-        Object[] args = new Object[] {pdfFile};
+        Object[] args = {pdfFile};
 
         when(joinPoint.getArgs()).thenReturn(args);
         when(request.getParameter("async")).thenReturn("true");

app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesDynamicYamlPropertySourceTest.java

@@ -20,11 +20,13 @@ class ApplicationPropertiesDynamicYamlPropertySourceTest {
     void loads_yaml_into_environment() throws Exception {
         // YAML-Config in Temp-Datei schreiben
         String yaml =
-                ""
-                        + "ui:\n"
-                        + "  appName: \"My App\"\n"
-                        + "system:\n"
-                        + "  enableAnalytics: true\n";
+                """
+                \
+                ui:
+                  appName: "My App"
+                system:
+                  enableAnalytics: true
+                """;
         Path tmp = Files.createTempFile("spdf-settings-", ".yml");
         Files.writeString(tmp, yaml);
 
@@ -44,7 +46,7 @@ void loads_yaml_into_environment() throws Exception {
     }
 
     @Test
-    void throws_when_settings_file_missing() throws Exception {
+    void throws_when_settings_file_missing() {
         String missing = "/path/does/not/exist/spdf.yml";
         try (MockedStatic<InstallationPathConfig> mocked =
                 Mockito.mockStatic(InstallationPathConfig.class)) {

app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesLogicTest.java

@@ -232,7 +232,7 @@ void collection_isValid_handles_null_and_empty() {
         Collection<String> nullColl = null;
         Collection<String> empty = List.of();
 
-        assertFalse(oauth2.isValid(nullColl, "scopes"));
+        assertFalse(oauth2.isValid((Collection<String>) null, "scopes"));
         assertFalse(oauth2.isValid(empty, "scopes"));
     }
 

app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesSaml2HttpTest.java

@@ -61,7 +61,7 @@ void spCert_else_branch_returns_FileSystemResource_for_filesystem_path() throws
         Resource r = s.getSpCert();
 
         assertNotNull(r);
-        assertTrue(r instanceof FileSystemResource, "Expected FileSystemResource for FS path");
+        assertInstanceOf(FileSystemResource.class, r, "Expected FileSystemResource for FS path");
         assertTrue(r.exists(), "Temp file should exist");
     }
 
@@ -75,7 +75,7 @@ void idpCert_else_branch_returns_FileSystemResource_even_if_missing() {
         Resource r = s.getIdpCert();
 
         assertNotNull(r);
-        assertTrue(r instanceof FileSystemResource, "Expected FileSystemResource for FS path");
+        assertInstanceOf(FileSystemResource.class, r, "Expected FileSystemResource for FS path");
         assertFalse(r.exists(), "Resource should not exist for missing file");
     }
 }

app/common/src/test/java/stirling/software/common/model/InputStreamTemplateResourceTest.java

@@ -7,6 +7,7 @@
 import java.io.Reader;
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 
 import org.junit.jupiter.api.Test;
@@ -47,7 +48,7 @@ void noSetterMethodsPresent() {
     @Test
     void readerReturnsCorrectContent() throws Exception {
         String content = "Hello, world!";
-        InputStream is = new ByteArrayInputStream(content.getBytes("UTF-8"));
+        InputStream is = new ByteArrayInputStream(content.getBytes(StandardCharsets.UTF_8));
         InputStreamTemplateResource resource = new InputStreamTemplateResource(is, "UTF-8");
 
         try (Reader reader = resource.reader()) {

app/common/src/test/java/stirling/software/common/service/CustomPDFDocumentFactoryTest.java

@@ -41,31 +41,53 @@ void setup() throws IOException {
         }
     }
 
+    private static byte[] inflatePdf(byte[] input, int sizeInMB) throws IOException {
+        try (PDDocument doc = Loader.loadPDF(input)) {
+            byte[] largeData = new byte[sizeInMB * 1024 * 1024];
+            Arrays.fill(largeData, (byte) 'A');
+
+            PDStream stream = new PDStream(doc, new ByteArrayInputStream(largeData));
+            stream.getCOSObject().setItem(COSName.TYPE, COSName.XOBJECT);
+            stream.getCOSObject().setItem(COSName.SUBTYPE, COSName.IMAGE);
+
+            doc.getDocumentCatalog()
+                    .getCOSObject()
+                    .setItem(COSName.getPDFName("DummyBigStream"), stream.getCOSObject());
+
+            ByteArrayOutputStream out = new ByteArrayOutputStream();
+            doc.save(out);
+            return out.toByteArray();
+        }
+    }
+
+    private static File writeTempFile(byte[] content) throws IOException {
+        File file = Files.createTempFile("pdf-test-", ".pdf").toFile();
+        Files.write(file.toPath(), content);
+        return file;
+    }
+
     @ParameterizedTest
     @CsvSource({"5,MEMORY_ONLY", "20,MIXED", "60,TEMP_FILE"})
     void testStrategy_FileInput(int sizeMB, StrategyType expected) throws IOException {
         File file = writeTempFile(inflatePdf(basePdfBytes, sizeMB));
-        try (PDDocument doc = factory.load(file)) {
-            Assertions.assertEquals(expected, factory.lastStrategyUsed);
-        }
+        factory.load(file);
+        Assertions.assertEquals(expected, factory.lastStrategyUsed);
     }
 
     @ParameterizedTest
     @CsvSource({"5,MEMORY_ONLY", "20,MIXED", "60,TEMP_FILE"})
     void testStrategy_ByteArray(int sizeMB, StrategyType expected) throws IOException {
         byte[] inflated = inflatePdf(basePdfBytes, sizeMB);
-        try (PDDocument doc = factory.load(inflated)) {
-            Assertions.assertEquals(expected, factory.lastStrategyUsed);
-        }
+        factory.load(inflated);
+        Assertions.assertEquals(expected, factory.lastStrategyUsed);
     }
 
     @ParameterizedTest
     @CsvSource({"5,MEMORY_ONLY", "20,MIXED", "60,TEMP_FILE"})
     void testStrategy_InputStream(int sizeMB, StrategyType expected) throws IOException {
         byte[] inflated = inflatePdf(basePdfBytes, sizeMB);
-        try (PDDocument doc = factory.load(new ByteArrayInputStream(inflated))) {
-            Assertions.assertEquals(expected, factory.lastStrategyUsed);
-        }
+        factory.load(new ByteArrayInputStream(inflated));
+        Assertions.assertEquals(expected, factory.lastStrategyUsed);
     }
 
     @ParameterizedTest
@@ -74,41 +96,8 @@ void testStrategy_MultipartFile(int sizeMB, StrategyType expected) throws IOExce
         byte[] inflated = inflatePdf(basePdfBytes, sizeMB);
         MockMultipartFile multipart =
                 new MockMultipartFile("file", "doc.pdf", MediaType.APPLICATION_PDF_VALUE, inflated);
-        try (PDDocument doc = factory.load(multipart)) {
-            Assertions.assertEquals(expected, factory.lastStrategyUsed);
-        }
-    }
-
-    @ParameterizedTest
-    @CsvSource({"5,MEMORY_ONLY", "20,MIXED", "60,TEMP_FILE"})
-    void testStrategy_PDFFile(int sizeMB, StrategyType expected) throws IOException {
-        byte[] inflated = inflatePdf(basePdfBytes, sizeMB);
-        MockMultipartFile multipart =
-                new MockMultipartFile("file", "doc.pdf", MediaType.APPLICATION_PDF_VALUE, inflated);
-        PDFFile pdfFile = new PDFFile();
-        pdfFile.setFileInput(multipart);
-        try (PDDocument doc = factory.load(pdfFile)) {
-            Assertions.assertEquals(expected, factory.lastStrategyUsed);
-        }
-    }
-
-    private byte[] inflatePdf(byte[] input, int sizeInMB) throws IOException {
-        try (PDDocument doc = Loader.loadPDF(input)) {
-            byte[] largeData = new byte[sizeInMB * 1024 * 1024];
-            Arrays.fill(largeData, (byte) 'A');
-
-            PDStream stream = new PDStream(doc, new ByteArrayInputStream(largeData));
-            stream.getCOSObject().setItem(COSName.TYPE, COSName.XOBJECT);
-            stream.getCOSObject().setItem(COSName.SUBTYPE, COSName.IMAGE);
-
-            doc.getDocumentCatalog()
-                    .getCOSObject()
-                    .setItem(COSName.getPDFName("DummyBigStream"), stream.getCOSObject());
-
-            ByteArrayOutputStream out = new ByteArrayOutputStream();
-            doc.save(out);
-            return out.toByteArray();
-        }
+        factory.load(multipart);
+        Assertions.assertEquals(expected, factory.lastStrategyUsed);
     }
 
     @Test
@@ -210,10 +199,16 @@ void testCreateNewBytesBasedOnOldDocument() throws IOException {
         assertTrue(newBytes.length > 0);
     }
 
-    private File writeTempFile(byte[] content) throws IOException {
-        File file = Files.createTempFile("pdf-test-", ".pdf").toFile();
-        Files.write(file.toPath(), content);
-        return file;
+    @ParameterizedTest
+    @CsvSource({"5,MEMORY_ONLY", "20,MIXED", "60,TEMP_FILE"})
+    void testStrategy_PDFFile(int sizeMB, StrategyType expected) throws IOException {
+        byte[] inflated = inflatePdf(basePdfBytes, sizeMB);
+        MockMultipartFile multipart =
+                new MockMultipartFile("file", "doc.pdf", MediaType.APPLICATION_PDF_VALUE, inflated);
+        PDFFile pdfFile = new PDFFile();
+        pdfFile.setFileInput(multipart);
+        factory.load(pdfFile);
+        Assertions.assertEquals(expected, factory.lastStrategyUsed);
     }
 
     @BeforeEach

app/common/src/test/java/stirling/software/common/service/JobExecutorServiceTest.java

@@ -1,8 +1,6 @@
 package stirling.software.common.service;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.ArgumentMatchers.anyString;
@@ -63,7 +61,7 @@ void setUp() {
     }
 
     @Test
-    void shouldRunSyncJobSuccessfully() throws Exception {
+    void shouldRunSyncJobSuccessfully() {
         // Given
         Supplier<Object> work = () -> "test-result";
 
@@ -79,7 +77,7 @@ void shouldRunSyncJobSuccessfully() throws Exception {
     }
 
     @Test
-    void shouldRunAsyncJobSuccessfully() throws Exception {
+    void shouldRunAsyncJobSuccessfully() {
         // Given
         Supplier<Object> work = () -> "test-result";
 
@@ -88,7 +86,7 @@ void shouldRunAsyncJobSuccessfully() throws Exception {
 
         // Then
         assertEquals(HttpStatus.OK, response.getStatusCode());
-        assertTrue(response.getBody() instanceof JobResponse);
+        assertInstanceOf(JobResponse.class, response.getBody());
         JobResponse<?> jobResponse = (JobResponse<?>) response.getBody();
         assertTrue(jobResponse.isAsync());
         assertNotNull(jobResponse.getJobId());
@@ -113,6 +111,7 @@ void shouldHandleSyncJobError() {
 
         @SuppressWarnings("unchecked")
         Map<String, String> errorMap = (Map<String, String>) response.getBody();
+        assertNotNull(errorMap);
         assertEquals("Job failed: Test error", errorMap.get("error"));
     }
 
@@ -133,7 +132,7 @@ void shouldQueueJobWhenResourcesLimited() {
 
         // Then
         assertEquals(HttpStatus.OK, response.getStatusCode());
-        assertTrue(response.getBody() instanceof JobResponse);
+        assertInstanceOf(JobResponse.class, response.getBody());
 
         // Verify job was queued
         verify(jobQueue).queueJob(anyString(), eq(80), any(), eq(5000L));
@@ -186,7 +185,7 @@ void shouldHandleTimeout() throws Exception {
         try {
             executeMethod.invoke(jobExecutorService, work, 1L); // Very short timeout
         } catch (Exception e) {
-            assertTrue(e.getCause() instanceof TimeoutException);
+            assertInstanceOf(TimeoutException.class, e.getCause());
         }
     }
 }

app/common/src/test/java/stirling/software/common/service/ResourceMonitorTest.java

@@ -33,11 +33,11 @@ class ResourceMonitorTest {
     @Mock private MemoryMXBean memoryMXBean;
 
     @Spy
-    private AtomicReference<ResourceStatus> currentStatus =
+    private final AtomicReference<ResourceStatus> currentStatus =
             new AtomicReference<>(ResourceStatus.OK);
 
     @Spy
-    private AtomicReference<ResourceMetrics> latestMetrics =
+    private final AtomicReference<ResourceMetrics> latestMetrics =
             new AtomicReference<>(new ResourceMetrics());
 
     @BeforeEach

app/common/src/test/java/stirling/software/common/service/TaskManagerTest.java

@@ -215,7 +215,7 @@ void testGetJobStats() throws Exception {
     }
 
     @Test
-    void testCleanupOldJobs() throws Exception {
+    void testCleanupOldJobs() {
         // Arrange
         // 1. Create a recent completed job
         String recentJobId = "recent-job";
@@ -253,6 +253,7 @@ void testCleanupOldJobs() throws Exception {
         taskManager.createTask(activeJobId);
 
         // Verify all jobs are in the map
+        assertNotNull(jobResultsMap);
         assertTrue(jobResultsMap.containsKey(recentJobId));
         assertTrue(jobResultsMap.containsKey(oldJobId));
         assertTrue(jobResultsMap.containsKey(activeJobId));
@@ -268,7 +269,7 @@ void testCleanupOldJobs() throws Exception {
     }
 
     @Test
-    void testShutdown() throws Exception {
+    void testShutdown() {
         // This mainly tests that the shutdown method doesn't throw exceptions
         taskManager.shutdown();
 

app/common/src/test/java/stirling/software/common/service/TempFileCleanupServiceTest.java

@@ -219,9 +219,9 @@ else if (fileName.equals("empty.tmp")) {
                             });
 
             // Act - set containerMode to false for this test
-            invokeCleanupDirectoryStreaming(systemTempDir, false, 0, 3600000);
-            invokeCleanupDirectoryStreaming(customTempDir, false, 0, 3600000);
-            invokeCleanupDirectoryStreaming(libreOfficeTempDir, false, 0, 3600000);
+            invokeCleanupDirectoryStreaming(systemTempDir, false, 3600000);
+            invokeCleanupDirectoryStreaming(customTempDir, false, 3600000);
+            invokeCleanupDirectoryStreaming(libreOfficeTempDir, false, 3600000);
 
             // Assert - Only old temp files and empty files should be deleted
             assertTrue(deletedFiles.contains(oldTempFile), "Old temp file should be deleted");
@@ -306,7 +306,7 @@ public void testContainerModeCleanup() throws IOException {
                             });
 
             // Act - set containerMode to true and maxAgeMillis to 0 for container startup cleanup
-            invokeCleanupDirectoryStreaming(systemTempDir, true, 0, 0);
+            invokeCleanupDirectoryStreaming(systemTempDir, true, 0);
 
             // Assert - In container mode, both our temp files and system temp files should be
             // deleted
@@ -379,7 +379,7 @@ public void testEmptyFileHandling() throws IOException {
                             });
 
             // Act
-            invokeCleanupDirectoryStreaming(systemTempDir, false, 0, 3600000);
+            invokeCleanupDirectoryStreaming(systemTempDir, false, 3600000);
 
             // Assert
             assertTrue(
@@ -462,7 +462,7 @@ public void testRecursiveDirectoryCleaning() throws IOException {
                             });
 
             // Act
-            invokeCleanupDirectoryStreaming(systemTempDir, false, 0, 3600000);
+            invokeCleanupDirectoryStreaming(systemTempDir, false, 3600000);
 
             // Debug - print what was deleted
             System.out.println("Deleted files: " + deletedFiles);
@@ -479,8 +479,7 @@ public void testRecursiveDirectoryCleaning() throws IOException {
 
     /** Helper method to invoke the private cleanupDirectoryStreaming method using reflection */
     private void invokeCleanupDirectoryStreaming(
-            Path directory, boolean containerMode, int depth, long maxAgeMillis)
-            throws IOException {
+            Path directory, boolean containerMode, long maxAgeMillis) {
         try {
             // Create a consumer that tracks deleted files
             AtomicInteger deleteCount = new AtomicInteger(0);
@@ -503,7 +502,7 @@ private void invokeCleanupDirectoryStreaming(
                     cleanupService,
                     directory,
                     containerMode,
-                    depth,
+                    0,
                     maxAgeMillis,
                     false,
                     deleteCallback);