Skip to content

Commit

Permalink
chore(test): deflake and refactor test fake service helper (#1267)
Browse files Browse the repository at this point in the history
There is a race condition when running unit tests in parallel that where they can pick the same port for a fake service. This PR adds a retry loop to let them resolve the race.
It also refactors the helper into a builder instead, avoiding the need for multiple constructors and empty args
  • Loading branch information
igorbernstein2 authored May 27, 2022
1 parent a7b8358 commit ae0d60d
Show file tree
Hide file tree
Showing 11 changed files with 197 additions and 189 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,9 @@
import com.google.cloud.bigtable.data.v2.internal.NameUtil;
import com.google.cloud.bigtable.data.v2.models.RowMutation;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.protobuf.ByteString;
import io.grpc.Attributes;
import io.grpc.BindableService;
import io.grpc.ServerInterceptor;
import io.grpc.Server;
import io.grpc.ServerTransportFilter;
import io.grpc.stub.StreamObserver;
import java.io.IOException;
Expand Down Expand Up @@ -68,7 +66,7 @@ public class BigtableDataClientFactoryTest {
private static final String DEFAULT_INSTANCE_ID = "fake-instance";
private static final String DEFAULT_APP_PROFILE_ID = "fake-app-profile";

private FakeServiceHelper serviceHelper;
private Server server;
private FakeBigtableService service;

private TransportChannelProvider transportChannelProvider;
Expand All @@ -77,37 +75,32 @@ public class BigtableDataClientFactoryTest {
private WatchdogProvider watchdogProvider;
private ApiClock apiClock;
private BigtableDataSettings defaultSettings;
private int port;

private final BlockingQueue<Attributes> setUpAttributes = new LinkedBlockingDeque<>();
private final BlockingQueue<Attributes> terminateAttributes = new LinkedBlockingDeque<>();

@Before
public void setUp() throws IOException {
service = new FakeBigtableService();
ServerTransportFilter transportFilter =
new ServerTransportFilter() {
@Override
public Attributes transportReady(Attributes transportAttrs) {
setUpAttributes.add(transportAttrs);
return super.transportReady(transportAttrs);
}

@Override
public void transportTerminated(Attributes transportAttrs) {
terminateAttributes.add(transportAttrs);
}
};
serviceHelper =
new FakeServiceHelper(
ImmutableList.<ServerInterceptor>of(),
transportFilter,
ImmutableList.<BindableService>of(service));
port = serviceHelper.getPort();
serviceHelper.start();
server =
FakeServiceBuilder.create(service)
.addTransportFilter(
new ServerTransportFilter() {
@Override
public Attributes transportReady(Attributes transportAttrs) {
setUpAttributes.add(transportAttrs);
return super.transportReady(transportAttrs);
}

@Override
public void transportTerminated(Attributes transportAttrs) {
terminateAttributes.add(transportAttrs);
}
})
.start();

BigtableDataSettings.Builder builder =
BigtableDataSettings.newBuilderForEmulator(port)
BigtableDataSettings.newBuilderForEmulator(server.getPort())
.setProjectId(DEFAULT_PROJECT_ID)
.setInstanceId(DEFAULT_INSTANCE_ID)
.setAppProfileId(DEFAULT_APP_PROFILE_ID);
Expand Down Expand Up @@ -152,7 +145,7 @@ public void transportTerminated(Attributes transportAttrs) {

@After
public void tearDown() {
serviceHelper.shutdown();
server.shutdown();
}

@Test
Expand Down Expand Up @@ -234,7 +227,7 @@ public void testCreateWithRefreshingChannel() throws Exception {
String[] tableIds = {"fake-table1", "fake-table2"};
int poolSize = 3;
BigtableDataSettings.Builder builder =
BigtableDataSettings.newBuilderForEmulator(port)
BigtableDataSettings.newBuilderForEmulator(server.getPort())
.setProjectId(DEFAULT_PROJECT_ID)
.setInstanceId(DEFAULT_INSTANCE_ID)
.setAppProfileId(DEFAULT_APP_PROFILE_ID)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/*
* Copyright 2022 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.google.cloud.bigtable.data.v2;

import io.grpc.BindableService;
import io.grpc.Server;
import io.grpc.ServerBuilder;
import io.grpc.ServerInterceptor;
import io.grpc.ServerTransportFilter;
import java.io.IOException;
import java.net.BindException;
import java.net.ServerSocket;
import java.util.ArrayList;
import java.util.List;

public class FakeServiceBuilder {
private final List<ServerInterceptor> interceptors = new ArrayList<>();
private final List<BindableService> services = new ArrayList<>();
private final List<ServerTransportFilter> transportFilters = new ArrayList<>();

public static FakeServiceBuilder create(BindableService... services) {
return new FakeServiceBuilder(services);
}

private FakeServiceBuilder(BindableService[] services) {
for (BindableService service : services) {
this.addService(service);
}
}

public FakeServiceBuilder intercept(ServerInterceptor interceptor) {
interceptors.add(interceptor);
return this;
}

public FakeServiceBuilder addService(BindableService service) {
services.add(service);
return this;
}

public FakeServiceBuilder addTransportFilter(ServerTransportFilter transportFilter) {
transportFilters.add(transportFilter);
return this;
}

public Server start() throws IOException {
IOException lastError = null;

for (int i = 0; i < 10; i++) {
try {
return startWithoutRetries();
} catch (IOException e) {
lastError = e;
if (!(e.getCause() instanceof BindException)) {
break;
}
}
}

throw lastError;
}

private Server startWithoutRetries() throws IOException {
int port;
try (ServerSocket ss = new ServerSocket(0)) {
port = ss.getLocalPort();
}
ServerBuilder<?> builder = ServerBuilder.forPort(port);
interceptors.forEach(builder::intercept);
services.forEach(builder::addService);
transportFilters.forEach(builder::addTransportFilter);

return builder.build().start();
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@
import com.google.bigtable.v2.ReadRowsResponse;
import com.google.bigtable.v2.RowFilter;
import com.google.bigtable.v2.RowSet;
import com.google.cloud.bigtable.data.v2.FakeServiceHelper;
import com.google.cloud.bigtable.data.v2.FakeServiceBuilder;
import com.google.common.collect.ImmutableList;
import io.grpc.ManagedChannel;
import io.grpc.ManagedChannelBuilder;
import io.grpc.Metadata;
import io.grpc.Server;
import io.grpc.ServerCall;
import io.grpc.ServerCall.Listener;
import io.grpc.ServerCallHandler;
Expand Down Expand Up @@ -60,15 +61,15 @@ public class BigtableChannelPrimerTest {
BigtableChannelPrimer primer;
ManagedChannel channel;
private LogHandler logHandler;
private FakeServiceHelper serviceHelper;
private Server server;

@Before
public void setup() throws IOException {
fakeService = new FakeService();
metadataInterceptor = new MetadataInterceptor();

serviceHelper = new FakeServiceHelper(metadataInterceptor, fakeService);
serviceHelper.start();
server = FakeServiceBuilder.create(fakeService).intercept(metadataInterceptor).start();

primer =
BigtableChannelPrimer.create(
OAuth2Credentials.create(new AccessToken(TOKEN_VALUE, null)),
Expand All @@ -78,9 +79,7 @@ public void setup() throws IOException {
ImmutableList.of("table1", "table2"));

channel =
ManagedChannelBuilder.forAddress("localhost", serviceHelper.getPort())
.usePlaintext()
.build();
ManagedChannelBuilder.forAddress("localhost", server.getPort()).usePlaintext().build();
logHandler = new LogHandler();
Logger.getLogger(BigtableChannelPrimer.class.toString()).addHandler(logHandler);
}
Expand All @@ -89,7 +88,7 @@ public void setup() throws IOException {
public void teardown() {
Logger.getLogger(BigtableChannelPrimer.class.toString()).removeHandler(logHandler);
channel.shutdown();
serviceHelper.shutdown();
server.shutdown();
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@
import com.google.bigtable.v2.ReadRowsRequest;
import com.google.bigtable.v2.ReadRowsResponse;
import com.google.cloud.bigtable.data.v2.BigtableDataSettings;
import com.google.cloud.bigtable.data.v2.FakeServiceHelper;
import com.google.cloud.bigtable.data.v2.FakeServiceBuilder;
import com.google.cloud.bigtable.data.v2.models.Query;
import com.google.cloud.bigtable.data.v2.models.Row;
import io.grpc.Server;
import io.grpc.Status;
import io.grpc.stub.StreamObserver;
import java.util.List;
Expand Down Expand Up @@ -54,7 +55,7 @@ public class EnhancedBigtableStubCloseRetryTest {
private BlockingQueue<ReadRowsRequest> requests;
private AtomicInteger numRequests;

private FakeServiceHelper serviceHelper;
private Server server;
private EnhancedBigtableStub stub;

@Before
Expand All @@ -63,11 +64,10 @@ public void setUp() throws Exception {
requests = new ArrayBlockingQueue<>(10);
numRequests = new AtomicInteger();

serviceHelper = new FakeServiceHelper(new FakeBigtable());
serviceHelper.start();
server = FakeServiceBuilder.create(new FakeBigtable()).start();

BigtableDataSettings.Builder settingBuilder =
BigtableDataSettings.newBuilderForEmulator(serviceHelper.getPort())
BigtableDataSettings.newBuilderForEmulator(server.getPort())
.setProjectId(PROJECT_ID)
.setInstanceId(INSTANCE_ID)
.setCredentialsProvider(NoCredentialsProvider.create())
Expand All @@ -80,7 +80,7 @@ public void setUp() throws Exception {
public void tearDown() throws Exception {
testExecutor.shutdown();
stub.close();
serviceHelper.shutdown();
server.shutdown();
}

@Test
Expand Down
Loading

0 comments on commit ae0d60d

Please sign in to comment.