ci: Add env vars for split units in sdk-platform-java#12765
ci: Add env vars for split units in sdk-platform-java#12765
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies Slf4jUtils to dynamically check for logging enablement at runtime instead of using a static final field. The associated unit tests were updated to toggle the logging state using a try-finally block. Feedback suggests improving test isolation by using @ResourceLock or a JUnit 5 extension to avoid potential race conditions when modifying global static state during parallel test execution.
| boolean originalValue = LoggingUtils.isLoggingEnabled(); | ||
| try { | ||
| LoggingUtils.setLoggingEnabled(false); | ||
| Logger logger = Slf4jUtils.getLogger(Slf4jUtilsTest.class); | ||
| assertEquals(NOPLogger.class, logger.getClass()); | ||
| assertFalse(logger.isInfoEnabled()); | ||
| assertFalse(logger.isDebugEnabled()); | ||
| } finally { | ||
| LoggingUtils.setLoggingEnabled(originalValue); | ||
| } |
There was a problem hiding this comment.
Modifying global static state via LoggingUtils.setLoggingEnabled(false) in a test can lead to flakiness and race conditions when tests are executed in parallel. While the try-finally block ensures the state is restored, it does not isolate this test from other concurrent tests that may rely on the same global state. Consider using a more isolated approach, such as a JUnit 5 extension or @ResourceLock, to manage shared state safely.
References
- Ensuring thread safety and avoiding race conditions is critical for reliable execution, especially in environments where tasks or tests may run concurrently.
1 participant
No description provided.