mirror of
https://github.com/Frooodle/Stirling-PDF.git
synced 2026-02-17 13:52:14 +01:00
refactor(tests): replaced redundant setups, simplified exception handling, and optimized code readability. (#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 <bszucs1209@gmail.com>
This commit is contained in:
@@ -14,7 +14,6 @@ import org.springframework.security.oauth2.client.authentication.OAuth2Authentic
|
||||
import jakarta.servlet.http.HttpServletRequest;
|
||||
import jakarta.servlet.http.HttpServletResponse;
|
||||
|
||||
import stirling.software.common.configuration.AppConfig;
|
||||
import stirling.software.common.model.ApplicationProperties;
|
||||
import stirling.software.proprietary.security.service.JwtServiceInterface;
|
||||
|
||||
@@ -23,8 +22,6 @@ class CustomLogoutSuccessHandlerTest {
|
||||
|
||||
@Mock private ApplicationProperties.Security securityProperties;
|
||||
|
||||
@Mock private AppConfig appConfig;
|
||||
|
||||
@Mock private JwtServiceInterface jwtService;
|
||||
|
||||
@InjectMocks private CustomLogoutSuccessHandler customLogoutSuccessHandler;
|
||||
|
||||
@@ -55,7 +55,7 @@ class LicenseKeyCheckerTest {
|
||||
|
||||
ApplicationProperties props = new ApplicationProperties();
|
||||
props.getPremium().setEnabled(true);
|
||||
props.getPremium().setKey("file:" + file.toString());
|
||||
props.getPremium().setKey("file:" + file);
|
||||
when(verifier.verifyLicense("filekey")).thenReturn(License.ENTERPRISE);
|
||||
|
||||
LicenseKeyChecker checker = new LicenseKeyChecker(verifier, props);
|
||||
@@ -69,7 +69,7 @@ class LicenseKeyCheckerTest {
|
||||
Path file = temp.resolve("missing.txt");
|
||||
ApplicationProperties props = new ApplicationProperties();
|
||||
props.getPremium().setEnabled(true);
|
||||
props.getPremium().setKey("file:" + file.toString());
|
||||
props.getPremium().setKey("file:" + file);
|
||||
|
||||
LicenseKeyChecker checker = new LicenseKeyChecker(verifier, props);
|
||||
|
||||
|
||||
@@ -35,11 +35,9 @@ import jakarta.servlet.ServletException;
|
||||
import jakarta.servlet.http.HttpServletRequest;
|
||||
import jakarta.servlet.http.HttpServletResponse;
|
||||
|
||||
import stirling.software.common.model.ApplicationProperties;
|
||||
import stirling.software.proprietary.security.model.exception.AuthenticationFailureException;
|
||||
import stirling.software.proprietary.security.service.CustomUserDetailsService;
|
||||
import stirling.software.proprietary.security.service.JwtServiceInterface;
|
||||
import stirling.software.proprietary.security.service.UserService;
|
||||
|
||||
@Disabled
|
||||
@ExtendWith(MockitoExtension.class)
|
||||
@@ -49,10 +47,6 @@ class JwtAuthenticationFilterTest {
|
||||
|
||||
@Mock private CustomUserDetailsService userDetailsService;
|
||||
|
||||
@Mock private UserService userService;
|
||||
|
||||
@Mock private ApplicationProperties.Security securityProperties;
|
||||
|
||||
@Mock private HttpServletRequest request;
|
||||
|
||||
@Mock private HttpServletResponse response;
|
||||
|
||||
@@ -62,7 +62,6 @@ class JwtSaml2AuthenticationRequestRepositoryTest {
|
||||
String id = "testId";
|
||||
String relayState = "testRelayState";
|
||||
String authnRequestUri = "example.com/authnRequest";
|
||||
Map<String, Object> claims = Map.of();
|
||||
String samlRequest = "testSamlRequest";
|
||||
String relyingPartyRegistrationId = "stirling-pdf";
|
||||
|
||||
|
||||
@@ -63,7 +63,7 @@ public class EmailServiceTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
void testSendEmailWithAttachmentThrowsExceptionForMissingFilename() throws MessagingException {
|
||||
void testSendEmailWithAttachmentThrowsExceptionForMissingFilename() {
|
||||
Email email = new Email();
|
||||
email.setTo("test@example.com");
|
||||
email.setSubject("Test Email");
|
||||
@@ -82,8 +82,7 @@ public class EmailServiceTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
void testSendEmailWithAttachmentThrowsExceptionForMissingFilenameNull()
|
||||
throws MessagingException {
|
||||
void testSendEmailWithAttachmentThrowsExceptionForMissingFilenameNull() {
|
||||
Email email = new Email();
|
||||
email.setTo("test@example.com");
|
||||
email.setSubject("Test Email");
|
||||
@@ -102,7 +101,7 @@ public class EmailServiceTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
void testSendEmailWithAttachmentThrowsExceptionForMissingFile() throws MessagingException {
|
||||
void testSendEmailWithAttachmentThrowsExceptionForMissingFile() {
|
||||
Email email = new Email();
|
||||
email.setTo("test@example.com");
|
||||
email.setSubject("Test Email");
|
||||
@@ -120,7 +119,7 @@ public class EmailServiceTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
void testSendEmailWithAttachmentThrowsExceptionForMissingFileNull() throws MessagingException {
|
||||
void testSendEmailWithAttachmentThrowsExceptionForMissingFileNull() {
|
||||
Email email = new Email();
|
||||
email.setTo("test@example.com");
|
||||
email.setSubject("Test Email");
|
||||
@@ -136,8 +135,7 @@ public class EmailServiceTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
void testSendEmailWithAttachmentThrowsExceptionForInvalidAddressNull()
|
||||
throws MessagingException {
|
||||
void testSendEmailWithAttachmentThrowsExceptionForInvalidAddressNull() {
|
||||
Email email = new Email();
|
||||
email.setTo(null); // Invalid address
|
||||
email.setSubject("Test Email");
|
||||
@@ -153,8 +151,7 @@ public class EmailServiceTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
void testSendEmailWithAttachmentThrowsExceptionForInvalidAddressEmpty()
|
||||
throws MessagingException {
|
||||
void testSendEmailWithAttachmentThrowsExceptionForInvalidAddressEmpty() {
|
||||
Email email = new Email();
|
||||
email.setTo(""); // Invalid address
|
||||
email.setSubject("Test Email");
|
||||
|
||||
@@ -138,9 +138,7 @@ class JwtServiceTest {
|
||||
|
||||
assertThrows(
|
||||
AuthenticationFailureException.class,
|
||||
() -> {
|
||||
jwtService.validateToken("invalid-token");
|
||||
});
|
||||
() -> jwtService.validateToken("invalid-token"));
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -152,9 +150,7 @@ class JwtServiceTest {
|
||||
AuthenticationFailureException exception =
|
||||
assertThrows(
|
||||
AuthenticationFailureException.class,
|
||||
() -> {
|
||||
jwtService.validateToken("malformed.token");
|
||||
});
|
||||
() -> jwtService.validateToken("malformed.token"));
|
||||
|
||||
assertTrue(exception.getMessage().contains("Invalid"));
|
||||
}
|
||||
@@ -167,10 +163,7 @@ class JwtServiceTest {
|
||||
|
||||
AuthenticationFailureException exception =
|
||||
assertThrows(
|
||||
AuthenticationFailureException.class,
|
||||
() -> {
|
||||
jwtService.validateToken("");
|
||||
});
|
||||
AuthenticationFailureException.class, () -> jwtService.validateToken(""));
|
||||
|
||||
assertTrue(
|
||||
exception.getMessage().contains("Claims are empty")
|
||||
@@ -296,7 +289,7 @@ class JwtServiceTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
void testGenerateTokenWithKeyId() throws Exception {
|
||||
void testGenerateTokenWithKeyId() {
|
||||
String username = "testuser";
|
||||
Map<String, Object> claims = new HashMap<>();
|
||||
|
||||
|
||||
@@ -7,7 +7,6 @@ import static org.mockito.Mockito.lenient;
|
||||
import static org.mockito.Mockito.mockStatic;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.security.KeyPair;
|
||||
@@ -84,7 +83,7 @@ class KeyPersistenceServiceInterfaceTest {
|
||||
String privateKeyBase64 =
|
||||
Base64.getEncoder().encodeToString(testKeyPair.getPrivate().getEncoded());
|
||||
|
||||
JwtVerificationKey existingKey = new JwtVerificationKey(keyId, publicKeyBase64);
|
||||
new JwtVerificationKey(keyId, publicKeyBase64);
|
||||
|
||||
Path keyFile = tempDir.resolve(keyId + ".key");
|
||||
Files.writeString(keyFile, privateKeyBase64);
|
||||
@@ -129,6 +128,7 @@ class KeyPersistenceServiceInterfaceTest {
|
||||
.getDeclaredField("verifyingKeyCache")
|
||||
.setAccessible(true);
|
||||
var cache = cacheManager.getCache("verifyingKeys");
|
||||
assertNotNull(cache);
|
||||
cache.put(keyId, signingKey);
|
||||
|
||||
Optional<KeyPair> result = keyPersistenceService.getKeyPair(keyId);
|
||||
@@ -174,7 +174,7 @@ class KeyPersistenceServiceInterfaceTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
void testInitializeKeystoreCreatesDirectory() throws IOException {
|
||||
void testInitializeKeystoreCreatesDirectory() {
|
||||
try (MockedStatic<InstallationPathConfig> mockedStatic =
|
||||
mockStatic(InstallationPathConfig.class)) {
|
||||
mockedStatic
|
||||
@@ -189,12 +189,12 @@ class KeyPersistenceServiceInterfaceTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
void testLoadExistingKeypairWithMissingPrivateKeyFile() throws Exception {
|
||||
void testLoadExistingKeypairWithMissingPrivateKeyFile() {
|
||||
String keyId = "test-key-missing-file";
|
||||
String publicKeyBase64 =
|
||||
Base64.getEncoder().encodeToString(testKeyPair.getPublic().getEncoded());
|
||||
|
||||
JwtVerificationKey existingKey = new JwtVerificationKey(keyId, publicKeyBase64);
|
||||
new JwtVerificationKey(keyId, publicKeyBase64);
|
||||
|
||||
try (MockedStatic<InstallationPathConfig> mockedStatic =
|
||||
mockStatic(InstallationPathConfig.class)) {
|
||||
|
||||
@@ -133,10 +133,9 @@ class UserServiceTest {
|
||||
AuthenticationType authType = AuthenticationType.WEB;
|
||||
|
||||
// When & Then
|
||||
IllegalArgumentException exception =
|
||||
assertThrows(
|
||||
IllegalArgumentException.class,
|
||||
() -> userService.saveUser(invalidUsername, authType));
|
||||
assertThrows(
|
||||
IllegalArgumentException.class,
|
||||
() -> userService.saveUser(invalidUsername, authType));
|
||||
|
||||
verify(userRepository, never()).save(any(User.class));
|
||||
verify(databaseService, never()).exportDatabase();
|
||||
@@ -208,10 +207,9 @@ class UserServiceTest {
|
||||
AuthenticationType authType = AuthenticationType.WEB;
|
||||
|
||||
// When & Then
|
||||
IllegalArgumentException exception =
|
||||
assertThrows(
|
||||
IllegalArgumentException.class,
|
||||
() -> userService.saveUser(reservedUsername, authType));
|
||||
assertThrows(
|
||||
IllegalArgumentException.class,
|
||||
() -> userService.saveUser(reservedUsername, authType));
|
||||
|
||||
verify(userRepository, never()).save(any(User.class));
|
||||
verify(databaseService, never()).exportDatabase();
|
||||
@@ -224,10 +222,9 @@ class UserServiceTest {
|
||||
AuthenticationType authType = AuthenticationType.WEB;
|
||||
|
||||
// When & Then
|
||||
IllegalArgumentException exception =
|
||||
assertThrows(
|
||||
IllegalArgumentException.class,
|
||||
() -> userService.saveUser(anonymousUsername, authType));
|
||||
assertThrows(
|
||||
IllegalArgumentException.class,
|
||||
() -> userService.saveUser(anonymousUsername, authType));
|
||||
|
||||
verify(userRepository, never()).save(any(User.class));
|
||||
verify(databaseService, never()).exportDatabase();
|
||||
|
||||
Reference in New Issue
Block a user