From 4d5aa595d6627b8a58032a46aab61db72a4fda2e Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Mon, 15 Jul 2019 11:31:37 -0400 Subject: [PATCH 1/4] Add debugging information for Firestore integration tests Add an assertion to verify the expected number of events are delivered to the queryWatch test. Awaiting the semaphore now has a 60sec timeout. If not satisfied within that timespan the test will fail, and provide an assertion message of the progress. Updates to allow running builds locally * Update build script to cd to checkout directory relative to itself instead of using a hard-coded path. * Update build script to be tolerant of absolute paths specified for GOOGLE_APPLICATION_CREDENTIALS --- .kokoro/build.sh | 7 +++-- .../cloud/firestore/it/ITSystemTest.java | 30 ++++++++++++++++++- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/.kokoro/build.sh b/.kokoro/build.sh index f3bc0b7542a6..86fe252267e4 100755 --- a/.kokoro/build.sh +++ b/.kokoro/build.sh @@ -15,7 +15,10 @@ set -eo pipefail -cd github/google-cloud-java/ +## Get the directory of the build script +scriptDir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" +## cd to the parent directory, i.e. the root of the git repo +cd ${scriptDir}/.. function client_has_changes() { CLIENT_NAME=$1 @@ -58,7 +61,7 @@ mvn install -B -V \ -T 1C # prepend Kokoro root directory onto GOOGLE_APPLICATION_CREDENTIALS path -if [[ ! -z "${GOOGLE_APPLICATION_CREDENTIALS}" ]]; then +if [[ ! -z "${GOOGLE_APPLICATION_CREDENTIALS}" && "${GOOGLE_APPLICATION_CREDENTIALS}" != /* ]]; then export GOOGLE_APPLICATION_CREDENTIALS=$(realpath ${KOKORO_ROOT}/src/${GOOGLE_APPLICATION_CREDENTIALS}) fi diff --git a/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java b/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java index 20f720014437..84d25e5bdc4a 100644 --- a/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java +++ b/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java @@ -17,6 +17,7 @@ package com.google.cloud.firestore.it; import static com.google.cloud.firestore.LocalFirestoreHelper.map; +import static com.google.common.collect.Sets.newHashSet; import static java.util.Arrays.asList; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -52,7 +53,9 @@ import com.google.cloud.firestore.Transaction.Function; import com.google.cloud.firestore.WriteBatch; import com.google.cloud.firestore.WriteResult; +import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Sets; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -60,9 +63,12 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.TreeSet; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import javax.annotation.Nullable; import org.junit.After; @@ -886,6 +892,10 @@ public void queryWatch() throws Exception { final Semaphore semaphore = new Semaphore(0); ListenerRegistration registration = null; + final Set expectedOperations = + Sets.newTreeSet(newHashSet("case 0", "case 1", "case 2", "case 3", "case 4", "case 5")); + final Set actualOperations = Collections.synchronizedSet(new TreeSet()); + try { registration = randomColl @@ -898,26 +908,31 @@ public void queryWatch() throws Exception { @Override public void onEvent( @Nullable QuerySnapshot value, @Nullable FirestoreException error) { + System.out.printf("onEvent(value : %s, error : %s)%n", value, error); try { switch (semaphore.availablePermits()) { case 0: + actualOperations.add("case 0"); assertTrue(value.isEmpty()); ref1 = randomColl.add(map("foo", "foo")).get(); ref2 = randomColl.add(map("foo", "bar")).get(); break; case 1: + actualOperations.add("case 1"); assertEquals(1, value.size()); assertEquals(1, value.getDocumentChanges().size()); assertEquals(Type.ADDED, value.getDocumentChanges().get(0).getType()); ref1.set(map("foo", "bar")); break; case 2: + actualOperations.add("case 2"); assertEquals(2, value.size()); assertEquals(1, value.getDocumentChanges().size()); assertEquals(Type.ADDED, value.getDocumentChanges().get(0).getType()); ref1.set(map("foo", "bar", "bar", " foo")); break; case 3: + actualOperations.add("case 3"); assertEquals(2, value.size()); assertEquals(1, value.getDocumentChanges().size()); assertEquals( @@ -925,12 +940,14 @@ public void onEvent( ref2.set(map("foo", "foo")); break; case 4: + actualOperations.add("case 4"); assertEquals(1, value.size()); assertEquals(1, value.getDocumentChanges().size()); assertEquals(Type.REMOVED, value.getDocumentChanges().get(0).getType()); ref1.delete(); break; case 5: + actualOperations.add("case 5"); assertTrue(value.isEmpty()); assertEquals(1, value.getDocumentChanges().size()); assertEquals(Type.REMOVED, value.getDocumentChanges().get(0).getType()); @@ -943,7 +960,18 @@ public void onEvent( } }); - semaphore.acquire(6); + final boolean tryAcquire = semaphore.tryAcquire(6, 60, TimeUnit.SECONDS); + + final Joiner j = Joiner.on(", "); + final String expectedString = j.join(expectedOperations); + final String actualString = j.join(actualOperations); + assertTrue( + String.format( + "did not receive all expected messages within the deadline.%n" + + "expectedOperations = [%s]%n" + + " actualOperations = [%s]%n", + expectedString, actualString), + tryAcquire); } finally { if (registration != null) { registration.remove(); From ab091ea35700c4c72bb0431d26e72e9fd46e7cc6 Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Thu, 18 Jul 2019 15:24:33 -0400 Subject: [PATCH 2/4] Update error message to be more explicit --- .../cloud/firestore/it/ITSystemTest.java | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java b/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java index 84d25e5bdc4a..0e24bce25b34 100644 --- a/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java +++ b/google-cloud-clients/google-cloud-firestore/src/test/java/com/google/cloud/firestore/it/ITSystemTest.java @@ -892,9 +892,10 @@ public void queryWatch() throws Exception { final Semaphore semaphore = new Semaphore(0); ListenerRegistration registration = null; - final Set expectedOperations = - Sets.newTreeSet(newHashSet("case 0", "case 1", "case 2", "case 3", "case 4", "case 5")); - final Set actualOperations = Collections.synchronizedSet(new TreeSet()); + final Set expectedEvents = + Sets.newTreeSet( + newHashSet("event 0", "event 1", "event 2", "event 3", "event 4", "event 5")); + final Set actualEvents = Collections.synchronizedSet(new TreeSet()); try { registration = @@ -912,27 +913,27 @@ public void onEvent( try { switch (semaphore.availablePermits()) { case 0: - actualOperations.add("case 0"); + actualEvents.add("event 0"); assertTrue(value.isEmpty()); ref1 = randomColl.add(map("foo", "foo")).get(); ref2 = randomColl.add(map("foo", "bar")).get(); break; case 1: - actualOperations.add("case 1"); + actualEvents.add("event 1"); assertEquals(1, value.size()); assertEquals(1, value.getDocumentChanges().size()); assertEquals(Type.ADDED, value.getDocumentChanges().get(0).getType()); ref1.set(map("foo", "bar")); break; case 2: - actualOperations.add("case 2"); + actualEvents.add("event 2"); assertEquals(2, value.size()); assertEquals(1, value.getDocumentChanges().size()); assertEquals(Type.ADDED, value.getDocumentChanges().get(0).getType()); ref1.set(map("foo", "bar", "bar", " foo")); break; case 3: - actualOperations.add("case 3"); + actualEvents.add("event 3"); assertEquals(2, value.size()); assertEquals(1, value.getDocumentChanges().size()); assertEquals( @@ -940,14 +941,14 @@ public void onEvent( ref2.set(map("foo", "foo")); break; case 4: - actualOperations.add("case 4"); + actualEvents.add("event 4"); assertEquals(1, value.size()); assertEquals(1, value.getDocumentChanges().size()); assertEquals(Type.REMOVED, value.getDocumentChanges().get(0).getType()); ref1.delete(); break; case 5: - actualOperations.add("case 5"); + actualEvents.add("event 5"); assertTrue(value.isEmpty()); assertEquals(1, value.getDocumentChanges().size()); assertEquals(Type.REMOVED, value.getDocumentChanges().get(0).getType()); @@ -963,13 +964,13 @@ public void onEvent( final boolean tryAcquire = semaphore.tryAcquire(6, 60, TimeUnit.SECONDS); final Joiner j = Joiner.on(", "); - final String expectedString = j.join(expectedOperations); - final String actualString = j.join(actualOperations); + final String expectedString = j.join(expectedEvents); + final String actualString = j.join(actualEvents); assertTrue( String.format( - "did not receive all expected messages within the deadline.%n" - + "expectedOperations = [%s]%n" - + " actualOperations = [%s]%n", + "did not receive all expected events within the deadline.%n" + + "expectedEvents = [%s]%n" + + " actualEvents = [%s]%n", expectedString, actualString), tryAcquire); } finally { From 498c7af6331042f9fb26a51dc49f774de48b55ca Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Mon, 22 Jul 2019 16:44:35 -0400 Subject: [PATCH 3/4] use realpath instead of cd && pwd Co-Authored-By: Jeff Ching --- .kokoro/build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.kokoro/build.sh b/.kokoro/build.sh index 86fe252267e4..319242755de8 100755 --- a/.kokoro/build.sh +++ b/.kokoro/build.sh @@ -16,7 +16,7 @@ set -eo pipefail ## Get the directory of the build script -scriptDir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" +scriptDir=$(realpath $(dirname "${BASH_SOURCE[0]}")) ## cd to the parent directory, i.e. the root of the git repo cd ${scriptDir}/.. From ac67b11a016b995b82d1b2f6c71454586d20a63b Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Mon, 22 Jul 2019 16:55:15 -0400 Subject: [PATCH 4/4] Update comment --- .kokoro/build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.kokoro/build.sh b/.kokoro/build.sh index 319242755de8..07007c9e5baa 100755 --- a/.kokoro/build.sh +++ b/.kokoro/build.sh @@ -60,7 +60,7 @@ mvn install -B -V \ -Dgcloud.download.skip=true \ -T 1C -# prepend Kokoro root directory onto GOOGLE_APPLICATION_CREDENTIALS path +# if GOOGLE_APPLICATION_CREDIENTIALS is specified as a relative path prepend Kokoro root directory onto it if [[ ! -z "${GOOGLE_APPLICATION_CREDENTIALS}" && "${GOOGLE_APPLICATION_CREDENTIALS}" != /* ]]; then export GOOGLE_APPLICATION_CREDENTIALS=$(realpath ${KOKORO_ROOT}/src/${GOOGLE_APPLICATION_CREDENTIALS}) fi