From d5d69147517f24e287305a113cff3884a1d5cff1 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 11 Dec 2019 18:03:36 -0800 Subject: [PATCH 1/3] Make buildifier sanity test fail-able --- BUILD | 2 +- bazel/grpc_build_system.bzl | 1 + test/core/iomgr/BUILD | 8 ++++---- test/core/iomgr/poller/BUILD | 1 - test/cpp/end2end/BUILD | 4 ++-- test/cpp/microbenchmarks/BUILD | 8 ++++---- tools/run_tests/sanity/check_buildifier.sh | 1 + 7 files changed, 13 insertions(+), 12 deletions(-) diff --git a/BUILD b/BUILD index 32eea118648..4b1361d2d45 100644 --- a/BUILD +++ b/BUILD @@ -873,7 +873,7 @@ grpc_cc_library( "src/core/lib/iomgr/is_epollexclusive_available.h", "src/core/lib/iomgr/load_file.h", "src/core/lib/iomgr/lockfree_event.h", - "src/core/lib/iomgr/logical_thread.h", + "src/core/lib/iomgr/logical_thread.h", "src/core/lib/iomgr/nameser.h", "src/core/lib/iomgr/polling_entity.h", "src/core/lib/iomgr/pollset.h", diff --git a/bazel/grpc_build_system.bzl b/bazel/grpc_build_system.bzl index 7bb6b8bdb98..3064940b01a 100644 --- a/bazel/grpc_build_system.bzl +++ b/bazel/grpc_build_system.bzl @@ -193,6 +193,7 @@ def grpc_cc_test(name, srcs = [], deps = [], external_deps = [], args = [], data copts = if_mac(["-DGRPC_CFSTREAM"]) if language.upper() == "C": copts = copts + if_not_windows(["-std=c99"]) + # NOTE: these attributes won't be used for the poller-specific versions of a test # automatically, you need to set them explicitly (if applicable) args = { diff --git a/test/core/iomgr/BUILD b/test/core/iomgr/BUILD index e0788e5cc87..8bb063afd5e 100644 --- a/test/core/iomgr/BUILD +++ b/test/core/iomgr/BUILD @@ -138,6 +138,9 @@ grpc_cc_test( name = "logical_thread_test", srcs = ["logical_thread_test.cc"], exec_properties = LARGE_MACHINE, + external_deps = [ + "gtest", + ], language = "C++", tags = ["no_windows"], # LARGE_MACHINE is not configured for windows RBE deps = [ @@ -145,9 +148,6 @@ grpc_cc_test( "//:grpc", "//test/core/util:grpc_test_util", ], - external_deps = [ - "gtest", - ], ) grpc_cc_test( @@ -271,8 +271,8 @@ grpc_cc_test( srcs = ["tcp_posix_test.cc"], language = "C++", tags = [ - "no_windows", "no_mac", # TODO(jtattermusch): Reenable once https://github.com/grpc/grpc/issues/21282 is fixed. + "no_windows", ], deps = [ ":endpoint_tests", diff --git a/test/core/iomgr/poller/BUILD b/test/core/iomgr/poller/BUILD index 276d75e9f03..28d36195cc8 100644 --- a/test/core/iomgr/poller/BUILD +++ b/test/core/iomgr/poller/BUILD @@ -1,4 +1,3 @@ - # Copyright 2019 gRPC authors. # # Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/test/cpp/end2end/BUILD b/test/cpp/end2end/BUILD index 55c594c7f15..2bc9854fa74 100644 --- a/test/cpp/end2end/BUILD +++ b/test/cpp/end2end/BUILD @@ -501,6 +501,7 @@ grpc_cc_test( external_deps = [ "gtest", ], + shard_count = 10, tags = [ "no_test_ios", "no_windows", @@ -520,7 +521,6 @@ grpc_cc_test( "//test/core/util:grpc_test_util", "//test/cpp/util:test_util", ], - shard_count = 10, ) grpc_cc_test( @@ -713,6 +713,7 @@ grpc_cc_test( external_deps = [ "gtest", ], + shard_count = 5, tags = ["no_windows"], # TODO(jtattermusch): fix test on windows deps = [ "//:gpr", @@ -724,7 +725,6 @@ grpc_cc_test( "//test/core/util:grpc_test_util", "//test/cpp/util:test_util", ], - shard_count = 5, ) grpc_cc_test( diff --git a/test/cpp/microbenchmarks/BUILD b/test/cpp/microbenchmarks/BUILD index d7d449e8fb7..db7c9db05ae 100644 --- a/test/cpp/microbenchmarks/BUILD +++ b/test/cpp/microbenchmarks/BUILD @@ -137,8 +137,8 @@ grpc_cc_binary( "bm_fullstack_streaming_ping_pong.cc", ], tags = [ - "no_windows", "no_mac", # to emulate "excluded_poll_engines: poll" + "no_windows", ], deps = [":fullstack_streaming_ping_pong_h"], ) @@ -159,8 +159,8 @@ grpc_cc_binary( "bm_fullstack_streaming_pump.cc", ], tags = [ - "no_windows", "no_mac", # to emulate "excluded_poll_engines: poll" + "no_windows", ], deps = [":fullstack_streaming_pump_h"], ) @@ -170,8 +170,8 @@ grpc_cc_binary( testonly = 1, srcs = ["bm_fullstack_trickle.cc"], tags = [ - "no_windows", "no_mac", # to emulate "excluded_poll_engines: poll" + "no_windows", ], deps = [":helpers"], ) @@ -192,8 +192,8 @@ grpc_cc_binary( "bm_fullstack_unary_ping_pong.cc", ], tags = [ - "no_windows", "no_mac", # to emulate "excluded_poll_engines: poll" + "no_windows", ], deps = [":fullstack_unary_ping_pong_h"], ) diff --git a/tools/run_tests/sanity/check_buildifier.sh b/tools/run_tests/sanity/check_buildifier.sh index 03e6847bf2c..37a346c8ee6 100755 --- a/tools/run_tests/sanity/check_buildifier.sh +++ b/tools/run_tests/sanity/check_buildifier.sh @@ -28,6 +28,7 @@ if [[ ${result} != 0 ]]; then echo "" echo " tools/distrib/buildifier_format_code.sh" echo "" + exit 1 else echo "==========BUILDIFIER CHECK PASSED==========" fi From e27782c1c56a3cb95b7f4804fc243cdd4863cade Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Wed, 11 Dec 2019 18:08:33 -0800 Subject: [PATCH 2/3] Simplify the check buildifier script logic --- tools/distrib/buildifier_format_code.sh | 2 +- tools/run_tests/sanity/check_buildifier.sh | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/tools/distrib/buildifier_format_code.sh b/tools/distrib/buildifier_format_code.sh index f205c6c4bb5..06b52767dec 100755 --- a/tools/distrib/buildifier_format_code.sh +++ b/tools/distrib/buildifier_format_code.sh @@ -1,4 +1,4 @@ -#! /bin/bash -ex +#! /bin/bash -e # Copyright 2019 The gRPC Authors # # Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/tools/run_tests/sanity/check_buildifier.sh b/tools/run_tests/sanity/check_buildifier.sh index 37a346c8ee6..115a74fd353 100755 --- a/tools/run_tests/sanity/check_buildifier.sh +++ b/tools/run_tests/sanity/check_buildifier.sh @@ -1,4 +1,4 @@ -#! /bin/bash -x +#! /bin/bash # Copyright 2019 The gRPC Authors # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -13,13 +13,10 @@ # See the License for the specific language governing permissions and # limitations under the License. # -# The script to sanitize Bazel files +# The script to check if Bazel files needs to be formatted. GIT_ROOT="$(dirname "$0")/../../.." -TMP_ROOT="/tmp/buildifier_grpc" -rm -rf "${TMP_ROOT}" -git clone -- "$GIT_ROOT" "$TMP_ROOT" -buildifier -r -v -mode=diff $TMP_ROOT +"$GIT_ROOT/tools/distrib/buildifier_format_code.sh" -mode=diff result=$? if [[ ${result} != 0 ]]; then From 0fd79a7d9df2c8d43c16c1f54eb0c21233a06845 Mon Sep 17 00:00:00 2001 From: Lidi Zheng Date: Thu, 12 Dec 2019 10:26:16 -0800 Subject: [PATCH 3/3] Make the shell script more readable --- tools/distrib/buildifier_format_code.sh | 4 +++- tools/run_tests/sanity/check_buildifier.sh | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/distrib/buildifier_format_code.sh b/tools/distrib/buildifier_format_code.sh index 06b52767dec..8245cd4ae50 100755 --- a/tools/distrib/buildifier_format_code.sh +++ b/tools/distrib/buildifier_format_code.sh @@ -1,4 +1,4 @@ -#! /bin/bash -e +#! /bin/bash # Copyright 2019 The gRPC Authors # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -13,6 +13,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +set -e + BUILDIFIER_VERSION="0.29.0" TEMP_BUILDIFIER_PATH="/tmp/buildifier" EXTRA_BUILDIFIER_FLAGS=$* diff --git a/tools/run_tests/sanity/check_buildifier.sh b/tools/run_tests/sanity/check_buildifier.sh index 115a74fd353..bb183711c42 100755 --- a/tools/run_tests/sanity/check_buildifier.sh +++ b/tools/run_tests/sanity/check_buildifier.sh @@ -13,7 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. # -# The script to check if Bazel files needs to be formatted. +# The script to check if Bazel files need to be formatted. GIT_ROOT="$(dirname "$0")/../../.." "$GIT_ROOT/tools/distrib/buildifier_format_code.sh" -mode=diff