Skip to content
Merged
12 changes: 10 additions & 2 deletions .github/workflows/verify_library_generation.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ on:
- library_generation/**

workflow_dispatch:
name: verify_library_generation_against_googleapis-gen
name: verify_library_generation
jobs:
verify_library_generation:
integration_tests:
strategy:
matrix:
java: [ 8 ]
Expand All @@ -30,3 +30,11 @@ jobs:
-d google-cloud-bigtable-v2-java \
--googleapis_gen_url https://cloud-java-bot:${{ secrets.CLOUD_JAVA_BOT_GITHUB_TOKEN }}@github.com/googleapis/googleapis-gen.git \
--os_type ${{ matrix.os }}
unit_tests:
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v3
- name: Run unit tests
run: |
set -x
library_generation/test/generate_library_unit_tests.sh
259 changes: 259 additions & 0 deletions library_generation/test/generate_library_unit_tests.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,259 @@
#!/usr/bin/env bash

set -xeo pipefail

# Variables used to generate final result
total_num=0
succeed_num=0
failed_num=0
failed_tests=""
# Unit tests against ./utilities.sh
script_dir=$(dirname "$(readlink -f "$0")")
source "${script_dir}"/../utilities.sh

# Helper functions, they shouldn't be called outside this file.
__assertEquals() {
expected=$1
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.

go/shell-style#use-local-variables
Functions should declare their local variables with the local keyword. We have several functions (not only in this PR) around that are not following this style. cc: @blakeli0 @suztomo would it make sense to have a "styling up" PR to make our scripts more compliant with the guidelines?

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.

go/shell-style#constants-environment-variables-and-readonly-variables
Another example, to have constants with upper case (e.g. SCRIPT_DIR instead of script_dir)

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.

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.

I plan to propose a PR to format code after merging this one.

actual=$2
if [[ "${expected}" == "${actual}" ]]; then
return 0
fi

echo "Error: expected ${expected}, got ${actual} instead."
return 1
}

__assertFileDoesNotExist() {
expected_file=$1
if [ ! -f "${expected_file}" ]; then
return 0
fi

echo "Error: ${expected_file} exists."
return 1
}

__test_executed() {
total_num=$((1 + total_num))
}

__test_succeed() {
succeed_num=$((1 + succeed_num))
}

__test_failed() {
failed_test=$1
failed_num=$((1 + failed_num))
failed_tests="${failed_tests} ${failed_test}"
}

# Unit tests
extract_folder_name_test() {
path="google/cloud/aiplatform/v1/google-cloud-aiplatform-v1-java"
folder_name=$(extract_folder_name "${path}")
__assertEquals "google-cloud-aiplatform-v1-java" "${folder_name}"
}

get_grpc_version_succeed_with_valid_generator_version_test() {
actual_version=$(get_grpc_version "2.24.0")
__assertEquals "1.56.1" "${actual_version}"
rm "gapic-generator-java-pom-parent-2.24.0.pom"
}

get_grpc_version_failed_with_invalid_generator_version_test() {
actual_version=$(get_grpc_version "1.99.0")
__assertEquals "" "${actual_version}"
}

get_protobuf_version_succeed_with_valid_generator_version_test() {
actual_version=$(get_protobuf_version "2.24.0")
__assertEquals "23.2" "${actual_version}"
rm "gapic-generator-java-pom-parent-2.24.0.pom"
}

get_protobuf_version_failed_with_invalid_generator_version_test() {
actual_version=$(get_protobuf_version "1.99.0")
__assertEquals "" "${actual_version}"
}

search_additional_protos_common_resources_test() {
proto_path="${script_dir}/resources/monitoring"
addition_protos=$(search_additional_protos)
__assertEquals "google/cloud/common_resources.proto" "${addition_protos}"
}

search_additional_protos_iam_test() {
proto_path="${script_dir}/resources/pubsub"
addition_protos=$(search_additional_protos)
__assertEquals \
"google/cloud/common_resources.proto google/iam/v1/iam_policy.proto" \
"${addition_protos}"
}

search_additional_protos_location_test() {
proto_path="${script_dir}/resources/firestore"
addition_protos=$(search_additional_protos)
__assertEquals \
"google/cloud/common_resources.proto google/cloud/location/locations.proto" \
"${addition_protos}"
}

search_additional_protos_iam_location_test() {
proto_path="${script_dir}/resources/alloydb"
addition_protos=$(search_additional_protos)
__assertEquals \
"google/cloud/common_resources.proto google/iam/v1/iam_policy.proto google/cloud/location/locations.proto" \
"${addition_protos}"
}

get_gapic_opts_with_rest_test() {
proto_path="${script_dir}/resources/monitoring"
transport="grpc"
rest_numeric_enums="true"
gapic_opts="$(get_gapic_opts)"
__assertEquals \
"transport=grpc,rest-numeric-enums,grpc-service-config=${proto_path}/monitoring_grpc_service_config.json,gapic-config=${proto_path}/monitoring_gapic.yaml,api-service-config=${proto_path}/monitoring.yaml" \
"${gapic_opts}"
}

get_gapic_opts_without_rest_test() {
proto_path="${script_dir}/resources/monitoring"
transport="grpc"
rest_numeric_enums="false"
gapic_opts="$(get_gapic_opts)"
__assertEquals \
"transport=grpc,grpc-service-config=${proto_path}/monitoring_grpc_service_config.json,gapic-config=${proto_path}/monitoring_gapic.yaml,api-service-config=${proto_path}/monitoring.yaml" \
"$gapic_opts"
}

remove_grpc_version_test() {
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.

Not directly related to the tests, I wonder why we have to remove the gRPC version manually, what made the differences? Separately, I think it might be good to keep the version for troubleshooting/informational purposes.

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.

When using bazel build, there's a "by gRPC proto compiler" at the beginning of *Grpc.java file. However, the annotation becomes "by gRPC proto compiler xxx" when using shell script, where xxx is the grpc version.

I use this function to remove the version number so that two files are totally identical.

Copy link
Copy Markdown
Contributor

@blakeli0 blakeli0 Sep 1, 2023

Choose a reason for hiding this comment

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

I thought bazel build eventually calls the same gRPC proto compiler? If not, we need to understand what exactly caused the differences, as there could be more differences when generating other services.

Copy link
Copy Markdown
Contributor Author

@JoeWang1127 JoeWang1127 Sep 1, 2023

Choose a reason for hiding this comment

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

I think bazel build download the gRPC proto compiler source code from github (defined here in gax/repository.bzl) and build from source.

In generate_library.sh, it download gPRC compiler executable from maven central. I don't know whether there are differences but the generated code are the same.

I haven't seen any code difference when generating other services and I'll add more libraries to the integration test to make sure the generated code is the same as bazel build.

I also post a question and hope someone from grpc team has the answer.

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.

I think bazel build download the gRPC proto compiler source code from github (defined here in gax/repository.bzl) and build from source.

Thanks, this makes sense! Since we were building from source, maybe there is no concept of version on main branch, let's see if anyone answers the question.

destination_path="${script_dir}/resources/monitoring"
cp "${destination_path}/QueryServiceGrpc_copy.java" "${destination_path}/QueryServiceGrpc.java"
remove_grpc_version
return_code=0
if grep -q 'value = "by gRPC proto compiler",' "${destination_path}/QueryServiceGrpc.java"; then
echo "grpc version is removed."
else
echo "Error: grpc version is not removed."
return_code=1
fi

rm "${destination_path}/QueryServiceGrpc.java"
return "${return_code}"
}

download_generator_success_with_valid_version_test() {
download_generator "2.24.0"
__assertFileExists "gapic-generator-java-2.24.0.jar"
rm "gapic-generator-java-2.24.0.jar"
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.

I think we can have an "afterEach" function that cleans up the downloaded jars/tars, even if they don't exist. It could even be useful for local development where I personally find it a bit hard to read dir contents after I call the scripts

Copy link
Copy Markdown
Contributor

@diegomarquezp diegomarquezp Sep 1, 2023

Choose a reason for hiding this comment

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

This could be expanded with a git status to confirm we don't have any unexpected untracked files that aren't deleted by such function

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.

Not all tests have clean up actions so it's difficult to write a function and apply it after every test execution.

However, I'll think about it in the reformat PR.

}

download_generator_failed_with_invalid_version_test() {
# The download function will exit the shell
# if download failed. Test the exit code instead of
# downloaded file (there will be no downloaded file).
# Use $() to execute the function in subshell so that
# the other tests can continue executing in the current
# shell.
res=0
$(download_generator "1.99.0") || res=$?
__assertEquals 1 $((res))
}

download_protobuf_succeed_with_valid_version_linux_test() {
download_protobuf "23.2" "linux-x86_64"
__assertFileExists "protobuf-23.2"
rm -rf "protobuf-23.2"
}

download_protobuf_succeed_with_valid_version_macos_test() {
download_protobuf "23.2" "osx-x86_64"
__assertFileExists "protobuf-23.2"
rm -rf "protobuf-23.2" "google"
}

download_protobuf_failed_with_invalid_version_linux_test() {
res=0
$(download_protobuf "22.99" "linux-x86_64") || res=$?
__assertEquals 1 $((res))
}

download_protobuf_failed_with_invalid_arch_test() {
res=0
$(download_protobuf "23.2" "customized-x86_64") || res=$?
__assertEquals 1 $((res))
}

download_grpc_plugin_succeed_with_valid_version_linux_test() {
download_grpc_plugin "1.55.1" "linux-x86_64"
__assertFileExists "protoc-gen-grpc-java-1.55.1-linux-x86_64.exe"
rm "protoc-gen-grpc-java-1.55.1-linux-x86_64.exe"
}

download_grpc_plugin_succeed_with_valid_version_macos_test() {
download_grpc_plugin "1.55.1" "osx-x86_64"
__assertFileExists "protoc-gen-grpc-java-1.55.1-osx-x86_64.exe"
rm "protoc-gen-grpc-java-1.55.1-osx-x86_64.exe"
}

download_grpc_plugin_failed_with_invalid_version_linux_test() {
res=0
$(download_grpc_plugin "0.99.0" "linux-x86_64") || res=$?
__assertEquals 1 $((res))
}

download_grpc_plugin_failed_with_invalid_arch_test() {
res=0
$(download_grpc_plugin "1.55.1" "customized-x86_64") || res=$?
__assertEquals 1 $((res))
}

# Execute tests.
# One line per test.
test_list=(
extract_folder_name_test
get_grpc_version_succeed_with_valid_generator_version_test
get_grpc_version_failed_with_invalid_generator_version_test
get_protobuf_version_succeed_with_valid_generator_version_test
get_protobuf_version_failed_with_invalid_generator_version_test
search_additional_protos_common_resources_test
search_additional_protos_iam_test
search_additional_protos_location_test
search_additional_protos_iam_location_test
get_gapic_opts_with_rest_test
get_gapic_opts_without_rest_test
remove_grpc_version_test
download_generator_success_with_valid_version_test
download_generator_failed_with_invalid_version_test
download_protobuf_succeed_with_valid_version_linux_test
download_protobuf_succeed_with_valid_version_macos_test
download_protobuf_failed_with_invalid_version_linux_test
download_protobuf_failed_with_invalid_arch_test
download_grpc_plugin_succeed_with_valid_version_linux_test
download_grpc_plugin_succeed_with_valid_version_macos_test
download_grpc_plugin_failed_with_invalid_version_linux_test
download_grpc_plugin_failed_with_invalid_arch_test
)

for ut in "${test_list[@]}"; do
pushd "${script_dir}"
__test_executed
result=0
"${ut}" || result=$?
if [[ "${result}" == 0 ]]; then
__test_succeed
else
__test_failed "${ut}"
fi
popd
done

echo "Test result: ${total_num} tests executed, ${succeed_num} succeed, ${failed_num} failed."
if [[ "${total_num}" == "${succeed_num}" ]]; then
echo "All tests passed."
exit
fi

echo "Test failed."
echo "Failed test(s): ${failed_tests}."
exit 1
Loading