From 0df71180345c201737915ed1d0715c628d03e866 Mon Sep 17 00:00:00 2001 From: Twice Date: Mon, 20 Mar 2023 21:35:48 +0800 Subject: [PATCH] Add clang-tidy checking for unit tests (#1337) --- .clang-tidy | 2 +- src/server/tls_util.cc | 2 +- tests/cppunit/cluster_test.cc | 4 ++-- tests/cppunit/cron_test.cc | 2 +- tests/cppunit/disk_test.cc | 8 ++++---- tests/cppunit/observer_or_unique_test.cc | 8 ++++---- tests/cppunit/parse_util.cc | 4 ++-- tests/cppunit/status_test.cc | 8 ++++---- tests/cppunit/string_reply_test.cc | 4 ++-- tests/cppunit/test_base.h | 2 +- tests/cppunit/types/stream_test.cc | 2 +- x.py | 2 +- 12 files changed, 24 insertions(+), 24 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index a6870e827b6..f4f8c336156 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,7 +1,7 @@ # refer to https://clang.llvm.org/extra/clang-tidy/checks/list.html Checks: -*, clang-analyzer-core.*, clang-analyzer-cplusplus.*, clang-analyzer-deadcode.*, clang-analyzer-nullability.*, clang-analyzer-security.*, clang-analyzer-unix.*, clang-analyzer-valist.*, cppcoreguidelines-init-variables, cppcoreguidelines-macro-usage, cppcoreguidelines-interfaces-global-init, cppcoreguidelines-narrowing-conversions, cppcoreguidelines-no-malloc, cppcoreguidelines-prefer-member-initializer, cppcoreguidelines-special-member-functions, cppcoreguidelines-slicing, google-build-explicit-make-pair, google-default-arguments, google-explicit-constructor, modernize-avoid-bind, modernize-loop-convert, modernize-macro-to-enum, modernize-make-shared, modernize-make-unique, modernize-pass-by-value, modernize-redundant-void-arg, modernize-return-braced-init-list, modernize-use-auto, modernize-use-bool-literals, modernize-use-emplace, modernize-use-equals-default, modernize-use-equals-delete, modernize-use-nullptr, modernize-use-override, modernize-use-using, performance-faster-string-find, performance-for-range-copy, performance-implicit-conversion-in-loop, performance-inefficient-algorithm, performance-inefficient-vector-operation, performance-move-const-arg, performance-move-constructor-init, performance-no-automatic-move, performance-trivially-destructible, performance-type-promotion-in-math-fn, performance-unnecessary-copy-initialization, performance-unnecessary-value-param -WarningsAsErrors: clang-analyzer-*, -clang-analyzer-security.insecureAPI.rand, cppcoreguidelines-interfaces-global-init, cppcoreguidelines-no-malloc, cppcoreguidelines-slicing, google-*, modernize-use-emplace, modernize-use-equals-default, modernize-use-equals-delete, performance-implicit-conversion-in-loop, performance-inefficient-algorithm, performance-move-constructor-init, performance-no-automatic-move, performance-trivially-destructible, performance-type-promotion-in-math-fn, performance-unnecessary-copy-initialization, modernize-use-bool-literals, performance-unnecessary-value-param, modernize-make-unique, performance-for-range-copy, performance-faster-string-find, modernize-redundant-void-arg, modernize-avoid-bind, modernize-use-auto, modernize-use-using, performance-inefficient-vector-operation, cppcoreguidelines-special-member-functions, modernize-loop-convert, cppcoreguidelines-init-variables, modernize-use-nullptr, cppcoreguidelines-macro-usage, cppcoreguidelines-narrowing-conversions, modernize-return-braced-init-list +WarningsAsErrors: clang-analyzer-*, -clang-analyzer-security.insecureAPI.rand, google-*, performance-*, cppcoreguidelines-*, modernize-* CheckOptions: - key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor diff --git a/src/server/tls_util.cc b/src/server/tls_util.cc index ec8587ade21..1fa6efc88a0 100644 --- a/src/server/tls_util.cc +++ b/src/server/tls_util.cc @@ -194,7 +194,7 @@ UniqueSSLContext CreateSSLContext(const Config *config, const SSL_METHOD *method SSL_CTX_set_default_passwd_cb(ssl_ctx.get(), [](char *buf, int size, int, void *cfg) -> int { strncpy(buf, static_cast(cfg)->tls_key_file_pass.c_str(), size); buf[size - 1] = '\0'; - return strlen(buf); + return static_cast(strlen(buf)); }); } diff --git a/tests/cppunit/cluster_test.cc b/tests/cppunit/cluster_test.cc index 865122bf94f..d23014e03c3 100644 --- a/tests/cppunit/cluster_test.cc +++ b/tests/cppunit/cluster_test.cc @@ -113,8 +113,8 @@ TEST(Cluster, CluseterGetNodes) { std::vector vnodes = Util::Split(output_nodes, "\n"); - for (unsigned i = 0; i < vnodes.size(); i++) { - std::vector node_fields = Util::Split(vnodes[i], " "); + for (const auto &vnode : vnodes) { + std::vector node_fields = Util::Split(vnode, " "); if (node_fields[0] == "07c37dfeb235213a872192d90877d0cd55635b91") { ASSERT_TRUE(node_fields.size() == 8); diff --git a/tests/cppunit/cron_test.cc b/tests/cppunit/cron_test.cc index 69d017b8856..c81f05c3c6d 100644 --- a/tests/cppunit/cron_test.cc +++ b/tests/cppunit/cron_test.cc @@ -39,7 +39,7 @@ class CronTest : public testing::Test { }; TEST_F(CronTest, IsTimeMatch) { - std::time_t t = std::time(0); + std::time_t t = std::time(nullptr); std::tm *now = std::localtime(&t); now->tm_hour = 3; ASSERT_TRUE(cron->IsTimeMatch(now)); diff --git a/tests/cppunit/disk_test.cc b/tests/cppunit/disk_test.cc index d161ef01864..a46f9aa427d 100644 --- a/tests/cppunit/disk_test.cc +++ b/tests/cppunit/disk_test.cc @@ -161,7 +161,7 @@ TEST_F(RedisDiskTest, BitmapDisk) { EXPECT_TRUE(bitmap->SetBit(key_, i, true, &bit).ok()); approximate_size += key_.size() + 8 + std::to_string(i / 1024 / 8).size(); } - uint64_t key_size; + uint64_t key_size = 0; EXPECT_TRUE(disk->GetKeySize(key_, kRedisBitmap, &key_size).ok()); EXPECT_GE(key_size, approximate_size * estimation_factor_); EXPECT_LE(key_size, approximate_size / estimation_factor_); @@ -172,13 +172,13 @@ TEST_F(RedisDiskTest, SortedintDisk) { std::unique_ptr sortedint = std::make_unique(storage_, "disk_ns_sortedint"); std::unique_ptr disk = std::make_unique(storage_, "disk_ns_sortedint"); key_ = "sortedintdisk_key"; - int ret; + int ret = 0; uint64_t approximate_size = 0; for (int i = 0; i < 100000; i++) { EXPECT_TRUE(sortedint->Add(key_, std::vector{uint64_t(i)}, &ret).ok() && ret == 1); approximate_size += key_.size() + 8 + 8; } - uint64_t key_size; + uint64_t key_size = 0; EXPECT_TRUE(disk->GetKeySize(key_, kRedisSortedint, &key_size).ok()); EXPECT_GE(key_size, approximate_size * estimation_factor_); EXPECT_LE(key_size, approximate_size / estimation_factor_); @@ -199,7 +199,7 @@ TEST_F(RedisDiskTest, StreamDisk) { EXPECT_TRUE(s.ok()); approximate_size += key_.size() + 8 + 8 + values[0].size() + values[1].size(); } - uint64_t key_size; + uint64_t key_size = 0; EXPECT_TRUE(disk->GetKeySize(key_, kRedisStream, &key_size).ok()); EXPECT_GE(key_size, approximate_size * estimation_factor_); EXPECT_LE(key_size, approximate_size / estimation_factor_); diff --git a/tests/cppunit/observer_or_unique_test.cc b/tests/cppunit/observer_or_unique_test.cc index 63e3d20b823..f127077b629 100644 --- a/tests/cppunit/observer_or_unique_test.cc +++ b/tests/cppunit/observer_or_unique_test.cc @@ -37,16 +37,16 @@ TEST(ObserverOrUniquePtr, Unique) { ASSERT_EQ(v, 0); } -TEST(CompositePtr, Observer) { +TEST(ObserverOrUniquePtr, Observer) { int v = 0; - Counter* c = nullptr; + std::unique_ptr c = nullptr; { ObserverOrUniquePtr observer(new Counter{&v}, ObserverOrUnique::Observer); ASSERT_EQ(v, 1); - c = observer.Get(); + c.reset(observer.Get()); } ASSERT_EQ(v, 1); - delete c; + c.reset(); ASSERT_EQ(v, 0); } diff --git a/tests/cppunit/parse_util.cc b/tests/cppunit/parse_util.cc index 07f494fb246..d0364b7b944 100644 --- a/tests/cppunit/parse_util.cc +++ b/tests/cppunit/parse_util.cc @@ -22,8 +22,8 @@ #include TEST(ParseUtil, TryParseInt) { - long long v; - const char *str = "12345hellooo", *end; + long long v = 0; + const char *str = "12345hellooo", *end = nullptr; std::tie(v, end) = *TryParseInt(str); ASSERT_EQ(v, 12345); diff --git a/tests/cppunit/status_test.cc b/tests/cppunit/status_test.cc index df080700927..4886fb0257b 100644 --- a/tests/cppunit/status_test.cc +++ b/tests/cppunit/status_test.cc @@ -77,7 +77,7 @@ TEST(StatusOr, Scalar) { } TEST(StatusOr, String) { - auto f = [](std::string x) -> StatusOr { + auto f = [](std::string x) -> StatusOr { // NOLINT if (x.size() > 10) { return {Status::NotOK, "string too long"}; } @@ -85,7 +85,7 @@ TEST(StatusOr, String) { return x + " hello"; }; - auto g = [f](std::string x) -> StatusOr { + auto g = [f](std::string x) -> StatusOr { // NOLINT if (x.size() < 5) { return {Status::NotOK, "string too short"}; } @@ -117,8 +117,8 @@ TEST(StatusOr, String) { } TEST(StatusOr, SharedPtr) { - struct A { - A(int *x) : x(x) { *x = 233; } + struct A { // NOLINT + explicit A(int *x) : x(x) { *x = 233; } ~A() { *x = 0; } int *x; diff --git a/tests/cppunit/string_reply_test.cc b/tests/cppunit/string_reply_test.cc index f0455bf3994..7604063fa13 100644 --- a/tests/cppunit/string_reply_test.cc +++ b/tests/cppunit/string_reply_test.cc @@ -31,9 +31,9 @@ class StringReplyTest : public testing::Test { } static void TearDownTestCase() { values.clear(); } static std::vector values; - virtual void SetUp() {} - virtual void TearDown() {} + void SetUp() override {} + void TearDown() override {} }; std::vector StringReplyTest::values; diff --git a/tests/cppunit/test_base.h b/tests/cppunit/test_base.h index cd8c4fec3f0..fb5190f68c6 100644 --- a/tests/cppunit/test_base.h +++ b/tests/cppunit/test_base.h @@ -27,7 +27,7 @@ #include "storage/redis_db.h" #include "types/redis_hash.h" -class TestBase : public testing::Test { +class TestBase : public testing::Test { // NOLINT protected: explicit TestBase() : config_(new Config()) { config_->db_dir = "testdb"; diff --git a/tests/cppunit/types/stream_test.cc b/tests/cppunit/types/stream_test.cc index 8c83d9d8ff4..2eba5a539be 100644 --- a/tests/cppunit/types/stream_test.cc +++ b/tests/cppunit/types/stream_test.cc @@ -24,7 +24,7 @@ #include "time_util.h" #include "types/redis_stream.h" -class RedisStreamTest : public TestBase { +class RedisStreamTest : public TestBase { // NOLINT public: void checkStreamEntryValues(const std::vector &got, const std::vector &expected) { EXPECT_EQ(got.size(), expected.size()); diff --git a/x.py b/x.py index f8d072fe898..0b0ac0d85e4 100755 --- a/x.py +++ b/x.py @@ -183,7 +183,7 @@ def clang_tidy(dir: str, jobs: Optional[int], clang_tidy_path: str, run_clang_ti if jobs is not None: options.append(f'-j{jobs}') - regexes = ['kvrocks/src/', 'kvrocks2redis/'] + regexes = ['kvrocks/src/', 'utils/kvrocks2redis/', 'tests/cppunit/'] options.append(f'-header-filter={"|".join(regexes)}')