-
Notifications
You must be signed in to change notification settings - Fork 75
chore: add unit tests for utilities.sh
#1948
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
Changes from all commits
8554d7b
488a67f
20c7550
823c974
9dcdd8f
3b1995a
6cb4180
8bfd885
b8d56d5
0f7c164
272627a
a5db4d2
5143528
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 | ||
| 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() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When using I use this function to remove the version number so that two files are totally identical.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think In 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 I also post a question and hope someone from grpc team has the answer.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Thanks, this makes sense! Since we were building from source, maybe there is no concept of |
||
| 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" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can have an "
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be expanded with a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
There was a problem hiding this comment.
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
localkeyword. 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?There was a problem hiding this comment.
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_DIRinstead ofscript_dir)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nvm, I saw at the end of the changes we have https://github.com/googleapis/sdk-platform-java/pull/1948/files#r1311724425 to address this!
There was a problem hiding this comment.
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.