From 8d4d52d3961263a72a071ded1fc418b41bba22a8 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Sat, 7 Oct 2017 17:28:58 -0700 Subject: [PATCH 1/7] Multithread & shard stats test, make it exhaustive --- test/core/debug/stats_test.cc | 74 +++++++++++++++++++++-------------- 1 file changed, 45 insertions(+), 29 deletions(-) diff --git a/test/core/debug/stats_test.cc b/test/core/debug/stats_test.cc index c85ab3598ab..35f11eae378 100644 --- a/test/core/debug/stats_test.cc +++ b/test/core/debug/stats_test.cc @@ -20,7 +20,10 @@ extern "C" { #include "src/core/lib/debug/stats.h" } +#include + #include +#include #include #include @@ -79,38 +82,51 @@ static int FindExpectedBucket(int i, int j) { grpc_stats_histo_bucket_boundaries[i] - 1; } -TEST(StatsTest, IncHistogram) { - for (int i = 0; i < GRPC_STATS_HISTOGRAM_COUNT; i++) { - std::vector test_values; - for (int j = -1000; - j < - grpc_stats_histo_bucket_boundaries[i] - [grpc_stats_histo_buckets[i] - 1] + - 1000; - j++) { - test_values.push_back(j); - } - std::random_shuffle(test_values.begin(), test_values.end()); - if (test_values.size() > 10000) { - test_values.resize(10000); - } - for (auto j : test_values) { - Snapshot snapshot; - - int expected_bucket = FindExpectedBucket(i, j); - - grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; - grpc_stats_inc_histogram[i](&exec_ctx, j); - grpc_exec_ctx_finish(&exec_ctx); - - auto delta = snapshot.delta(); - - EXPECT_EQ(delta.histograms[grpc_stats_histo_start[i] + expected_bucket], - 1); - } +class HistogramTest : public ::testing::TestWithParam {}; + +TEST_P(HistogramTest, IncHistogram) { + const int kHistogram = GetParam(); + const int kThreads = std::max(1, (int)gpr_cpu_num_cores()); + std::vector threads; + for (int thread = 0; thread < kThreads; thread++) { + threads.emplace_back([kHistogram, kThreads, thread]() { + std::vector test_values; + for (int j = -1000 + thread; + j < grpc_stats_histo_bucket_boundaries + [kHistogram][grpc_stats_histo_buckets[kHistogram] - 1] + + 1000; + j += kThreads) { + test_values.push_back(j); + } + std::random_shuffle(test_values.begin(), test_values.end()); + for (auto j : test_values) { + Snapshot snapshot; + + int expected_bucket = FindExpectedBucket(kHistogram, j); + + grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; + grpc_stats_inc_histogram[kHistogram](&exec_ctx, j); + grpc_exec_ctx_finish(&exec_ctx); + + auto delta = snapshot.delta(); + + EXPECT_GE(delta.histograms[grpc_stats_histo_start[kHistogram] + + expected_bucket], + 1) + << "\nhistogram:" << kHistogram + << "\nexpected_bucket:" << expected_bucket << "\nthread:" << thread + << "\nj:" << j; + } + }); + } + for (auto& t : threads) { + t.join(); } } +INSTANTIATE_TEST_CASE_P(HistogramTestCases, HistogramTest, + ::testing::Range(0, GRPC_STATS_HISTOGRAM_COUNT)); + } // namespace testing } // namespace grpc From 520e59f2fb8541a91d9a8565301ed0e496c3d59c Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Sat, 7 Oct 2017 17:34:41 -0700 Subject: [PATCH 2/7] test/core should trigger c++ suite nowadays --- tools/run_tests/python_utils/filter_pull_request_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/run_tests/python_utils/filter_pull_request_tests.py b/tools/run_tests/python_utils/filter_pull_request_tests.py index 393aef86621..56d6e4e988c 100644 --- a/tools/run_tests/python_utils/filter_pull_request_tests.py +++ b/tools/run_tests/python_utils/filter_pull_request_tests.py @@ -78,7 +78,7 @@ _WHITELIST_DICT = { '^src/python/': [_PYTHON_TEST_SUITE], '^src/ruby/': [_RUBY_TEST_SUITE], '^templates/': [], - '^test/core/': [_CORE_TEST_SUITE], + '^test/core/': [_CORE_TEST_SUITE, _CPP_TEST_SUITE], '^test/cpp/': [_CPP_TEST_SUITE], '^test/distrib/cpp/': [_CPP_TEST_SUITE], '^test/distrib/csharp/': [_CSHARP_TEST_SUITE], From c5fb7e5b73ca587ea59c1657bd221af0743929d5 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Sun, 8 Oct 2017 03:25:31 +0000 Subject: [PATCH 3/7] Use a mutex for an exact test --- build.yaml | 1 + test/core/debug/stats_test.cc | 14 +++++++++----- tools/run_tests/generated/tests.json | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/build.yaml b/build.yaml index 19ce721be4e..211cf6913fd 100644 --- a/build.yaml +++ b/build.yaml @@ -4607,6 +4607,7 @@ targets: - grpc - gpr_test_util - gpr + uses_polling: false - name: status_test build: test language: c++ diff --git a/test/core/debug/stats_test.cc b/test/core/debug/stats_test.cc index 35f11eae378..c652e446b8e 100644 --- a/test/core/debug/stats_test.cc +++ b/test/core/debug/stats_test.cc @@ -20,6 +20,7 @@ extern "C" { #include "src/core/lib/debug/stats.h" } +#include #include #include @@ -86,10 +87,12 @@ class HistogramTest : public ::testing::TestWithParam {}; TEST_P(HistogramTest, IncHistogram) { const int kHistogram = GetParam(); - const int kThreads = std::max(1, (int)gpr_cpu_num_cores()); + const int kBuckets = grpc_stats_histo_buckets[kHistogram]; + const int kThreads = kBuckets; std::vector threads; + std::vector mutexes(kBuckets); for (int thread = 0; thread < kThreads; thread++) { - threads.emplace_back([kHistogram, kThreads, thread]() { + threads.emplace_back([kHistogram, kThreads, thread, &mutexes]() { std::vector test_values; for (int j = -1000 + thread; j < grpc_stats_histo_bucket_boundaries @@ -100,9 +103,10 @@ TEST_P(HistogramTest, IncHistogram) { } std::random_shuffle(test_values.begin(), test_values.end()); for (auto j : test_values) { - Snapshot snapshot; - int expected_bucket = FindExpectedBucket(kHistogram, j); + std::lock_guard lock(mutexes[expected_bucket]); + + Snapshot snapshot; grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; grpc_stats_inc_histogram[kHistogram](&exec_ctx, j); @@ -110,7 +114,7 @@ TEST_P(HistogramTest, IncHistogram) { auto delta = snapshot.delta(); - EXPECT_GE(delta.histograms[grpc_stats_histo_start[kHistogram] + + EXPECT_EQ(delta.histograms[grpc_stats_histo_start[kHistogram] + expected_bucket], 1) << "\nhistogram:" << kHistogram diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 6f9b3cab897..72e840e2f20 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -4195,7 +4195,7 @@ "posix", "windows" ], - "uses_polling": true + "uses_polling": false }, { "args": [], From 3cf8d50d1ecc3e9c2dc756cff4d1ff9fcb6c80b9 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Sun, 8 Oct 2017 05:11:15 +0000 Subject: [PATCH 4/7] accurate, exhaustive "fast" version of this test --- build.yaml | 1 + test/core/debug/stats_test.cc | 58 ++++++++++++++-------------- tools/run_tests/generated/tests.json | 1 + tools/run_tests/run_tests.py | 2 +- 4 files changed, 32 insertions(+), 30 deletions(-) diff --git a/build.yaml b/build.yaml index 211cf6913fd..1cb00ec146f 100644 --- a/build.yaml +++ b/build.yaml @@ -4607,6 +4607,7 @@ targets: - grpc - gpr_test_util - gpr + timeout_seconds: 1200 uses_polling: false - name: status_test build: test diff --git a/test/core/debug/stats_test.cc b/test/core/debug/stats_test.cc index c652e446b8e..db9105672ee 100644 --- a/test/core/debug/stats_test.cc +++ b/test/core/debug/stats_test.cc @@ -87,42 +87,42 @@ class HistogramTest : public ::testing::TestWithParam {}; TEST_P(HistogramTest, IncHistogram) { const int kHistogram = GetParam(); - const int kBuckets = grpc_stats_histo_buckets[kHistogram]; - const int kThreads = kBuckets; std::vector threads; - std::vector mutexes(kBuckets); - for (int thread = 0; thread < kThreads; thread++) { - threads.emplace_back([kHistogram, kThreads, thread, &mutexes]() { + int cur_bucket = 0; + auto run = [kHistogram](const std::vector& test_values, int expected_bucket) { + gpr_log(GPR_DEBUG, "expected_bucket:%d nvalues=%" PRIdPTR, expected_bucket, test_values.size()); + for (auto j : test_values) { + Snapshot snapshot; + + grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; + grpc_stats_inc_histogram[kHistogram](&exec_ctx, j); + grpc_exec_ctx_finish(&exec_ctx); + + auto delta = snapshot.delta(); + + EXPECT_EQ(delta.histograms[grpc_stats_histo_start[kHistogram] + + expected_bucket], + 1) + << "\nhistogram:" << kHistogram + << "\nexpected_bucket:" << expected_bucket + << "\nj:" << j; + } + }; std::vector test_values; - for (int j = -1000 + thread; + for (int j = -1000; j < grpc_stats_histo_bucket_boundaries [kHistogram][grpc_stats_histo_buckets[kHistogram] - 1] + 1000; - j += kThreads) { + j ++) { + int expected_bucket = FindExpectedBucket(kHistogram, j); + if (cur_bucket != expected_bucket) { + threads.emplace_back([test_values, run, cur_bucket]() { run(test_values, cur_bucket); }); + cur_bucket = expected_bucket; + test_values.clear(); + } test_values.push_back(j); } - std::random_shuffle(test_values.begin(), test_values.end()); - for (auto j : test_values) { - int expected_bucket = FindExpectedBucket(kHistogram, j); - std::lock_guard lock(mutexes[expected_bucket]); - - Snapshot snapshot; - - grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; - grpc_stats_inc_histogram[kHistogram](&exec_ctx, j); - grpc_exec_ctx_finish(&exec_ctx); - - auto delta = snapshot.delta(); - - EXPECT_EQ(delta.histograms[grpc_stats_histo_start[kHistogram] + - expected_bucket], - 1) - << "\nhistogram:" << kHistogram - << "\nexpected_bucket:" << expected_bucket << "\nthread:" << thread - << "\nj:" << j; - } - }); - } + run(test_values, cur_bucket); for (auto& t : threads) { t.join(); } diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 72e840e2f20..fa75017781e 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -4195,6 +4195,7 @@ "posix", "windows" ], + "timeout_seconds": 1200, "uses_polling": false }, { diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 7c65067857f..6bdd8be7e95 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -353,7 +353,7 @@ class CLanguage(object): out.append(self.config.job_spec(cmdline, shortname='%s %s' % (' '.join(cmdline), shortname_ext), cpu_cost=cpu_cost, - timeout_seconds=_DEFAULT_TIMEOUT_SECONDS * timeout_scaling, + timeout_seconds=target.get('timeout_seconds', _DEFAULT_TIMEOUT_SECONDS) * timeout_scaling, environ=env)) else: cmdline = [binary] + target['args'] From f0a24123389beba7350071d0b20f05ac664b525d Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Sat, 7 Oct 2017 22:15:41 -0700 Subject: [PATCH 5/7] clang-format --- test/core/debug/stats_test.cc | 75 +++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 35 deletions(-) diff --git a/test/core/debug/stats_test.cc b/test/core/debug/stats_test.cc index db9105672ee..501581952d9 100644 --- a/test/core/debug/stats_test.cc +++ b/test/core/debug/stats_test.cc @@ -88,41 +88,46 @@ class HistogramTest : public ::testing::TestWithParam {}; TEST_P(HistogramTest, IncHistogram) { const int kHistogram = GetParam(); std::vector threads; - int cur_bucket = 0; - auto run = [kHistogram](const std::vector& test_values, int expected_bucket) { - gpr_log(GPR_DEBUG, "expected_bucket:%d nvalues=%" PRIdPTR, expected_bucket, test_values.size()); - for (auto j : test_values) { - Snapshot snapshot; - - grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; - grpc_stats_inc_histogram[kHistogram](&exec_ctx, j); - grpc_exec_ctx_finish(&exec_ctx); - - auto delta = snapshot.delta(); - - EXPECT_EQ(delta.histograms[grpc_stats_histo_start[kHistogram] + - expected_bucket], - 1) - << "\nhistogram:" << kHistogram - << "\nexpected_bucket:" << expected_bucket - << "\nj:" << j; - } - }; - std::vector test_values; - for (int j = -1000; - j < grpc_stats_histo_bucket_boundaries - [kHistogram][grpc_stats_histo_buckets[kHistogram] - 1] + - 1000; - j ++) { - int expected_bucket = FindExpectedBucket(kHistogram, j); - if (cur_bucket != expected_bucket) { - threads.emplace_back([test_values, run, cur_bucket]() { run(test_values, cur_bucket); }); - cur_bucket = expected_bucket; - test_values.clear(); - } - test_values.push_back(j); - } - run(test_values, cur_bucket); + int cur_bucket = 0; + auto run = [kHistogram](const std::vector& test_values, + int expected_bucket) { + gpr_log(GPR_DEBUG, "expected_bucket:%d nvalues=%" PRIdPTR, expected_bucket, + test_values.size()); + for (auto j : test_values) { + Snapshot snapshot; + + grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; + grpc_stats_inc_histogram[kHistogram](&exec_ctx, j); + grpc_exec_ctx_finish(&exec_ctx); + + auto delta = snapshot.delta(); + + EXPECT_EQ( + delta + .histograms[grpc_stats_histo_start[kHistogram] + expected_bucket], + 1) + << "\nhistogram:" << kHistogram + << "\nexpected_bucket:" << expected_bucket << "\nj:" << j; + } + }; + std::vector test_values; + for (int j = -1000; + j < + grpc_stats_histo_bucket_boundaries[kHistogram] + [grpc_stats_histo_buckets[kHistogram] - + 1] + + 1000; + j++) { + int expected_bucket = FindExpectedBucket(kHistogram, j); + if (cur_bucket != expected_bucket) { + threads.emplace_back( + [test_values, run, cur_bucket]() { run(test_values, cur_bucket); }); + cur_bucket = expected_bucket; + test_values.clear(); + } + test_values.push_back(j); + } + run(test_values, cur_bucket); for (auto& t : threads) { t.join(); } From 4c2f7025921d07bad72db4ee00749a4fdfd4c2fd Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 10 Oct 2017 22:37:20 -0700 Subject: [PATCH 6/7] Fixes --- tools/run_tests/sanity/check_test_filtering.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/run_tests/sanity/check_test_filtering.py b/tools/run_tests/sanity/check_test_filtering.py index 3ebb9389f79..a523f087a13 100755 --- a/tools/run_tests/sanity/check_test_filtering.py +++ b/tools/run_tests/sanity/check_test_filtering.py @@ -81,7 +81,8 @@ class TestFilteringTest(unittest.TestCase): self.test_filtering(['src/core/foo.bar'], [_LIST_OF_LANGUAGE_LABELS]) # Testing individual languages self.test_filtering(['test/core/foo.bar'], [label for label in _LIST_OF_LANGUAGE_LABELS if label not in - filter_pull_request_tests._CORE_TEST_SUITE.labels]) + filter_pull_request_tests._CORE_TEST_SUITE.labels + + filter_pull_request_tests._CPP_TEST_SUITE.labels]) self.test_filtering(['src/cpp/foo.bar'], [label for label in _LIST_OF_LANGUAGE_LABELS if label not in filter_pull_request_tests._CPP_TEST_SUITE.labels]) self.test_filtering(['src/csharp/foo.bar'], [label for label in _LIST_OF_LANGUAGE_LABELS if label not in From 5f7ec2b019e22bc3dda119a8e1d12e9630b659ae Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 18 Oct 2017 09:10:45 -0700 Subject: [PATCH 7/7] Exclude stats_test from tsan --- build.yaml | 2 ++ tools/run_tests/generated/tests.json | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/build.yaml b/build.yaml index 117f8c50bb7..e3627130c26 100644 --- a/build.yaml +++ b/build.yaml @@ -4719,6 +4719,8 @@ targets: - grpc - gpr_test_util - gpr + exclude_configs: + - tsan timeout_seconds: 1200 uses_polling: false - name: status_test diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 7b1d6fccb37..1ba386b59cb 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -4206,7 +4206,9 @@ "windows" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "tsan" + ], "exclude_iomgrs": [], "flaky": false, "gtest": true,