Skip to content
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.

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

Writing quickstart + test for Bigtable #1431

Merged
merged 17 commits into from
Jun 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .kokoro/tests/run_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ if [[ "$SCRIPT_DEBUG" != "true" ]]; then
source "${KOKORO_GFILE_DIR}/aws-secrets.sh"
source "${KOKORO_GFILE_DIR}/storage-hmac-credentials.sh"
source "${KOKORO_GFILE_DIR}/dlp_secrets.txt"
source "${KOKORO_GFILE_DIR}/bigtable_secrets.txt"
# Activate service account
gcloud auth activate-service-account \
--key-file="$GOOGLE_APPLICATION_CREDENTIALS" \
Expand Down
11 changes: 0 additions & 11 deletions bigtable/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -98,17 +98,6 @@
<artifactId>maven-project-info-reports-plugin</artifactId>
<version>3.0.0</version>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>3.0.0-M3</version>
<configuration>
<systemPropertyVariables>
<bigtable.project>java-docs-samples-testing</bigtable.project>
<bigtable.instance>instance</bigtable.instance>
</systemPropertyVariables>
</configuration>
</plugin>
</plugins>
</pluginManagement>
</build>
Expand Down
59 changes: 59 additions & 0 deletions bigtable/src/main/java/com/m/examples/bigtable/Quickstart.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* Copyright 2019 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
*
* http://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.m.examples.bigtable;

// START [bigtable_quickstart_veneer]

import com.google.api.gax.rpc.NotFoundException;
import com.google.cloud.bigtable.data.v2.BigtableDataClient;
import com.google.cloud.bigtable.data.v2.BigtableDataSettings;
import com.google.cloud.bigtable.data.v2.models.Row;
import com.google.cloud.bigtable.data.v2.models.RowCell;

public class Quickstart {

public static void quickstart(String projectId, String instanceId, String tableId) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please include a main()

// String projectId = "my-project-id";
// String instanceId = "my-instance-id";
// String tableId = "my-table-id";

BigtableDataSettings settings =
BigtableDataSettings.newBuilder().setProjectId(projectId).setInstanceId(instanceId).build();

// Initialize client that will be used to send requests. This client only needs to be created
// once, and can be reused for multiple requests. After completing all of your requests, call
// the "close" method on the client to safely clean up any remaining background resources.
try (BigtableDataClient dataClient = BigtableDataClient.create(settings)) {
try {
System.out.println("\nReading a single row by row key");
Row row = dataClient.readRow(tableId, "r1");
System.out.println("Row: " + row.getKey().toStringUtf8());
for (RowCell cell : row.getCells()) {
System.out.printf(
"Family: %s Qualifier: %s Value: %s%n",
cell.getFamily(), cell.getQualifier().toStringUtf8(), cell.getValue().toStringUtf8());
}
} catch (NotFoundException e) {
System.err.println("Failed to read from a non-existent table: " + e.getMessage());
}

} catch (Exception e) {
System.out.println("Error during functionName: \n" + e.toString());
Copy link
Contributor

Choose a reason for hiding this comment

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

Whats functionName?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, please use System.err

}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we collapse the try-catch blocks into one?

try (BigtableDataClient dataClient = BigtableDataClient.create(settings)) {
...
} catch (NotFoundException e) {
   System.err.println("Failed to read from a non-existent table: " + e.getMessage());
} catch (Exception e) {
   System.out.println("Error during functionName: \n" + e.toString());
 }

}
}
// END [bigtable_quickstart_veneer]
21 changes: 10 additions & 11 deletions bigtable/src/test/java/com/m/examples/bigtable/HelloWorldTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@
/** Integration tests for {@link HelloWorld} */
public class HelloWorldTest {

private static final String PROJECT_PROPERTY_NAME = "bigtable.project";
private static final String INSTANCE_PROPERTY_NAME = "bigtable.instance";
private static final String INSTANCE_PROPERTY_NAME = "BIGTABLE_TESTING_INSTANCE";
Copy link
Contributor

Choose a reason for hiding this comment

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

These shouldn't be called properties any more since its using env var now

private static final String TABLE_PREFIX = "table";
private static String tableId;
private static BigtableDataClient dataClient;
Expand All @@ -50,10 +49,17 @@ public class HelloWorldTest {
private static String instanceId;
private HelloWorld helloWorld;

private static String requireEnv(String varName) {
assertNotNull(
System.getenv(varName),
"System property '%s' is required to perform these tests.".format(varName));
return System.getenv(varName);
Copy link
Contributor

Choose a reason for hiding this comment

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

you can combine the return here:

return assertNotNull(
        System.getenv(varName),
        "System property '%s' is required to perform these tests.".format(varName));

Copy link
Member Author

Choose a reason for hiding this comment

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

assertNotNull doesn't return the value of System.getenv(varName)

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops, I thought you were using Preconditions.checkNotNull, which does. Any reason not to use that?

}

@BeforeClass
public static void beforeClass() throws IOException {
projectId = System.getProperty(PROJECT_PROPERTY_NAME);
instanceId = System.getProperty(INSTANCE_PROPERTY_NAME);
projectId = requireEnv("GOOGLE_CLOUD_PROJECT");
instanceId = requireEnv(INSTANCE_PROPERTY_NAME);
if (projectId == null || instanceId == null) {
dataClient = null;
adminClient = null;
Expand All @@ -79,13 +85,6 @@ public static void afterClass() throws Exception {

@Before
public void setup() throws IOException {
if (adminClient == null || dataClient == null) {
throw new AssumptionViolatedException(
PROJECT_PROPERTY_NAME
+ " or "
+ INSTANCE_PROPERTY_NAME
+ " property is not set, skipping integration tests.");
}
tableId = generateTableId();
helloWorld = new HelloWorld(projectId, instanceId, tableId);
adminClient.createTable(CreateTableRequest.of(tableId).addFamily("cf1"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
/** Integration tests for {@link InstanceAdminExample} */
public class InstanceAdminExampleTest {

private static final String PROJECT_PROPERTY_NAME = "bigtable.project";
private static final String ID_PREFIX = "instanceadmin";
private static final String CLUSTER = "cluster";
private static String projectId;
Expand All @@ -51,9 +50,16 @@ public class InstanceAdminExampleTest {
private String instanceId;
private InstanceAdminExample instanceAdmin;

private static String requireEnv(String varName) {
assertNotNull(
System.getenv(varName),
"System property '%s' is required to perform these tests.".format(varName));
return System.getenv(varName);
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

}

@BeforeClass
public static void beforeClass() throws IOException {
projectId = System.getProperty(PROJECT_PROPERTY_NAME);
projectId = requireEnv("GOOGLE_CLOUD_PROJECT");
if (projectId == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

requireEnv should throw an error if projectID is null, so you can remove this check

adminClient = null;
return;
Expand All @@ -71,10 +77,6 @@ public static void afterClass() {

@Before
public void setup() throws IOException {
if (adminClient == null) {
throw new AssumptionViolatedException(
PROJECT_PROPERTY_NAME + " property is not set, skipping integration tests.");
}
instanceId = generateId();
clusterId = generateId();
instanceAdmin = new InstanceAdminExample(projectId, instanceId, clusterId);
Expand Down
82 changes: 82 additions & 0 deletions bigtable/src/test/java/com/m/examples/bigtable/QuickstartTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
* Copyright 2019 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
*
* http://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.m.examples.bigtable;

import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThat;

import com.google.cloud.bigtable.admin.v2.BigtableTableAdminClient;
import com.google.cloud.bigtable.admin.v2.BigtableTableAdminSettings;
import com.google.cloud.bigtable.data.v2.BigtableDataClient;
import com.google.cloud.bigtable.data.v2.BigtableDataSettings;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.PrintStream;
import org.hamcrest.CoreMatchers;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.AssumptionViolatedException;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;

/**
* Integration tests for {@link Quickstart}
*/
public class QuickstartTest {

private static final String INSTANCE_PROPERTY_NAME = "BIGTABLE_TESTING_INSTANCE";
private static final String TABLE_ID = "quickstart-table";
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't hardcode the table id, it will cause these tests to be flaky when multiple PRs are opened

private static String projectId;
private static String instanceId;
private ByteArrayOutputStream bout;

private static String requireEnv(String varName) {
assertNotNull(
System.getenv(varName),
"System property '%s' is required to perform these tests.".format(varName));
return System.getenv(varName);
}

@BeforeClass
public static void beforeClass() {
projectId = requireEnv("GOOGLE_CLOUD_PROJECT");
instanceId = requireEnv(INSTANCE_PROPERTY_NAME);
}

@Before
public void setupStream() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use the junit convention of calling this setUp()

bout = new ByteArrayOutputStream();
System.setOut(new PrintStream(bout));
}

@Test
public void testQuickstart() {
try {
Quickstart.quickstart(projectId, instanceId, TABLE_ID);
} catch (Exception e) {
System.out.println("Failed to run quickstart.");
System.out.println(e);
}

String output = bout.toString();
assertThat(output, CoreMatchers.containsString("Reading a single row by row key"));
assertThat(output, CoreMatchers.containsString("Row: r1"));
assertThat(
output, CoreMatchers.containsString("Family: cf1 Qualifier: c1 Value: quickstart"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import static com.google.cloud.bigtable.admin.v2.models.GCRules.GCRULES;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;

import com.google.cloud.bigtable.admin.v2.BigtableTableAdminClient;
Expand Down Expand Up @@ -45,19 +46,25 @@
/** Integration tests for {@link TableAdminExample} */
public class TableAdminExampleTest {

private static final String PROJECT_PROPERTY_NAME = "bigtable.project";
private static final String INSTANCE_PROPERTY_NAME = "bigtable.instance";
private static final String INSTANCE_PROPERTY_NAME = "BIGTABLE_TESTING_INSTANCE";
private static final String TABLE_PREFIX = "table";
private static BigtableTableAdminClient adminClient;
private static String instanceId;
private static String projectId;
private String tableId;
private TableAdminExample tableAdmin;

private static String requireEnv(String varName) {
assertNotNull(
System.getenv(varName),
"System property '%s' is required to perform these tests.".format(varName));
return System.getenv(varName);
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

}

@BeforeClass
public static void beforeClass() throws IOException {
projectId = System.getProperty(PROJECT_PROPERTY_NAME);
instanceId = System.getProperty(INSTANCE_PROPERTY_NAME);
projectId = requireEnv("GOOGLE_CLOUD_PROJECT");
instanceId = requireEnv(INSTANCE_PROPERTY_NAME);
if (projectId == null || instanceId == null) {
adminClient = null;
return;
Expand All @@ -78,13 +85,6 @@ public static void afterClass() {

@Before
public void setup() throws IOException {
if (adminClient == null) {
throw new AssumptionViolatedException(
INSTANCE_PROPERTY_NAME
+ " or "
+ PROJECT_PROPERTY_NAME
+ " property is not set, skipping integration tests.");
}
tableId = generateTableId();
tableAdmin = new TableAdminExample(projectId, instanceId, tableId);
adminClient.createTable(CreateTableRequest.of(tableId).addFamily("cf"));
Expand Down