Refactor: Use Standard OpenRewrite Recipes to implement Slf4j logging automatically #48
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… recipe to use slf4j logging
… to use @Sl4j annotation for loggers
Replaced `System.out.println` with proper logging using `log.error` to ensure exceptions are logged consistently and include stack trace details. This improves debugging and aligns with standard logging practices.
Eliminated unnecessary `verify(mockUtility)` calls from unit tests to improve clarity and maintain focus on actual test assertions. This helps streamline the code and reduce noise in test output.
Eliminated unnecessary `verify(mockUtility)` calls from unit tests to improve clarity and maintain focus on actual test assertions. This helps streamline the code and reduce noise in test output.
Eliminated unnecessary `verify(mockUtility)` calls from unit tests to improve clarity and maintain focus on actual test assertions. This helps streamline the code and reduce noise in test output.
Eliminated unnecessary `verify(mockUtility)` calls from unit tests to improve clarity and maintain focus on actual test assertions. This helps streamline the code and reduce noise in test output.
Log message assertions were removed as they provide limited value and rely on inspecting logs, which is discouraged. A low-value test was disabled due to its dependency on log messages without meaningful functional validation.
Removed unnecessary log and verify statements from CertExpirationConsumerTest to improve code clarity and maintainability. These changes help streamline test assertions without impacting functionality.
Added SLF4J best practices recipe to enforce proper logging conventions. Updated Maven dependencies and configuration to include the new rewrite-logging-frameworks artifact.
…actices to implement best practices Replaced `log.info` with appropriate `log.warn` or `log.error` in error-prone scenarios and updated parameterized logging to use placeholders instead of string formatting. This improves log readability, corrects log levels, and standardizes logging practices across the codebase.
mcook42
commented
Apr 6, 2025
| verify(mockUtility, times(3)).logWithDate(logMessageCaptor.capture(), eq(ValidateTmdd.class)); | ||
| Assertions.assertEquals("Error sending summary email:", logMessageCaptor.getAllValues().get(1)); | ||
| Assertions.assertEquals("Failed to send error summary email:", logMessageCaptor.getAllValues().get(2)); | ||
| // TODO: how to assert behavior without inspecting logs |
There was a problem hiding this comment.
Note: this TODO is intentional. The value of this automated refactor is to get more useful and detailed logs for debugging and system validation. The test in question is highly coupled to logging to the point that verification of behaviors is difficult without inspecting the logs directly. The value of the refactor was determined to greatly outweigh the value of this edge case test verification. Instead of removing the test entirely, I disabled it with a note for future maintainers about why it's disabled. Should we find in the future that this test case is important, the structure is there and available to whomever decides to reimplement the test.
Deleted Maven wrapper files to clean up the project. These files are no longer required as the project setup does not rely on the Maven wrapper tooling.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Use OpenRewrite to replace direct system calls with structured logging. This will significantly improve maintainers' ability to understand and debug program execution. The only manual edits were made in the tests. All other code changes were automatically performed by their respective OpenRewrite Recipes using the Maven plugin.
How
logWithDateand inlined theirSystem.out.printlncalls (necessary prep for the next step)@Slf4jannotations instead of explicit properties