Skip to content

test: Refactor telemetry tests to follow repository style guidelines #345

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

Open
suni72 wants to merge 6 commits into
base: main
Choose a base branch
from
Open

test: Refactor telemetry tests to follow repository style guidelines #345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7c7f370
Refactor telemetry tests to follow AAA pattern
suni72 Jul 14, 2026
5a62c89
chore: clean up telemetry unit tests
suni72 Jul 14, 2026
3ec9992
refactor: consolidate test constants in GcsClientImplTest and remove …
suni72 Jul 14, 2026
8759175
Merge branch 'main' into telemetry-tests
suni72 Aug 3, 2026
b0a94d5
refactor: rename telemetry test methods to follow standard naming con…
suni72 Aug 3, 2026
81b89f2
Merge branch 'main' into telemetry-tests
suni72 Aug 5, 2026
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
64 changes: 20 additions & 44 deletions client/src/test/java/com/google/cloud/gcs/analyticscore/client/GcsClientImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ class GcsClientImplTest {
private static final String TEST_WRITE_OBJECT = "test-write-object";
private static final String TEST_NULL_OPTIONS_OBJECT = "test-null-options";
private static final String TEST_NON_EXISTENT_OBJECT = "non-existent";
private static final String NON_EXISTENT_BUCKET = "non-existent-bucket";
private static final String TEST_OBJECT_NAME = "test-object-name";
private static final String BLOB_WRITE_SESSION_CONFIG_FIELD = "blobWriteSessionConfig";
private static final int MB = 1024 * 1024;
Expand Down Expand Up @@ -168,7 +169,7 @@ void getGcsItemInfo_nonExistentBlob_throwsIOException() {
void openReadChannel_gcsObjectExists_returnsChannelWithCorrectSizeAndContent()
throws IOException {
String objectData = "hello world";
GcsReadOptions readOptions = GcsReadOptions.builder().setUserProjectId("test-project").build();
GcsReadOptions readOptions = GcsReadOptions.builder().setUserProjectId(TEST_PROJECT).build();
GcsItemId itemId =
GcsItemId.builder().setBucketName(TEST_BUCKET_NAME).setObjectName(TEST_OBJECT_NAME).build();
GcsItemInfo itemInfo =
Expand All @@ -193,7 +194,7 @@ void openReadChannel_gcsObjectExists_returnsChannelWithCorrectSizeAndContent()
void openReadChannel_itemId_gcsObjectExists_returnsChannelWithCorrectSizeAndContent()
throws IOException {
String objectData = "hello world";
GcsReadOptions readOptions = GcsReadOptions.builder().setUserProjectId("test-project").build();
GcsReadOptions readOptions = GcsReadOptions.builder().setUserProjectId(TEST_PROJECT).build();
GcsItemId itemId =
GcsItemId.builder().setBucketName(TEST_BUCKET_NAME).setObjectName(TEST_OBJECT_NAME).build();
StorageTestUtils.createBlobInStorage(
Expand All @@ -212,8 +213,7 @@ void openReadChannel_itemId_gcsObjectExists_returnsChannelWithCorrectSizeAndCont

@Test
void openReadChannel_nullItemId_throwsNullPointerException() {
GcsReadOptions readOptions =
GcsReadOptions.builder().setUserProjectId("test-project-id").build();
GcsReadOptions readOptions = GcsReadOptions.builder().setUserProjectId(TEST_PROJECT).build();

NullPointerException e =
assertThrows(
Expand All @@ -224,8 +224,7 @@ void openReadChannel_nullItemId_throwsNullPointerException() {

@Test
void openReadChannel_nullItemInfo_throwsNullPointerException() {
GcsReadOptions readOptions =
GcsReadOptions.builder().setUserProjectId("test-project-id").build();
GcsReadOptions readOptions = GcsReadOptions.builder().setUserProjectId(TEST_PROJECT).build();

NullPointerException e =
assertThrows(
Expand Down Expand Up @@ -255,8 +254,7 @@ void openReadChannel_itemInfoPointsToDirectory_throwsIllegalArgumentException()
.setSize(0L)
.setContentGeneration(-1L)
.build();
GcsReadOptions readOptions =
GcsReadOptions.builder().setUserProjectId("test-project-id").build();
GcsReadOptions readOptions = GcsReadOptions.builder().setUserProjectId(TEST_PROJECT).build();

IllegalArgumentException e =
assertThrows(
Expand All @@ -282,7 +280,7 @@ void getUserAgent_noOptionalUserAgent() throws Exception {
void getUserAgent_withOptionalUserAgent() throws Exception {
GcsClientOptions options =
GcsClientOptions.builder()
.setProjectId("test-project")
.setProjectId(TEST_PROJECT)
.setUserAgent("custom-app/1.0")
.build();
GcsClientImpl client = new GcsClientImpl(options, executorServiceSupplier, telemetry);
Expand Down Expand Up @@ -353,9 +351,9 @@ void getBucketProperties_hnsNull_returnsFalse() throws IOException {
void getBucketProperties_bucketNotFound_returnsDisabledHns() throws Exception {
Storage mockStorage = mock(Storage.class);
GcsClientImpl localGcsClient = createClientWithMockStorage(mockStorage);
doReturn(null).when(mockStorage).get(eq("non-existent-bucket"), any(BucketGetOption.class));
doReturn(null).when(mockStorage).get(eq(NON_EXISTENT_BUCKET), any(BucketGetOption.class));

BucketProperties properties = localGcsClient.getBucketProperties("non-existent-bucket");
BucketProperties properties = localGcsClient.getBucketProperties(NON_EXISTENT_BUCKET);

assertThat(properties.isHnsEnabled()).isFalse();
}
Expand Down Expand Up @@ -507,12 +505,9 @@ void create_whenBucketOrObjectNotFound_throwsFileNotFoundException() throws Exce
Storage mockStorage = mock(Storage.class);
GcsClientImpl clientWithMock = createClientWithMockStorage(mockStorage);
GcsItemId itemId =
GcsItemId.builder()
.setBucketName("non-existent-bucket")
.setObjectName("test-object")
.build();
GcsItemId.builder().setBucketName(NON_EXISTENT_BUCKET).setObjectName(TEST_OBJECT).build();
BlobInfo blobInfo =
BlobInfo.newBuilder(BlobId.of("non-existent-bucket", "test-object"))
BlobInfo.newBuilder(BlobId.of(NON_EXISTENT_BUCKET, TEST_OBJECT))
.setContentType("application/octet-stream")
.build();
StorageException e404 = new StorageException(404, "Not Found");
Expand Down Expand Up @@ -795,19 +790,6 @@ void create_whenOpenThrowsIOException_propagatesIOException() throws Exception {
assertThat(thrown).isSameInstanceAs(ioException);
}

@Test
void getBlob_whenBucketNameIsNull_throwsNullPointerException() throws Exception {
Comment thread
dheerajsngh marked this conversation as resolved.
GcsItemId itemId = mock(GcsItemId.class);
when(itemId.getBucketName()).thenReturn(null);
when(itemId.getObjectName()).thenReturn(Optional.of(TEST_OBJECT));
when(itemId.isGcsObject()).thenReturn(true);

GcsClientImpl client =
new GcsClientImpl(TEST_GCS_CLIENT_OPTIONS, executorServiceSupplier, telemetry);

assertThrows(NullPointerException.class, () -> client.getGcsItemInfo(itemId));
}

@Test
void getGcsItemInfo_whenBucketItemIdProvided_throwsUnsupportedOperationException()
throws Exception {
Expand Down Expand Up @@ -1000,30 +982,27 @@ void createStorage_bidiDisabled_usesHttpTransport() throws IOException {
void createStorage_bidiEnabled_usesGrpcTransport() throws IOException {
GcsClientOptions options =
GcsClientOptions.builder()
.setProjectId("test-project")
.setProjectId(TEST_PROJECT)
.setGcsReadOptions(GcsReadOptions.builder().setBidiReadEnabled(true).build())
.build();

GcsClientImpl client =
new GcsClientImpl(NoCredentials.getInstance(), options, executorServiceSupplier, telemetry);

assertThat(client.storage.getOptions()).isInstanceOf(GrpcStorageOptions.class);
}

@Test
void openReadChannel_bidiEnabled_returnsGcsBidiReadChannel() throws IOException {
GcsReadOptions readOptions =
GcsReadOptions.builder().setUserProjectId("test-project").setBidiReadEnabled(true).build();
GcsReadOptions.builder().setUserProjectId(TEST_PROJECT).setBidiReadEnabled(true).build();
GcsItemId itemId =
GcsItemId.builder()
.setBucketName("test-bucket-name")
.setObjectName("test-object-name")
.build();
GcsItemId.builder().setBucketName(TEST_BUCKET_NAME).setObjectName(TEST_OBJECT_NAME).build();
GcsItemInfo itemInfo =
GcsItemInfo.builder().setItemId(itemId).setSize(100L).setContentGeneration(0L).build();

Storage mockStorage = mock(Storage.class);
ApiFuture<BlobReadSession> mockSessionFuture = mock(ApiFuture.class);
when(mockStorage.blobReadSession(any(BlobId.class))).thenReturn(mockSessionFuture);

GcsClient bidiClient =
new GcsClientImpl(TEST_GCS_CLIENT_OPTIONS, executorServiceSupplier, telemetry) {
@Override
Expand All @@ -1033,23 +1012,19 @@ protected Storage createStorage(Optional<Credentials> credentials) {
};

VectoredSeekableByteChannel channel = bidiClient.openReadChannel(itemInfo, readOptions);

assertThat(channel).isInstanceOf(GcsBidiReadChannel.class);
}

@Test
void openReadChannel_itemId_bidiEnabled_returnsGcsBidiReadChannel() throws IOException {
GcsReadOptions readOptions =
GcsReadOptions.builder().setUserProjectId("test-project").setBidiReadEnabled(true).build();
GcsReadOptions.builder().setUserProjectId(TEST_PROJECT).setBidiReadEnabled(true).build();
GcsItemId itemId =
GcsItemId.builder()
.setBucketName("test-bucket-name")
.setObjectName("test-object-name")
.build();

GcsItemId.builder().setBucketName(TEST_BUCKET_NAME).setObjectName(TEST_OBJECT_NAME).build();
Storage mockStorage = mock(Storage.class);
ApiFuture<BlobReadSession> mockSessionFuture = mock(ApiFuture.class);
when(mockStorage.blobReadSession(any(BlobId.class))).thenReturn(mockSessionFuture);

GcsClient bidiClient =
new GcsClientImpl(TEST_GCS_CLIENT_OPTIONS, executorServiceSupplier, telemetry) {
@Override
Expand All @@ -1059,6 +1034,7 @@ protected Storage createStorage(Optional<Credentials> credentials) {
};

VectoredSeekableByteChannel channel = bidiClient.openReadChannel(itemId, readOptions);

assertThat(channel).isInstanceOf(GcsBidiReadChannel.class);
}
}
6 changes: 3 additions & 3 deletions ...com/google/cloud/gcs/analyticscore/common/telemetry/LoggingOpenTelemetryProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
class LoggingOpenTelemetryProviderTest {

@Test
void testGet_returnsNonNullInstance() {
void get_returnsNonNullInstance() {
try (LoggingOpenTelemetryProvider provider =
new LoggingOpenTelemetryProvider(OpenTelemetryOptions.builder().build())) {
OpenTelemetry openTelemetry = provider.getOpenTelemetry();
Expand All @@ -33,7 +33,7 @@ void testGet_returnsNonNullInstance() {
}

@Test
void testGet_returnsSameInstanceOnMultipleCalls() {
void get_returnsSameInstanceOnMultipleCalls() {
try (LoggingOpenTelemetryProvider provider =
new LoggingOpenTelemetryProvider(OpenTelemetryOptions.builder().build())) {
OpenTelemetry firstCall = provider.getOpenTelemetry();
Expand All @@ -44,7 +44,7 @@ void testGet_returnsSameInstanceOnMultipleCalls() {
}

@Test
void testConstructor_withDuration_createsSuccessfully() {
void constructor_withDuration_createsSuccessfully() {
try (LoggingOpenTelemetryProvider provider =
new LoggingOpenTelemetryProvider(
OpenTelemetryOptions.builder().setExportIntervalSeconds(30).build())) {
Expand Down
8 changes: 4 additions & 4 deletions ...java/com/google/cloud/gcs/analyticscore/common/telemetry/LoggingTelemetryOptionsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@
class LoggingTelemetryOptionsTest {

@Test
void testLoggingTelemetryOptionsDefaultValues() {
void loggingTelemetryOptions_defaultValues_returnsDefaultOptions() {
LoggingTelemetryOptions options = LoggingTelemetryOptions.builder().build();

assertThat(options.isEnabled()).isFalse();
assertThat(options.getLogLevel()).isEqualTo(LoggingTelemetryOptions.LogLevel.DEBUG);
}

@Test
void testCreateFromOptions_NoOptions() {
void createFromOptions_noOptions_returnsEmpty() {
Map<String, String> options = new HashMap<>();
Optional<LoggingTelemetryOptions> telemetryOptions =
LoggingTelemetryOptions.createFromOptions(options, "prefix.");
Expand All @@ -42,7 +42,7 @@ void testCreateFromOptions_NoOptions() {
}

@Test
void testCreateFromOptions_WithAllOptions() {
void createFromOptions_withAllOptions_returnsPresentOptions() {
Map<String, String> options = new HashMap<>();
options.put("prefix.telemetry.logging.enabled", "true");
options.put("prefix.telemetry.logging.level", "ERROR");
Expand All @@ -57,7 +57,7 @@ void testCreateFromOptions_WithAllOptions() {
}

@Test
void testCreateFromOptions_WithInvalidLevel_fallsbackToDefaults() {
void createFromOptions_withInvalidLevel_fallsBackToDefaults() {
Map<String, String> options = new HashMap<>();
options.put("prefix.telemetry.logging.enabled", "true");
options.put("prefix.telemetry.logging.level", "INVALID_LEVEL");
Expand Down
26 changes: 3 additions & 23 deletions ...ava/com/google/cloud/gcs/analyticscore/common/telemetry/LoggingTelemetryReporterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,27 +26,7 @@
class LoggingTelemetryReporterTest {

@Test
public void testLoggingOptionsDefaultValues() {
Comment thread
dheerajsngh marked this conversation as resolved.
LoggingTelemetryOptions options = LoggingTelemetryOptions.builder().build();

assertThat(options.isEnabled()).isFalse();
assertThat(options.getLogLevel()).isEqualTo(LoggingTelemetryOptions.LogLevel.DEBUG);
}

@Test
public void testLoggingOptionsCustomValues() {

@dheerajsngh dheerajsngh Aug 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't this tests is present. We can add this back.

@suni72 suni72 Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

LoggingTelemetryOptions is an AutoValue class so generally we don't need to test the builder. Also testCreateFromOptions_WithAllOptions in LoggingTelemetryOptionsTest.java checks that custom values from options are set correctly.

LoggingTelemetryOptions options =
LoggingTelemetryOptions.builder()
.setEnabled(true)
.setLogLevel(LoggingTelemetryOptions.LogLevel.INFO)
.build();

assertThat(options.isEnabled()).isTrue();
assertThat(options.getLogLevel()).isEqualTo(LoggingTelemetryOptions.LogLevel.INFO);
}

@Test
public void testFormatMetrics_singleMetricWithoutAttributes() {
void formatMetrics_singleMetricWithoutAttributes() {
try (LoggingTelemetryReporter reporter =
new LoggingTelemetryReporter(LoggingTelemetryOptions.builder().build())) {
Map<MetricKey, Long> metrics =
Expand All @@ -63,7 +43,7 @@ public void testFormatMetrics_singleMetricWithoutAttributes() {
}

@Test
public void testFormatMetrics_singleMetricWithAttributes() {
void formatMetrics_singleMetricWithAttributes() {
try (LoggingTelemetryReporter reporter =
new LoggingTelemetryReporter(LoggingTelemetryOptions.builder().build())) {
Map<MetricKey, Long> metrics =
Expand All @@ -84,7 +64,7 @@ public void testFormatMetrics_singleMetricWithAttributes() {
}

@Test
public void testFormatMetrics_multipleMetrics() {
void formatMetrics_multipleMetrics() {
try (LoggingTelemetryReporter reporter =
new LoggingTelemetryReporter(LoggingTelemetryOptions.builder().build())) {
Map<MetricKey, Long> metrics =
Expand Down
12 changes: 6 additions & 6 deletions ...st/java/com/google/cloud/gcs/analyticscore/common/telemetry/OpenTelemetryOptionsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@
class OpenTelemetryOptionsTest {

@Test
void testOpenTelemetryOptionsDefaultValues() {
void openTelemetryOptions_defaultValues_returnsDefaultOptions() {
OpenTelemetryOptions options = OpenTelemetryOptions.builder().build();
assertThat(options.isEnabled()).isFalse();
assertThat(options.getProviderType()).isEqualTo(OpenTelemetryOptions.ProviderType.GLOBAL);
}

@Test
void testOpenTelemetryOptionsCustomValues() {
void openTelemetryOptions_customValues_returnsCustomOptions() {
OpenTelemetry customTelemetry = GlobalOpenTelemetry.get();
OpenTelemetryOptions options =
OpenTelemetryOptions.builder()
Expand All @@ -52,7 +52,7 @@ void testOpenTelemetryOptionsCustomValues() {
}

@Test
void testOpenTelemetryOptionsLoggingProvider() {
void openTelemetryOptions_loggingProvider_returnsLoggingOptions() {
OpenTelemetryOptions options =
OpenTelemetryOptions.builder()
.setEnabled(true)
Expand All @@ -64,7 +64,7 @@ void testOpenTelemetryOptionsLoggingProvider() {
}

@Test
void testCreateFromOptions_NoOptions() {
void createFromOptions_noOptions_returnsEmpty() {
Map<String, String> options = new HashMap<>();
Optional<OpenTelemetryOptions> telemetryOptions =
OpenTelemetryOptions.createFromOptions(options, "prefix.");
Expand All @@ -73,7 +73,7 @@ void testCreateFromOptions_NoOptions() {
}

@Test
void testCreateFromOptions_WithAllOptions() {
void createFromOptions_withAllOptions_returnsPresentOptions() {
Map<String, String> options = new HashMap<>();
options.put("prefix.telemetry.opentelemetry.enabled", "true");
options.put("prefix.telemetry.opentelemetry.provider-type", "PRE_CONFIGURED");
Expand All @@ -90,7 +90,7 @@ void testCreateFromOptions_WithAllOptions() {
}

@Test
void testCreateFromOptions_WithInvalidValues_fallsbackToDefaults() {
void createFromOptions_withInvalidValues_fallsBackToDefaults() {
Map<String, String> options = new HashMap<>();
options.put("prefix.telemetry.opentelemetry.enabled", "true");
options.put("prefix.telemetry.opentelemetry.provider-type", "INVALID_PROVIDER");
Expand Down
2 changes: 1 addition & 1 deletion ...t/java/com/google/cloud/gcs/analyticscore/common/telemetry/OpenTelemetryReporterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ void setUp() {
}

@Test
void testOperationEnd_recordsMetrics() {
void operationEnd_recordsMetrics() {
OpenTelemetryOptions options =
OpenTelemetryOptions.builder()
.setEnabled(true)
Expand Down
Loading
Loading