Skip to content
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
23 changes: 15 additions & 8 deletions java-bigquery-jdbc/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ PACKAGE_DESTINATION=$(PWD)/drivers
SRC="$(PWD)"
skipSurefire ?= true
skipShade ?= true
excludedGroups ?=
comma := ,
empty :=
space := $(empty) $(empty)
Comment on lines +9 to +12

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.

high

Passing an empty -DexcludedGroups="" to Maven Surefire/Failsafe when excludedGroups is not specified (which is the default) can cause JUnit 5 to throw an IllegalArgumentException (e.g., Tag name must not be null or empty or Invalid tag expression).

To prevent this and ensure the default build runs successfully, we can conditionally define a helper variable EXCLUDED_GROUPS_PROP in the Makefile using GNU Make's $(if ...) function, and only pass the -DexcludedGroups property to Maven when excludedGroups is actually set.

excludedGroups ?=
EXCLUDED_GROUPS_PROP = $(if $(excludedGroups),-DexcludedGroups="$(excludedGroups)",)
comma := ,
empty :=
space := $(empty) $(empty)
References
  1. When replacing a hardcoded default environment variable with a Makefile variable, verify whether an empty or unset value will break the execution, or confirm that the application handles an empty string identically to the default value.

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.

Verified that it works

JDBC_DRIVER_VERSION = $(shell mvn help:evaluate -Dexpression=project.version -q -DforceStdout)
JDBC_JAR = $(PACKAGE_DESTINATION)/google-cloud-bigquery-jdbc-$(JDBC_DRIVER_VERSION)-all.jar

Expand Down Expand Up @@ -33,10 +37,12 @@ unittest: |
-Dclirr.skip=true \
-Denforcer.skip=true \
-Dtest=$(test) \
-DexcludedGroups="$(excludedGroups)" \
Comment thread
logachev marked this conversation as resolved.
test

# Important: By default, this command will skip unittests & uberjar build.
# To include unit tests, run: make integration-test skipSurefire=false
# To exclude test tags/groups, run: make integration-test excludedGroups="known_issues"
integration-test:
mvn -B -ntp \
-Penable-integration-tests \
Expand All @@ -47,17 +53,18 @@ integration-test:
-Denforcer.skip=true \
-Dit.failIfNoSpecifiedTests=true \
-Dit.test=$(test) \
-DexcludedGroups="$(excludedGroups)" \
Comment thread
logachev marked this conversation as resolved.
integration-test \
verify

unit-test-coverage:
$(MAKE) unittest
$(MAKE) unittest excludedGroups="$(excludedGroups)"
mvn -B -ntp jacoco:report
BUILD_DIR=$$(mvn -B -ntp help:evaluate -Dexpression=project.build.directory -q -DforceStdout); \
cd $$BUILD_DIR/site && zip -r $$OLDPWD/jacoco-unittests.zip jacoco && cd $$OLDPWD

full-coverage:
$(MAKE) integration-test skipSurefire=false test=ITBigQueryJDBCTest,ITNightlyBigQueryTest
$(MAKE) integration-test skipSurefire=false test=ITBigQueryJDBCTest,ITNightlyBigQueryTest excludedGroups="$(excludedGroups)"
mvn -B -ntp jacoco:report
BUILD_DIR=$$(mvn -B -ntp help:evaluate -Dexpression=project.build.directory -q -DforceStdout); \
cd $$BUILD_DIR/site && zip -r $$OLDPWD/jacoco-full.zip jacoco && cd $$OLDPWD
Expand All @@ -70,10 +77,10 @@ package:
cp target/google-cloud-bigquery-jdbc-*-all.jar $(PACKAGE_DESTINATION)/

build-it-standalone:
mvn -Dmaven.test.skip=true package -f pom-it.xml -Dbigquery-jdbc.version=$(JDBC_DRIVER_VERSION)
mvn clean package -f pom-it.xml -Dmaven.test.skip=true -Dbigquery-jdbc.version=$(JDBC_DRIVER_VERSION)

run-it-standalone:
java -cp $(JDBC_JAR):target-it/* org.junit.platform.console.ConsoleLauncher --select-class com.google.cloud.bigquery.jdbc.it.suites.ITDriverAgnosticTests
java -cp "$(JDBC_JAR):target-it/*" org.junit.platform.console.ConsoleLauncher --select-class com.google.cloud.bigquery.jdbc.it.suites.ITDriverAgnosticTests $(foreach tag,$(subst $(comma),$(space),$(excludedGroups)),--exclude-tag $(tag))


# Commands for dockerized environments
Expand Down Expand Up @@ -143,14 +150,14 @@ docker-package: docker-build
cp --no-preserve=ownership /mvn/test-target/google-cloud-bigquery-jdbc-*-all.jar /pkg "

docker-unittest: |
$(MAKE) .docker-run args="make unittest test=$(test)"
$(MAKE) .docker-run args="make unittest test=$(test) excludedGroups='$(excludedGroups)'"

docker-integration-test: .check-env
$(MAKE) .docker-run args="make integration-test test=$(test) skipSurefire=$(skipSurefire)"
$(MAKE) .docker-run args="make integration-test test=$(test) skipSurefire=$(skipSurefire) excludedGroups='$(excludedGroups)'"

docker-proxy-integration-test: .check-env docker-proxy-build
$(MAKE) docker-integration-test CONTAINER_NAME=$(PROXY_CONTAINER_NAME) BIGQUERY_URL_FLAGS="ProxyHost=127.0.0.1;ProxyPort=3128;"

docker-coverage:
$(MAKE) .docker-run args="make unit-test-coverage"
$(MAKE) .docker-run args="make full-coverage"
$(MAKE) .docker-run args="make unit-test-coverage excludedGroups='$(excludedGroups)'"
$(MAKE) .docker-run args="make full-coverage excludedGroups='$(excludedGroups)'"
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import java.util.Arrays;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
Expand Down Expand Up @@ -110,6 +111,7 @@ public void testValidServiceAccountAuthenticationOAuthPvtKeyAsPath()
}

@Test
@Tag("advanced")
public void testValidServiceAccountAuthenticationViaEmailAndPkcs8Key()
throws SQLException, IOException {
final JsonObject authJson = getAuthJson();
Expand Down Expand Up @@ -272,6 +274,7 @@ public void testValidExternalAccountAuthenticationRawJson() throws SQLException
"https://www.googleapis.com/auth/bigquery.readonly, true",
"https://www.googleapis.com/auth/bigquery, false"
})
@Tag("advanced")
public void testValidPreGeneratedAccessTokenAuthentication(String scope, boolean isReadOnly)
throws Exception {
final JsonObject authJson = getAuthJson();
Expand Down Expand Up @@ -335,6 +338,7 @@ public void testValidApplicationDefaultCredentialsAuthentication() throws SQLExc
// It requires account to have 'tokenCreator' permission, see
// https://cloud.google.com/docs/authentication/use-service-account-impersonation#required-roles
@Test
@Tag("advanced")
public void testServiceAccountAuthenticationWithImpersonation() throws IOException, SQLException {
final JsonObject authJson = getAuthJson();

Expand All @@ -350,6 +354,7 @@ public void testServiceAccountAuthenticationWithImpersonation() throws IOExcepti
// This test uses the same client email for the main authorization and a chain of impersonations.
// It requires the account to have 'tokenCreator' permission on itself.
@Test
@Tag("advanced")
public void testServiceAccountAuthenticationWithChainedImpersonation()
throws IOException, SQLException {
final JsonObject authJson = getAuthJson();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;

public class ITBigQueryJDBCTest extends ITBase {
Expand Down Expand Up @@ -101,6 +102,7 @@ public static void afterClass() throws SQLException {
}

@Test
@Tag("known_issue") // b/539615199
public void testValidAllDataTypesSerializationFromSelectQueryArrowDataset() throws SQLException {
String DATASET = "JDBC_INTEGRATION_DATASET";
String TABLE_NAME = "JDBC_INTEGRATION_ARROW_TEST_TABLE";
Expand Down Expand Up @@ -2488,6 +2490,7 @@ private void validate(
}

@Test
@Tag("known_issue") // b/539615199
public void validateGetString() throws Exception {
final ImmutableMap<String, Object> stringResults =
new ImmutableMap.Builder<String, Object>()
Expand Down Expand Up @@ -2660,6 +2663,7 @@ public void validateGetShort() throws Exception {
}

@Test
@Tag("known_issue") // b/539615199
public void validateGetTime() throws Exception {
final ImmutableMap<String, Object> result =
new ImmutableMap.Builder<String, Object>()
Expand Down Expand Up @@ -2698,6 +2702,7 @@ public void validateGetDate() throws Exception {
}

@Test
@Tag("known_issue") // b/539615199
public void validateGetTimestamp() throws Exception {
final ImmutableMap<String, Object> result =
new ImmutableMap.Builder<String, Object>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;

public class ITConnectionTest {
Expand All @@ -61,6 +62,7 @@ public static void afterClass() throws InterruptedException {
}

@Test
@Tag("advanced")
public void testGetMetaData() throws SQLException {
Connection connection = DriverManager.getConnection(ITBase.connectionUrl);
assertFalse(connection.isClosed());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;

public class ITDatabaseMetadataTest extends ITBase {
Expand Down Expand Up @@ -272,6 +273,7 @@ public void testGetPrimaryKeys() throws SQLException {
}

@Test
@Tag("advanced")
public void testTableConstraints() throws SQLException {
Connection connection = DriverManager.getConnection(ITBase.connectionUrl);
ResultSet primaryKey1 =
Expand Down Expand Up @@ -390,6 +392,7 @@ public void testGetExportedKeys_noKeys() throws SQLException {
}

@Test
@Tag("advanced")
public void testMetadataResultSetsDoNotInterfere() throws SQLException {
try (Connection connection = DriverManager.getConnection(ITBase.connectionUrl)) {
DatabaseMetaData metaData = connection.getMetaData();
Expand Down Expand Up @@ -445,6 +448,7 @@ public void testDatabaseMetadataGetCatalogs() throws SQLException {
}

@Test
@Tag("advanced")
public void testDatabaseMetadataGetProcedures() throws SQLException {

Connection connection = DriverManager.getConnection(ITBase.connectionUrl);
Expand Down Expand Up @@ -1188,6 +1192,7 @@ public void testAdditionalProjectsInMetadata() throws SQLException {
}

@Test
@Tag("advanced")
public void testFilterTablesOnDefaultDataset_getTables() throws SQLException {

String defaultDatasetValue = CONSTRAINTS_DATASET;
Expand Down Expand Up @@ -1258,6 +1263,7 @@ public void testFilterTablesOnDefaultDataset_getTables() throws SQLException {
}

@Test
@Tag("advanced")
public void testFilterTablesOnDefaultDataset_getColumns() throws SQLException {
String defaultDatasetValue = CONSTRAINTS_DATASET;
String tableInDefaultDataset = CONSTRAINTS_TABLE_NAME;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import java.util.concurrent.atomic.AtomicBoolean;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;

public class ITNightlyBigQueryTest extends ITBase {
Expand Down Expand Up @@ -1038,6 +1039,7 @@ public void testMultipleExecuteBatches() throws SQLException {
}

@Test
@Tag("known_issue") // b/539615199
public void testValidAllDataTypesSerializationFromSelectQuery() throws SQLException {
String DATASET = "JDBC_INTEGRATION_DATASET";
String TABLE_NAME = "JDBC_DATATYPES_INTEGRATION_TEST_TABLE";
Expand Down
Loading