From b726ef617fa05a285d018dd459572fff118a085a Mon Sep 17 00:00:00 2001
From: Nithin Nayak Sujir <nsujir@google.com>
Date: Fri, 14 Sep 2018 15:42:53 -0700
Subject: [PATCH 1/2] spanner: Expand test coverage for getDatabaseClient()

This change also adds the jacoco coverage plugin in the spanner pom.xml
and sets jacoco.skip to true to disable it by default. It can be enabled
by passing -Djacoco.skip=false to the mvn command.
---
 .../google-cloud-spanner/pom.xml              | 20 ++++++++
 .../google/cloud/spanner/SpannerImplTest.java | 48 ++++++++++++++++++-
 2 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/google-cloud-clients/google-cloud-spanner/pom.xml b/google-cloud-clients/google-cloud-spanner/pom.xml
index da79b31493e7..3e630d51ab3e 100644
--- a/google-cloud-clients/google-cloud-spanner/pom.xml
+++ b/google-cloud-clients/google-cloud-spanner/pom.xml
@@ -18,9 +18,29 @@
   </parent>
   <properties>
     <site.installationModule>google-cloud-spanner</site.installationModule>
+    <jacoco.skip>true</jacoco.skip>
   </properties>
   <build>
     <plugins>
+      <plugin>
+        <groupId>org.jacoco</groupId>
+        <artifactId>jacoco-maven-plugin</artifactId>
+        <version>0.8.2</version>
+        <executions>
+          <execution>
+            <goals>
+              <goal>prepare-agent</goal>
+            </goals>
+          </execution>
+          <execution>
+            <id>report</id>
+            <phase>prepare-package</phase>
+            <goals>
+              <goal>report</goal>
+            </goals>
+          </execution>
+        </executions>
+      </plugin>
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-surefire-plugin</artifactId>
diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java
index 71b0a1569a2d..4e5dce65cc62 100644
--- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java
+++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java
@@ -17,9 +17,10 @@
 package com.google.cloud.spanner;
 
 import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.fail;
 
+import com.google.cloud.grpc.GrpcTransportOptions;
 import com.google.cloud.spanner.spi.v1.SpannerRpc;
-
 import java.util.HashMap;
 import java.util.Map;
 import org.junit.Before;
@@ -70,4 +71,49 @@ public void createAndCloseSession() {
     // The same channelHint is passed for deleteSession (contained in "options").
     Mockito.verify(rpc).deleteSession(sessionName, options.getValue());
   }
+
+  @Test
+  public void getDbclientAgainGivesSame() {
+    Map<String, String> labels = new HashMap<>();
+    labels.put("env", "dev");
+    Mockito.when(spannerOptions.getSessionLabels()).thenReturn(labels);
+    String dbName = "projects/p1/instances/i1/databases/d1";
+    DatabaseId db = DatabaseId.of(dbName);
+
+    Mockito.when(spannerOptions.getTransportOptions())
+        .thenReturn(GrpcTransportOptions.newBuilder().build());
+    Mockito.when(spannerOptions.getSessionPoolOptions())
+        .thenReturn(SessionPoolOptions.newBuilder().build());
+
+    DatabaseClient databaseClient = impl.getDatabaseClient(db);
+
+    // Get db client again
+    DatabaseClient databaseClient1 = impl.getDatabaseClient(db);
+
+    assertThat(databaseClient == databaseClient1);
+  }
+
+  @Test
+  public void getDbclientAfterCloseThrows() {
+    SpannerImpl imp = new SpannerImpl(rpc, 1, spannerOptions);
+    Map<String, String> labels = new HashMap<>();
+    labels.put("env", "dev");
+    Mockito.when(spannerOptions.getSessionLabels()).thenReturn(labels);
+    String dbName = "projects/p1/instances/i1/databases/d1";
+    DatabaseId db = DatabaseId.of(dbName);
+
+    Mockito.when(spannerOptions.getTransportOptions())
+        .thenReturn(GrpcTransportOptions.newBuilder().build());
+    Mockito.when(spannerOptions.getSessionPoolOptions())
+        .thenReturn(SessionPoolOptions.newBuilder().build());
+
+    imp.close();
+
+    try {
+      imp.getDatabaseClient(db);
+      fail("Expected exception");
+    } catch (IllegalStateException e) {
+      assertThat(e.getMessage()).contains("Cloud Spanner client has been closed");
+    }
+  }
 }

From 33dabf335a2e8e59c360030092c871934ce24231 Mon Sep 17 00:00:00 2001
From: Nithin Nayak Sujir <nsujir@google.com>
Date: Mon, 17 Sep 2018 14:05:39 -0700
Subject: [PATCH 2/2] Replace == with isSameAs()

---
 .../src/test/java/com/google/cloud/spanner/SpannerImplTest.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java
index 4e5dce65cc62..fe81f34fd0f6 100644
--- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java
+++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java
@@ -90,7 +90,7 @@ public void getDbclientAgainGivesSame() {
     // Get db client again
     DatabaseClient databaseClient1 = impl.getDatabaseClient(db);
 
-    assertThat(databaseClient == databaseClient1);
+    assertThat(databaseClient1).isSameAs(databaseClient);
   }
 
   @Test