Skip to content

test: switch all test logging to slf4j instead of j.u.l #3108

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

Merged
merged 1 commit into from
May 22, 2025
Merged

test: switch all test logging to slf4j instead of j.u.l #3108

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
7 changes: 4 additions & 3 deletions .../src/test/java/com/google/cloud/storage/ITGapicBidiUnbufferedWritableByteChannelTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,12 @@
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.function.BiConsumer;
import java.util.logging.Logger;
import java.util.stream.Collector;
import java.util.stream.Collectors;
import org.checkerframework.checker.nullness.qual.NonNull;
import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public final class ITGapicBidiUnbufferedWritableByteChannelTest {

Expand Down Expand Up @@ -830,7 +831,7 @@ public void incremental_partialSuccess() throws Exception {
}

static class BidiWriteService extends StorageImplBase {
private static final Logger LOGGER = Logger.getLogger(BidiWriteService.class.getName());
private static final Logger LOGGER = LoggerFactory.getLogger(BidiWriteService.class);
private final BiConsumer<StreamObserver<BidiWriteObjectResponse>, List<BidiWriteObjectRequest>>
c;

Expand Down Expand Up @@ -878,7 +879,7 @@ private static void logUnexpectedRequest(
.map(l -> l.stream().map(StorageV2ProtoUtils::fmtProto).collect(oneLine))
.collect(joining),
build.stream().map(StorageV2ProtoUtils::fmtProto).collect(oneLine));
LOGGER.warning(msg);
LOGGER.warn(msg);
}

@Override
Expand Down
11 changes: 6 additions & 5 deletions ...rage/src/test/java/com/google/cloud/storage/ITGapicUnbufferedWritableByteChannelTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,15 @@
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.BiConsumer;
import java.util.logging.Logger;
import java.util.stream.Collector;
import java.util.stream.Collectors;
import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public final class ITGapicUnbufferedWritableByteChannelTest {
private static final Logger LOGGER =
Logger.getLogger(ITGapicUnbufferedWritableByteChannelTest.class.getName());
LoggerFactory.getLogger(ITGapicUnbufferedWritableByteChannelTest.class);

private static final Hasher HASHER = Hasher.enabled();
private static final ChunkSegmenter segmenter =
Expand Down Expand Up @@ -202,7 +203,7 @@ public void resumableUpload() throws IOException, InterruptedException, Executio
c.close();
} catch (PermissionDeniedException ignore) {
for (String debugMessage : debugMessages) {
LOGGER.warning(debugMessage);
LOGGER.warn(debugMessage);
}
}
assertThat(result.get()).isEqualTo(resp5);
Expand Down Expand Up @@ -331,7 +332,7 @@ public void resumableUpload_finalizeWhenWriteAndCloseCalledEvenWhenQuantumAligne
}

static class DirectWriteService extends StorageImplBase {
private static final Logger LOGGER = Logger.getLogger(DirectWriteService.class.getName());
private static final Logger LOGGER = LoggerFactory.getLogger(DirectWriteService.class);
private final BiConsumer<StreamObserver<WriteObjectResponse>, List<WriteObjectRequest>> c;

private ImmutableList.Builder<WriteObjectRequest> requests;
Expand Down Expand Up @@ -368,7 +369,7 @@ private static void logUnexpectedRequest(
.map(l -> l.stream().map(StorageV2ProtoUtils::fmtProto).collect(oneLine))
.collect(joining),
build.stream().map(StorageV2ProtoUtils::fmtProto).collect(oneLine));
LOGGER.warning(msg);
LOGGER.warn(msg);
}

@Override
Expand Down
19 changes: 10 additions & 9 deletions ...rage/src/test/java/com/google/cloud/storage/conformance/retry/ITRetryConformanceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,15 @@
import java.util.Set;
import java.util.function.BiPredicate;
import java.util.function.Predicate;
import java.util.logging.Logger;
import java.util.stream.Collectors;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.junit.After;
import org.junit.AssumptionViolatedException;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Load and dynamically generate a series of test cases to verify if the {@link Storage} and
Expand All @@ -86,7 +87,7 @@
@Parameterized(RetryConformanceParameterProvider.class)
@ParallelFriendly
public class ITRetryConformanceTest {
private static final Logger LOGGER = Logger.getLogger(ITRetryConformanceTest.class.getName());
private static final Logger LOGGER = LoggerFactory.getLogger(ITRetryConformanceTest.class);

private RetryTestFixture retryTestFixture;

Expand All @@ -100,7 +101,7 @@ public class ITRetryConformanceTest {

@Before
public void setUp() throws Throwable {
LOGGER.fine("Running setup...");
LOGGER.trace("Running setup...");
retryTestFixture = retryParameter.retryTestFixture;
testRetryConformance = retryParameter.testRetryConformance;
mapping = retryParameter.rpcMethodMapping;
Expand All @@ -111,20 +112,20 @@ public void setUp() throws Throwable {
// the case setup fails for some reason
ctx = ctx(nonTestStorage, empty());
ctx = mapping.getSetup().apply(ctx, testRetryConformance).leftMap(s -> testStorage);
LOGGER.fine("Running setup complete");
LOGGER.trace("Running setup complete");
}

@After
public void tearDown() throws Throwable {
LOGGER.fine("Running teardown...");
LOGGER.trace("Running teardown...");
if (ctx != null) {
ctx = ctx.leftMap(s -> nonTestStorage);
getReplaceStorageInObjectsFromCtx()
.andThen(mapping.getTearDown())
.apply(ctx, testRetryConformance);
}
retryTestFixture.finished(null);
LOGGER.fine("Running teardown complete");
LOGGER.trace("Running teardown complete");
}

/**
Expand All @@ -133,7 +134,7 @@ public void tearDown() throws Throwable {
*/
@Test
public void test() throws Throwable {
LOGGER.fine("Running test...");
LOGGER.trace("Running test...");
assertThat(ctx).isNotNull();
try {
ctx =
Expand All @@ -146,7 +147,7 @@ public void test() throws Throwable {
retryTestFixture.failed(e, null);
throw e;
}
LOGGER.fine("Running test complete");
LOGGER.trace("Running test complete");
}

/**
Expand Down Expand Up @@ -374,7 +375,7 @@ private void validateGeneratedTestCases(
.collect(Collectors.toSet()));

if (!unusedMappings.isEmpty()) {
LOGGER.warning(
LOGGER.warn(
String.format(
Locale.US,
"Declared but unused mappings with ids: [%s]",
Expand Down
23 changes: 15 additions & 8 deletions ...ud-storage/src/test/java/com/google/cloud/storage/conformance/retry/RetryTestFixture.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,12 @@
import com.google.common.collect.ImmutableMap;
import java.io.IOException;
import java.util.Locale;
import java.util.logging.Logger;
import org.junit.AssumptionViolatedException;
import org.junit.rules.TestRule;
import org.junit.rules.TestWatcher;
import org.junit.runner.Description;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* A JUnit 4 {@link TestRule} which integrates with {@link TestBench} and {@link
Expand All @@ -44,7 +45,7 @@
* <p>Provides pre-configured instances of {@link Storage} for setup/teardown & test.
*/
final class RetryTestFixture extends TestWatcher {
private static final Logger LOGGER = Logger.getLogger(RetryTestFixture.class.getName());
private static final Logger LOGGER = LoggerFactory.getLogger(RetryTestFixture.class);
private static final int STATUS_CODE_NOT_IMPLEMENTED = 501;

private final CleanupStrategy cleanupStrategy;
Expand Down Expand Up @@ -83,7 +84,7 @@ public Storage getTestStorage() {

@Override
protected void starting(Description description) {
LOGGER.fine("Setting up retry_test resource...");
LOGGER.trace("Setting up retry_test resource...");
RetryTestResource retryTestResource =
RetryTestResource.newRetryTestResource(
testRetryConformance.getMethod(),
Expand All @@ -93,20 +94,26 @@ protected void starting(Description description) {
retryTest = testBench.createRetryTest(retryTestResource);
} catch (HttpResponseException e) {
if (e.getStatusCode() == STATUS_CODE_NOT_IMPLEMENTED) {
throw new AssumptionViolatedException(
"Testbench not yet implemented for " + retryTestResource);
AssumptionViolatedException exception =
new AssumptionViolatedException(
"Testbench not yet implemented for " + retryTestResource);
// make skips due to not implemented more terse
// we know where this comes from, we don't need the full stack trace for each of the
// 200+ occurrences.
exception.setStackTrace(new StackTraceElement[0]);
throw exception;
} else {
throw new RuntimeException(e);
}
} catch (IOException e) {
throw new RuntimeException(e);
}
LOGGER.fine("Setting up retry_test resource complete");
LOGGER.trace("Setting up retry_test resource complete");
}

@Override
protected void finished(Description description) {
LOGGER.fine("Verifying end state of retry_test resource...");
LOGGER.trace("Verifying end state of retry_test resource...");
try (Storage ignore1 = nonTestStorage;
Storage ignore2 = testStorage) { // use try-with to shut down grpc resources
try {
Expand All @@ -117,7 +124,7 @@ protected void finished(Description description) {
}
}
} finally {
LOGGER.fine("Verifying end state of retry_test resource complete");
LOGGER.trace("Verifying end state of retry_test resource complete");
if ((shouldCleanup(testSuccess, testSkipped)) && retryTest != null) {
testBench.deleteRetryTest(retryTest);
retryTest = null;
Expand Down
9 changes: 5 additions & 4 deletions ...d-storage/src/test/java/com/google/cloud/storage/conformance/retry/RpcMethodMappings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,9 @@
import java.util.OptionalInt;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.logging.Logger;
import java.util.stream.Collectors;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* A class which serves to try and organize all of the {@link RpcMethodMapping} for the retry
Expand All @@ -120,7 +121,7 @@
@Immutable
@SuppressWarnings("Guava")
final class RpcMethodMappings {
private static final Logger LOGGER = Logger.getLogger(RpcMethodMappings.class.getName());
private static final Logger LOGGER = LoggerFactory.getLogger(RpcMethodMappings.class);

private static final Predicate<TestRetryConformance> groupIsDownload =
methodGroupIs("storage.objects.download");
Expand Down Expand Up @@ -224,7 +225,7 @@ private void validateMappingDefinitions(ArrayList<RpcMethodMapping> a) {

private void reportMappingSummary() {
int mappingCount = funcMap.values().stream().mapToInt(m -> 1).sum();
LOGGER.info("Current total number of mappings defined: " + mappingCount);
LOGGER.info("Current total number of mappings defined: {}", mappingCount);
String counts =
funcMap.asMap().entrySet().stream()
.map(
Expand All @@ -244,7 +245,7 @@ private void reportMappingSummary() {
})
.sorted()
.collect(Collectors.joining("\n", "\n", ""));
LOGGER.info("Current number of mappings per rpc method: " + counts);
LOGGER.info("Current number of mappings per rpc method: {}", counts);
OptionalInt max =
funcMap.values().stream().map(RpcMethodMapping::getMappingId).mapToInt(i -> i).max();
if (max.isPresent()) {
Expand Down
28 changes: 14 additions & 14 deletions google-cloud-storage/src/test/java/com/google/cloud/storage/it/BucketCleaner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,18 +42,18 @@
import java.util.Comparator;
import java.util.List;
import java.util.Locale;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public final class BucketCleaner {

private static final Logger LOGGER = Logger.getLogger(BucketCleaner.class.getName());
private static final Logger LOGGER = LoggerFactory.getLogger(BucketCleaner.class);

public static void doCleanup(String bucketName, Storage s) {
LOGGER.fine("Starting bucket cleanup...");
LOGGER.trace("Starting bucket cleanup...");
String projectId = s.getOptions().getProjectId();
try {
// TODO: probe bucket existence, a bad test could have deleted the bucket
Expand All @@ -73,16 +73,16 @@ public static void doCleanup(String bucketName, Storage s) {
if (!anyFailedObjectDeletes) {
s.delete(bucketName, BucketSourceOption.userProject(projectId));
} else {
LOGGER.warning("Unable to delete bucket due to previous failed object deletes");
LOGGER.warn("Unable to delete bucket due to previous failed object deletes");
}
LOGGER.fine("Bucket cleanup complete");
LOGGER.trace("Bucket cleanup complete");
} catch (Exception e) {
LOGGER.log(Level.SEVERE, e, () -> "Error during bucket cleanup.");
LOGGER.error("Error during bucket cleanup.");
}
}

public static void doCleanup(String bucketName, Storage s, StorageControlClient ctrl) {
LOGGER.warning("Starting bucket cleanup: " + bucketName);
LOGGER.warn("Starting bucket cleanup: {}", bucketName);
String projectId = s.getOptions().getProjectId();
try {
// TODO: probe bucket existence, a bad test could have deleted the bucket
Expand Down Expand Up @@ -140,7 +140,7 @@ public static void doCleanup(String bucketName, Storage s, StorageControlClient
folder -> {
String formatted =
String.format(Locale.US, "folder = %s", folder.getName());
LOGGER.warning(formatted);
LOGGER.warn(formatted);
boolean success = true;
try {
ctrl.deleteFolderCallable()
Expand Down Expand Up @@ -176,7 +176,7 @@ public static void doCleanup(String bucketName, Storage s, StorageControlClient
managedFolder -> {
String formatted =
String.format(Locale.US, "managedFolder = %s", managedFolder.getName());
LOGGER.warning(formatted);
LOGGER.warn(formatted);
boolean success = true;
try {
ctrl.deleteManagedFolderCallable()
Expand Down Expand Up @@ -214,17 +214,17 @@ public static void doCleanup(String bucketName, Storage s, StorageControlClient
if (!anyFailedObjectDelete && !anyFailedFolderDelete && !anyFailedManagedFolderDelete) {
s.delete(bucketName, BucketSourceOption.userProject(projectId));
} else {
LOGGER.warning(
LOGGER.warn(
String.format(
Locale.US,
"Unable to delete bucket %s due to previous failed %s deletes",
bucketName,
failed));
}

LOGGER.warning("Bucket cleanup complete: " + bucketName);
LOGGER.warn("Bucket cleanup complete: {}", bucketName);
} catch (Exception e) {
LOGGER.log(Level.SEVERE, e, () -> "Error during bucket cleanup.");
LOGGER.error("Error during bucket cleanup.");
}
}

Expand All @@ -234,7 +234,7 @@ private static boolean getIfAnyFailedAndReport(
deleteResults.stream().filter(r -> !r.success).collect(Collectors.toList());
failedDeletes.forEach(
r ->
LOGGER.warning(
LOGGER.warn(
String.format(
Locale.US, "Failed to delete %s %s/%s", resourceType, bucketName, r.name)));
return !failedDeletes.isEmpty();
Expand Down
Loading