Skip to content

ci: Add env vars for split units in sdk-platform-java #12765

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
lqiu96 wants to merge 1 commit into
base: main
Choose a base branch
from
+40 −6
Draft

ci: Add env vars for split units in sdk-platform-java #12765

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,16 @@ jobs:
BUILD_SUBDIR: ${{matrix.package}}
JOB_TYPE: test
JOB_NAME: units-${{matrix.package}}-${{matrix.java}}
- name: Env Var Tests
if: matrix.package == 'sdk-platform-java'
run: .kokoro/build.sh
env:
BUILD_SUBDIR: ${{matrix.package}}
JOB_TYPE: test
SUREFIRE_JVM_OPT: '-PenvVarTest'
GOOGLE_CLOUD_UNIVERSE_DOMAIN: random.com
GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS: true
GOOGLE_SDK_JAVA_LOGGING: true
split-units-8:
runs-on: ubuntu-latest
name: "split-units"
Expand Down Expand Up @@ -198,6 +208,25 @@ jobs:
JOB_TYPE: test
JOB_NAME: units-8-runtime-${{matrix.java}}
working-directory: ${{matrix.package}}
- name: Run Env Var Tests in Java ${{matrix.java}} with the source compiled in Java 11
if: matrix.package == 'sdk-platform-java'
run: |
mvn test \
-B -ntp \
-Pquick-build \
-PenvVarTest \
-Dorg.slf4j.simpleLogger.showDateTime=true \
-Dorg.slf4j.simpleLogger.dateTimeFormat=HH:mm:ss:SSS \
-Dmaven.wagon.http.retryHandler.count=5 \
--also-make \
-T 1C
env:
BUILD_SUBDIR: ${{matrix.package}}
JOB_TYPE: test
GOOGLE_CLOUD_UNIVERSE_DOMAIN: random.com
GOOGLE_CLOUD_ENABLE_DIRECT_PATH_XDS: true
GOOGLE_SDK_JAVA_LOGGING: true
working-directory: ${{matrix.package}}
windows:
runs-on: windows-latest
steps:
Expand Down
3 changes: 1 addition & 2 deletions sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/logging/Slf4jUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
class Slf4jUtils {

private static final Logger NO_OP_LOGGER = org.slf4j.helpers.NOPLogger.NOP_LOGGER;
private static final boolean loggingEnabled = LoggingUtils.isLoggingEnabled();

private static final boolean isSLF4J2x;

Expand All @@ -70,7 +69,7 @@ static Logger getLogger(Class<?> clazz) {

// constructor with LoggerFactoryProvider to make testing easier
static Logger getLogger(Class<?> clazz, LoggerFactoryProvider factoryProvider) {
if (loggingEnabled) {
if (LoggingUtils.isLoggingEnabled()) {
ILoggerFactory loggerFactory = factoryProvider.getLoggerFactory();
return loggerFactory.getLogger(clazz.getName());
} else {
Expand Down
14 changes: 10 additions & 4 deletions sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/logging/Slf4jUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,16 @@ class Slf4jUtilsTest {
// This test should only run GOOGLE_SDK_JAVA_LOGGING != true
@Test
void testGetLogger_loggingDisabled_shouldGetNOPLogger() {
Logger logger = Slf4jUtils.getLogger(Slf4jUtilsTest.class);
assertEquals(NOPLogger.class, logger.getClass());
assertFalse(logger.isInfoEnabled());
assertFalse(logger.isDebugEnabled());
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);
}
Comment on lines +60 to +69
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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
  1. Ensuring thread safety and avoiding race conditions is critical for reliable execution, especially in environments where tasks or tests may run concurrently.

}

// These tests does not require GOOGLE_SDK_JAVA_LOGGING
Expand Down
Loading