Skip to content
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

Add clang-tidy checking for unit tests #1337

Merged
merged 4 commits into from
Mar 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .clang-tidy
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/server/tls_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<const Config *>(cfg)->tls_key_file_pass.c_str(), size);
buf[size - 1] = '\0';
return strlen(buf);
return static_cast<int>(strlen(buf));
});
}

Expand Down
4 changes: 2 additions & 2 deletions tests/cppunit/cluster_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ TEST(Cluster, CluseterGetNodes) {

std::vector<std::string> vnodes = Util::Split(output_nodes, "\n");

for (unsigned i = 0; i < vnodes.size(); i++) {
std::vector<std::string> node_fields = Util::Split(vnodes[i], " ");
for (const auto &vnode : vnodes) {
std::vector<std::string> node_fields = Util::Split(vnode, " ");

if (node_fields[0] == "07c37dfeb235213a872192d90877d0cd55635b91") {
ASSERT_TRUE(node_fields.size() == 8);
Expand Down
2 changes: 1 addition & 1 deletion tests/cppunit/cron_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
8 changes: 4 additions & 4 deletions tests/cppunit/disk_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_);
Expand All @@ -172,13 +172,13 @@ TEST_F(RedisDiskTest, SortedintDisk) {
std::unique_ptr<Redis::Sortedint> sortedint = std::make_unique<Redis::Sortedint>(storage_, "disk_ns_sortedint");
std::unique_ptr<Redis::Disk> disk = std::make_unique<Redis::Disk>(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>{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_);
Expand All @@ -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_);
Expand Down
8 changes: 4 additions & 4 deletions tests/cppunit/observer_or_unique_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<Counter> c = nullptr;
{
ObserverOrUniquePtr<Counter> 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);
}
4 changes: 2 additions & 2 deletions tests/cppunit/parse_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
#include <parse_util.h>

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);
Expand Down
8 changes: 4 additions & 4 deletions tests/cppunit/status_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,15 @@ TEST(StatusOr, Scalar) {
}

TEST(StatusOr, String) {
auto f = [](std::string x) -> StatusOr<std::string> {
auto f = [](std::string x) -> StatusOr<std::string> { // NOLINT
if (x.size() > 10) {
return {Status::NotOK, "string too long"};
}

return x + " hello";
};

auto g = [f](std::string x) -> StatusOr<std::string> {
auto g = [f](std::string x) -> StatusOr<std::string> { // NOLINT
if (x.size() < 5) {
return {Status::NotOK, "string too short"};
}
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions tests/cppunit/string_reply_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ class StringReplyTest : public testing::Test {
}
static void TearDownTestCase() { values.clear(); }
static std::vector<std::string> values;
virtual void SetUp() {}

virtual void TearDown() {}
void SetUp() override {}
void TearDown() override {}
};

std::vector<std::string> StringReplyTest::values;
Expand Down
2 changes: 1 addition & 1 deletion tests/cppunit/test_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
2 changes: 1 addition & 1 deletion tests/cppunit/types/stream_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string> &got, const std::vector<std::string> &expected) {
EXPECT_EQ(got.size(), expected.size());
Expand Down
2 changes: 1 addition & 1 deletion x.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)}')

Expand Down