Skip to content

Commit f2e1205

Browse files
devjgmdevjgm
authored
cleanup: make the test labels/tags more consistent (#4257)
Fixes #3868 This PR attempts to make our use of labels (cmake) and tags (bazel) a bit simpler and more consistent. This might be a small step in that direction: Use the singular term "integration-test" everywhere Removed all $product-integration-test labels. These are no longer needed. Products are identified in ctest invocations using -R $product, and in bazel invocations using bazel test google/cloud/$product/.... All integration tests in bazel and cmake have an integration-test label. This is most helpful when excluding all integration tests from a test run. [CMAKE ONLY] All integration tests also have a label of either integration-test-emulator or integration-test-production, to identify whether or not they should run w/ an emulator. (there's one documented exception here about a bigquery integration test that just doesn't fully work today because it needs a script to invoke it w/ some arguments, and that's more than we need to do today). That last bullet point suggests something odd: The bazel and cmake labels are not in sync. In bazel we have no indication of whether integration tests use an emulator or not. Instead, when we run integration tests in bazel, we manually pick and choose which tests do what (see build-in-docker-bazel.sh). It would be nice to clean that up in another PR.
1 parent 094a134 commit f2e1205

36 files changed

Lines changed: 75 additions & 77 deletions

‎ci/kokoro/docker/build-in-docker-bazel.sh‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ echo "================================================================"
6161
io::log "Compiling and running unit tests"
6262
echo "================================================================"
6363
"${BAZEL_BIN}" test \
64-
"${bazel_args[@]}" "--test_tag_filters=-integration-tests" \
64+
"${bazel_args[@]}" "--test_tag_filters=-integration-test" \
6565
-- //google/cloud/...:all
6666

6767
echo "================================================================"
@@ -173,7 +173,13 @@ if should_run_integration_tests; then
173173
access_token_targets=(
174174
"//google/cloud/bigtable/examples:bigtable_grpc_credentials"
175175
)
176-
excluded_targets=()
176+
excluded_targets=(
177+
# The Bigtable integrations that use the emulator *and* production were
178+
# already run by the "run_integration_tests_emulator_bazel.sh" script that
179+
# was called above. The one exception is the bigtable_grpc_credentials
180+
# test, which requires an access token and will be run separately.
181+
"-//google/cloud/bigtable/..."
182+
)
177183
for t in "${hmac_service_account_targets[@]}" "${access_token_targets[@]}"; do
178184
excluded_targets+=("-${t}")
179185
done
@@ -183,7 +189,7 @@ if should_run_integration_tests; then
183189
# below to avoid invalidating the cached test results for all the other tests.
184190
"${BAZEL_BIN}" test \
185191
"${bazel_args[@]}" \
186-
"--test_tag_filters=storage-integration-tests,pubsub-integration-tests,spanner-integration-tests" \
192+
"--test_tag_filters=integration-test" \
187193
-- //google/cloud/...:all "${excluded_targets[@]}"
188194

189195
# Changing the PATH disables the Bazel cache, so use an absolute path.

‎ci/kokoro/docker/build-in-docker-cmake.sh‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ if [[ "${BUILD_TESTING:-}" = "yes" ]]; then
187187
echo
188188
io::log_yellow "Running unit tests"
189189
echo
190-
(cd "${BINARY_DIR}" && ctest "-LE" "integration-tests" "${ctest_args[@]}")
190+
(cd "${BINARY_DIR}" && ctest "-LE" "integration-test" "${ctest_args[@]}")
191191
echo
192192
io::log_yellow "Completed unit tests"
193193
echo
@@ -209,7 +209,7 @@ if [[ "${BUILD_TESTING:-}" = "yes" ]]; then
209209
# TODO(#441) - when the emulator crashes the tests can take a long time.
210210
# The slowest test normally finishes in about 6 seconds, 60 seems safe.
211211
if "${PROJECT_ROOT}/google/cloud/bigtable/ci/${EMULATOR_SCRIPT}" \
212-
"${BINARY_DIR}" "${ctest_args[@]}" --timeout 60; then
212+
"${BINARY_DIR}" "${ctest_args[@]}" -L integration-test-emulator --timeout 60; then
213213
success=yes
214214
break
215215
fi
@@ -224,13 +224,13 @@ if [[ "${BUILD_TESTING:-}" = "yes" ]]; then
224224
io::log_yellow "running storage integration tests via CTest+Emulator"
225225
echo
226226
"${PROJECT_ROOT}/google/cloud/storage/ci/${EMULATOR_SCRIPT}" \
227-
"${BINARY_DIR}" "${ctest_args[@]}"
227+
"${BINARY_DIR}" "${ctest_args[@]}" -L integration-test-emulator
228228

229229
echo
230230
io::log_yellow "running spanner integration tests via CTest+Emulator"
231231
echo
232232
"${PROJECT_ROOT}/google/cloud/spanner/ci/${EMULATOR_SCRIPT}" \
233-
"${BINARY_DIR}" "${ctest_args[@]}"
233+
"${BINARY_DIR}" "${ctest_args[@]}" -L integration-test-emulator
234234
fi
235235

236236
readonly GOOGLE_CLOUD_CPP_STORAGE_TEST_KEY_FILE_JSON="/c/kokoro-run-key.json"
@@ -304,16 +304,16 @@ if [[ "${BUILD_TESTING:-}" = "yes" ]]; then
304304

305305
# Since we already run multiple integration tests against the emulator we
306306
# only need to run the tests here that cannot use the emulator. Some
307-
# libraries will tag all their tests as "integration-tests-no-emulator",
307+
# libraries will tag all their tests as "integration-test-production",
308308
# that is fine too. As long as we do not repeat all the tests we are
309309
# winning.
310310
if [[ "${BUILD_NAME:-}" != "coverage" ]]; then
311311
# TODO(#4234) - the Bigtable tests are only enabled on the coverage
312312
# builds because they consume too much quota.
313-
ctest_args+=(-E bigtable)
313+
ctest_args+=(-E "^bigtable_")
314314
fi
315-
env -C "${BINARY_DIR}" ctest \
316-
-L integration-tests-no-emulator "${ctest_args[@]}"
315+
env -C "${BINARY_DIR}" ctest "${ctest_args[@]}" \
316+
-L integration-test-production
317317

318318
echo "================================================================"
319319
io::log_yellow "Completed the integration tests against production"

‎ci/kokoro/macos/build-bazel.sh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ echo
6969
echo "================================================================"
7070
io::log_yellow "build and run unit tests."
7171
"${BAZEL_BIN}" test \
72-
"${bazel_args[@]}" "--test_tag_filters=-integration-tests" \
72+
"${bazel_args[@]}" "--test_tag_filters=-integration-test" \
7373
-- //google/cloud/...:all
7474

7575
echo
@@ -130,7 +130,7 @@ if should_run_integration_tests; then
130130

131131
"${BAZEL_BIN}" test \
132132
"${bazel_args[@]}" \
133-
"--test_tag_filters=bigtable-integration-tests,storage-integration-tests,spanner-integration-tests" \
133+
"--test_tag_filters=integration-test" \
134134
-- //google/cloud/...:all \
135135
-//google/cloud/bigtable/examples:bigtable_grpc_credentials \
136136
-//google/cloud/storage/examples:storage_service_account_samples \

‎ci/kokoro/macos/build-cmake.sh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ if [[ -r "${BINARY_DIR}/CTestTestfile.cmake" ]]; then
7070
(
7171
cd "${BINARY_DIR}"
7272
ctest \
73-
-LE integration-tests \
73+
-LE integration-test \
7474
--output-on-failure -j "${NCPU}"
7575
)
7676
echo "================================================================"
@@ -106,7 +106,7 @@ if should_run_integration_tests; then
106106
cd "${BINARY_DIR}"
107107
fi
108108
ctest \
109-
-L '(bigtable-integration-tests|storage-integration-tests|spanner-integration-tests|integration-tests-no-emulator)' \
109+
-L 'integration-test-production' \
110110
-E '(bigtable_grpc_credentials|storage_service_account_samples|service_account_integration_test)' \
111111
--output-on-failure -j "${NCPU}"
112112
)

‎ci/kokoro/windows/build-bazel.ps1‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ ForEach($_ in (1, 2, 3)) {
120120

121121
Write-Host -ForegroundColor Yellow "`n$(Get-Date -Format o) Compiling and running unit tests"
122122
bazel $common_flags test $test_flags `
123-
--test_tag_filters=-integration-tests `
123+
--test_tag_filters=-integration-test `
124124
-- //google/cloud/...:all
125125
if ($LastExitCode) {
126126
Write-Host -ForegroundColor Red "bazel test failed with exit code ${LastExitCode}."
@@ -188,7 +188,7 @@ if (Integration-Tests-Enabled) {
188188
"--test_env=GOOGLE_CLOUD_CPP_SPANNER_TEST_SERVICE_ACCOUNT=${env:GOOGLE_CLOUD_CPP_SPANNER_TEST_SERVICE_ACCOUNT}"
189189
)
190190
bazel $common_flags test $test_flags $integration_flags `
191-
"--test_tag_filters=bigtable-integration-tests,storage-integration-tests,spanner-integration-tests" `
191+
"--test_tag_filters=integration-test" `
192192
-- //google/cloud/...:all `
193193
-//google/cloud/bigtable/examples:bigtable_grpc_credentials `
194194
-//google/cloud/storage/examples:storage_service_account_samples `

‎ci/kokoro/windows/build-cmake.ps1‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,15 @@ if (Test-Path env:RUNNING_CI) {
6868
$NCPU=(Get-CimInstance Win32_ComputerSystem).NumberOfLogicalProcessors
6969

7070
$ctest_flags = @("--output-on-failure", "-j", $NCPU, "-C", $env:CONFIG)
71-
ctest $ctest_flags -LE integration-tests
71+
ctest $ctest_flags -LE integration-test
7272
if ($LastExitCode) {
7373
Write-Host -ForegroundColor Red "ctest failed with exit code $LastExitCode"
7474
Exit ${LastExitCode}
7575
}
7676

7777
if ((Test-Path env:RUN_INTEGRATION_TESTS) -and ($env:RUN_INTEGRATION_TESTS -eq "true")) {
7878
Write-Host -ForegroundColor Yellow "`n$(Get-Date -Format o) Running integration tests $env:CONFIG"
79-
ctest $ctest_flags -L integration-tests
79+
ctest $ctest_flags -L integration-test
8080
if ($LastExitCode) {
8181
Write-Host -ForegroundColor Red "Integration tests failed with exit code $LastExitCode"
8282
Exit ${LastExitCode}
@@ -110,7 +110,7 @@ if (Integration-Tests-Enabled) {
110110
Set-Location "${project_root}"
111111
Set-Location "${binary_dir}"
112112
ctest $ctest_flags `
113-
-L '(bigtable-integration-tests|storage-integration-tests|spanner-integration-tests|integration-tests-no-emulator)' `
113+
-L 'integration-test-production' `
114114
-E '(bigtable_grpc_credentials|storage_service_account_samples|service_account_integration_test)'
115115
if ($LastExitCode) {
116116
Write-Host -ForegroundColor Red "Integration tests failed with exit code ${LastExitCode}."

‎doc/setup-development-environment.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ You may need to clone and compile the code as described [here](setup-cmake-envir
7878
Run the tests using:
7979

8080
```console
81-
env -C cmake-out/home ctest --output-on-failure -LE integration-tests
81+
env -C cmake-out/home ctest --output-on-failure -LE integration-test
8282
```
8383

8484
Run the Google Cloud Storage integration tests:

‎google/cloud/bigquery/samples/CMakeLists.txt‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,13 @@ function (bigquery_client_define_samples)
3939

4040
foreach (fname ${bigquery_client_integration_samples})
4141
google_cloud_cpp_set_target_name(target "bigquery" "${fname}")
42-
set_tests_properties(${target} PROPERTIES LABELS "integration-tests")
42+
# These tests are intended to be integration tests, but they don't yet
43+
# work against an emulator or production. Until they work, we'll add the
44+
# 'integration-test' label so they can be excluded from certain test
45+
# patterns (e.g., `ctest -LE integration-test`), but we're not adding
46+
# the 'integration-test-production' or 'integration-test-emulator'
47+
# labels until they work.
48+
set_tests_properties(${target} PROPERTIES LABELS "integration-test")
4349
endforeach ()
4450
endfunction ()
4551

‎google/cloud/bigtable/benchmarks/BUILD‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,7 @@ load(":bigtable_benchmark_programs.bzl", "bigtable_benchmark_programs")
3838
"//conditions:default": ["-lpthread"],
3939
}),
4040
tags = [
41-
"bigtable-integration-tests",
42-
"integration-tests",
41+
"integration-test",
4342
],
4443
deps = [
4544
":bigtable_benchmark_common",

‎google/cloud/bigtable/benchmarks/CMakeLists.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,6 @@ foreach (fname ${bigtable_benchmark_programs})
9999
add_test(NAME ${target} COMMAND ${target})
100100
set_tests_properties(
101101
${target} PROPERTIES LABELS
102-
"integration-tests;bigtable-integration-tests")
102+
"integration-test;integration-test-emulator")
103103
endif ()
104104
endforeach ()

0 commit comments

Comments
 (0)