From 15ee4332e3d8aadc18c695aa6de979ff30477325 Mon Sep 17 00:00:00 2001 From: Kenneth Jia <48558845+kenneth-jia@users.noreply.github.com> Date: Wed, 3 Jul 2024 15:35:09 +0800 Subject: [PATCH] Update CI and fix failures --- .clang-tidy | 7 +++ .github/workflows/kafka_api_bazel_build.yml | 2 +- .github/workflows/kafka_api_ci_tests.yml | 43 ++++++++++--------- .../workflows/kafka_api_demo_conan_build.yml | 2 +- CMakeLists.txt | 2 +- README.md | 2 +- examples/example_ProducerRecordHeaders.cc | 4 +- examples/kafka_auto_commit_consumer.cc | 1 + include/kafka/Log.h | 6 +-- include/kafka/addons/KafkaMetrics.h | 1 + scripts/start-local-kafka-cluster.py | 2 +- tests/integration/TestKafkaConsumer.cc | 2 - .../TestKafkaRecoverableProducer.cc | 8 ++-- tests/robustness/TestAdminClient.cc | 2 + tests/unit/TestBrokerMetadata.cc | 5 +++ tests/unit/TestKafkaMetrics.cc | 8 ++-- tests/utils/TestUtility.h | 2 +- tools/console_clients/KafkaConsoleConsumer.cc | 1 + 18 files changed, 59 insertions(+), 41 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 71c2b9b31..330655584 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -4,6 +4,7 @@ Checks: "*,\ -llvmlibc-restrict-system-libc-headers,\ -llvmlibc-callee-namespace,\ -llvmlibc-implementation-in-namespace,\ + -llvmlibc-inline-function-decl,\ -altera-*,\ -fuchsia-*,\ -google-readability-namespace-comments,\ @@ -14,6 +15,7 @@ Checks: "*,\ -modernize-deprecated-headers,\ -modernize-use-trailing-return-type,\ -modernize-concat-nested-namespaces,\ + -modernize-type-traits,\ -hicpp-special-member-functions,\ -hicpp-vararg,\ -hicpp-no-malloc,\ @@ -36,15 +38,20 @@ Checks: "*,\ -cppcoreguidelines-pro-type-union-access,\ -misc-non-private-member-variables-in-classes,\ -misc-no-recursion,\ + -misc-include-cleaner,\ -readability-magic-numbers,\ -readability-implicit-bool-conversion,\ -readability-braces-around-statements,\ -readability-isolate-declaration,\ -readability-identifier-length,\ -readability-function-cognitive-complexity,\ + -readability-avoid-nested-conditional-operator,\ -bugprone-unused-return-value,\ -bugprone-easily-swappable-parameters,\ -bugprone-exception-escape,\ + -bugprone-optional-value-conversion,\ -cert-err58-cpp,\ + -performance-avoid-endl,\ + -performance-enum-size,\ -clang-analyzer-optin.performance.Padding" diff --git a/.github/workflows/kafka_api_bazel_build.yml b/.github/workflows/kafka_api_bazel_build.yml index f32ec5959..1fb1a25df 100644 --- a/.github/workflows/kafka_api_bazel_build.yml +++ b/.github/workflows/kafka_api_bazel_build.yml @@ -9,7 +9,7 @@ on: env: KAFKA_SRC_LINK: https://archive.apache.org/dist/kafka/3.3.1/kafka_2.13-3.3.1.tgz CPU_CORE_NUM: 2 - LIBRDKAFKA_TAG: v2.0.2 + LIBRDKAFKA_TAG: v2.4.0 jobs: kafka-api-bazel-build: diff --git a/.github/workflows/kafka_api_ci_tests.yml b/.github/workflows/kafka_api_ci_tests.yml index 1ca2a5e40..ee952b289 100644 --- a/.github/workflows/kafka_api_ci_tests.yml +++ b/.github/workflows/kafka_api_ci_tests.yml @@ -9,7 +9,7 @@ on: env: KAFKA_SRC_LINK: https://archive.apache.org/dist/kafka/3.3.1/kafka_2.13-3.3.1.tgz CPU_CORE_NUM: 2 - LIBRDKAFKA_TAG: v2.0.2 + LIBRDKAFKA_TAG: v2.4.0 BUILD_SUB_DIR: builds/sub-build jobs: @@ -49,48 +49,48 @@ jobs: test-labels: unit|integration enable-ut-stubs: true - - os: ubuntu-22.04 + - os: ubuntu-24.04 build-cxx: g++ build-type: Release test-labels: robustness - - os: ubuntu-22.04 + - os: ubuntu-24.04 build-cxx: g++ build-type: Release cxx-standard: 14 test-labels: unit|integration - - os: ubuntu-22.04 + - os: ubuntu-24.04 build-cxx: g++ check-option: asan test-labels: unit|integration - - os: ubuntu-22.04 + - os: ubuntu-24.04 build-cxx: g++ check-option: tsan test-labels: unit|integration - - os: ubuntu-22.04 + - os: ubuntu-24.04 build-cxx: g++ check-option: ubsan test-labels: unit|integration - - os: ubuntu-22.04 + - os: ubuntu-24.04 build-cxx: clang++ test-labels: unit|integration generate-doc: true with-installation: true - - os: ubuntu-22.04 + - os: ubuntu-24.04 build-cxx: clang++ check-option: clang-tidy enable-ut-stubs: true - - os: ubuntu-20.04 + - os: ubuntu-22.04 build-cxx: g++ test-labels: unit|integration - - os: ubuntu-20.04 + - os: ubuntu-22.04 build-cxx: clang++ test-labels: robustness @@ -155,8 +155,7 @@ jobs: # 6. Install tools to generate document if [ ${GENERATE_DOC} ]; then - sudo apt install -y python3-pip - sudo pip3 install markdown + sudo apt install -y python3-markdown sudo apt install -y doxygen fi @@ -300,15 +299,17 @@ jobs: # Install googletest vcpkg install gtest - cp -v "C:\VCPKG\INSTALLED\x86-windows\lib\manual-link\gtest_main*" "C:\VCPKG\INSTALLED\x86-windows\lib\" - cp -v "C:\VCPKG\INSTALLED\x86-windows\lib\manual-link\gtest_main*" "C:\VCPKG\INSTALLED\x86-windows\lib\" + + cp -v "C:\VCPKG\INSTALLED\x64-windows\lib\manual-link\gtest_main*" "C:\VCPKG\INSTALLED\x64-windows\lib\" + cp -v "C:\VCPKG\INSTALLED\x64-windows\lib\manual-link\gtest_main*" "C:\VCPKG\INSTALLED\x64-windows\lib\" # Install boost headers/libraries vcpkg install boost-optional vcpkg install boost-algorithm vcpkg install boost-program-options - cp -v "C:\VCPKG\INSTALLED\x86-windows\lib\boost_program_options-vc140-mt.lib" "C:\VCPKG\INSTALLED\x86-windows\lib\boost_program_options.lib" + ls "C:\VCPKG\INSTALLED\x64-windows\lib" + cp -v "C:\VCPKG\INSTALLED\x64-windows\lib\boost_program_options-vc144-mt-x64-1_85.lib" "C:\VCPKG\INSTALLED\x64-windows\lib\boost_program_options.lib" # Install rapidjson vcpkg install rapidjson @@ -319,13 +320,13 @@ jobs: run: | cd $Env:BUILD_SUB_DIR - $Env:GTEST_ROOT='C:\VCPKG\INSTALLED\x86-windows\' - $Env:BOOST_ROOT='C:\VCPKG\INSTALLED\x86-windows\' - $Env:LIBRDKAFKA_INCLUDE_DIR='C:\VCPKG\INSTALLED\x86-windows\include\' - $Env:LIBRDKAFKA_LIBRARY_DIR='C:\VCPKG\INSTALLED\x86-windows\lib\' - $Env:RAPIDJSON_INCLUDE_DIRS='C:\VCPKG\INSTALLED\x86-windows\include\' + $Env:GTEST_ROOT='C:\VCPKG\INSTALLED\x64-windows\' + $Env:BOOST_ROOT='C:\VCPKG\INSTALLED\x64-windows\' + $Env:LIBRDKAFKA_INCLUDE_DIR='C:\VCPKG\INSTALLED\x64-windows\include\' + $Env:LIBRDKAFKA_LIBRARY_DIR='C:\VCPKG\INSTALLED\x64-windows\lib\' + $Env:RAPIDJSON_INCLUDE_DIRS='C:\VCPKG\INSTALLED\x64-windows\include\' - cmake -B ./ -A Win32 -S ../.. "-DCMAKE_TOOLCHAIN_FILE=C:/vcpkg/scripts/buildsystems/vcpkg.cmake" + cmake -B ./ -A x64 -S ../.. "-DCMAKE_TOOLCHAIN_FILE=C:/vcpkg/scripts/buildsystems/vcpkg.cmake" - name: Build run: | diff --git a/.github/workflows/kafka_api_demo_conan_build.yml b/.github/workflows/kafka_api_demo_conan_build.yml index 8d13a0cd4..ea3014fa1 100644 --- a/.github/workflows/kafka_api_demo_conan_build.yml +++ b/.github/workflows/kafka_api_demo_conan_build.yml @@ -28,7 +28,7 @@ jobs: - name: Prepare run: | - pip3 install conan==1.59.0 + pip3 install conan==1.64.1 - name: Build (non-windows) if: ${{!contains(matrix.os, 'windows')}} diff --git a/CMakeLists.txt b/CMakeLists.txt index 11f81aaeb..99022eee1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -7,7 +7,7 @@ if (NOT parent_directory) set(cppkafka_master_project ON) # Use Strict Options if ((CMAKE_CXX_COMPILER_ID STREQUAL "Clang") OR (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")) - add_compile_options("-Wall" "-Werror" "-Wextra" "-Wshadow" "-Wno-unused-result") + add_compile_options("-Wall" "-Werror" "-Wextra" "-Wshadow" "-Wno-unused-result" "-Wno-array-bounds") elseif (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") add_definitions(-D_CRT_SECURE_NO_WARNINGS) endif () diff --git a/README.md b/README.md index dd0f6a7bf..f42ed8a86 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ About the *Modern C++ Kafka API* The [modern-cpp-kafka API](http://opensource.morganstanley.com/modern-cpp-kafka/doxygen/annotated.html) is a layer of ***C++*** wrapper based on [librdkafka](https://github.com/confluentinc/librdkafka) (the ***C*** part only), with high quality, but more friendly to users. -- By now, [modern-cpp-kafka](https://github.com/morganstanley/modern-cpp-kafka) is compatible with [librdkafka v2.0.2](https://github.com/confluentinc/librdkafka/releases/tag/v2.0.2). +- By now, [modern-cpp-kafka](https://github.com/morganstanley/modern-cpp-kafka) is compatible with [librdkafka v2.4.0](https://github.com/confluentinc/librdkafka/releases/tag/v2.4.0). ``` diff --git a/examples/example_ProducerRecordHeaders.cc b/examples/example_ProducerRecordHeaders.cc index 64ee59d2e..412392579 100644 --- a/examples/example_ProducerRecordHeaders.cc +++ b/examples/example_ProducerRecordHeaders.cc @@ -1,5 +1,7 @@ -#include +#include +#include +#include #include #include diff --git a/examples/kafka_auto_commit_consumer.cc b/examples/kafka_auto_commit_consumer.cc index 79d5bdc85..dbc684a07 100644 --- a/examples/kafka_auto_commit_consumer.cc +++ b/examples/kafka_auto_commit_consumer.cc @@ -1,5 +1,6 @@ #include "kafka/KafkaConsumer.h" +#include #include #include diff --git a/include/kafka/Log.h b/include/kafka/Log.h index bbc0985dd..3cdfb5e6a 100644 --- a/include/kafka/Log.h +++ b/include/kafka/Log.h @@ -43,12 +43,12 @@ template class LogBuffer { public: - LogBuffer():_wptr(_buf.data()) { _buf[0] = 0; } // NOLINT + LogBuffer() { clear(); } LogBuffer& clear() { - _wptr = _buf.data(); _buf[0] = 0; + _wptr = _buf.data(); return *this; } @@ -72,7 +72,7 @@ class LogBuffer private: std::array _buf; - char* _wptr; + char* _wptr = nullptr; }; diff --git a/include/kafka/addons/KafkaMetrics.h b/include/kafka/addons/KafkaMetrics.h index 96fe3ce3a..400f8da4f 100644 --- a/include/kafka/addons/KafkaMetrics.h +++ b/include/kafka/addons/KafkaMetrics.h @@ -8,6 +8,7 @@ #include #include +#include #include #include #include diff --git a/scripts/start-local-kafka-cluster.py b/scripts/start-local-kafka-cluster.py index 6856460b3..a39c0dec7 100755 --- a/scripts/start-local-kafka-cluster.py +++ b/scripts/start-local-kafka-cluster.py @@ -157,7 +157,7 @@ def main(): cmd = 'lsof -nP -iTCP:{0} | grep LISTEN'.format(brokerPort) cmdCall = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) (out, err) = cmdCall.communicate(); - matched = re.search('[^\s-]+ +([0-9]+) +.*', out.decode('utf-8')) + matched = re.search(r'[^\s-]+ +([0-9]+) +.*', out.decode('utf-8')) if matched: kafkaBrokerPids.append(matched.group(1)) diff --git a/tests/integration/TestKafkaConsumer.cc b/tests/integration/TestKafkaConsumer.cc index 516454200..dc584dc8d 100644 --- a/tests/integration/TestKafkaConsumer.cc +++ b/tests/integration/TestKafkaConsumer.cc @@ -5,8 +5,6 @@ #include "gtest/gtest.h" -#include - #include #include #include diff --git a/tests/integration/TestKafkaRecoverableProducer.cc b/tests/integration/TestKafkaRecoverableProducer.cc index 43d5018c6..51bc90c3a 100644 --- a/tests/integration/TestKafkaRecoverableProducer.cc +++ b/tests/integration/TestKafkaRecoverableProducer.cc @@ -104,11 +104,11 @@ TEST(KafkaRecoverableProducer, MockFatalError) { auto toSend = messagesToSend.front(); { - std::lock_guard lock(messagesMutex); + const std::lock_guard lock(messagesMutex); messagesToSend.pop_front(); } - std::shared_ptr payload = std::make_shared(std::to_string(toSend)); + const std::shared_ptr payload = std::make_shared(std::to_string(toSend)); auto record = kafka::clients::producer::ProducerRecord(topic, partition, kafka::NullKey, kafka::Value(payload->c_str(), payload->size()), @@ -121,7 +121,7 @@ TEST(KafkaRecoverableProducer, MockFatalError) // Would resend the message if (error) { - std::lock_guard lock(messagesMutex); + const std::lock_guard lock(messagesMutex); messagesToSend.push_front(static_cast(std::stoi(*payload))); } @@ -156,7 +156,7 @@ TEST(KafkaRecoverableProducer, MockFatalError) std::map countMap; for (const auto& record: records) { - std::string payload(static_cast(record.value().data()), record.value().size()); + const std::string payload(static_cast(record.value().data()), record.value().size()); ++countMap[static_cast(std::stoi(payload))]; } diff --git a/tests/robustness/TestAdminClient.cc b/tests/robustness/TestAdminClient.cc index e38b775ff..4b7d06296 100755 --- a/tests/robustness/TestAdminClient.cc +++ b/tests/robustness/TestAdminClient.cc @@ -1,6 +1,8 @@ #include "../utils/TestUtility.h" #include "kafka/AdminClient.h" +#include "kafka/Types.h" +#include "kafka/Utility.h" #include "gtest/gtest.h" diff --git a/tests/unit/TestBrokerMetadata.cc b/tests/unit/TestBrokerMetadata.cc index 000e3bc0b..da1e66a26 100644 --- a/tests/unit/TestBrokerMetadata.cc +++ b/tests/unit/TestBrokerMetadata.cc @@ -1,7 +1,12 @@ #include "kafka/BrokerMetadata.h" +#include "kafka/Types.h" #include "gtest/gtest.h" +#include +#include +#include + TEST(BrokerMetadata, Node) { diff --git a/tests/unit/TestKafkaMetrics.cc b/tests/unit/TestKafkaMetrics.cc index ef4e44a9b..0fc4b97bc 100644 --- a/tests/unit/TestKafkaMetrics.cc +++ b/tests/unit/TestKafkaMetrics.cc @@ -332,7 +332,7 @@ TEST(KafkaMetrics, FailureCases) const kafka::KafkaMetrics invalidMetrics("{invalid: 3}"); EXPECT_FALSE(true); } - catch (const std::runtime_error& e) {} + catch (const std::runtime_error& e) { std::cout << "Exception std::runtime_error(" << e.what() << " caught as expected!" << std::endl; } catch (...) { EXPECT_FALSE(true); } const kafka::KafkaMetrics metrics(consumerMetricsSample); @@ -343,7 +343,7 @@ TEST(KafkaMetrics, FailureCases) metrics.getInt({"*", "127.0.0.1:29003/2", "stateage"}); EXPECT_FALSE(true); } - catch (const std::invalid_argument& e) {} + catch (const std::invalid_argument& e) { std::cout << "Exception std::invalid_argument(" << e.what() << ") caught as expected!" << std::endl; } catch (...) { EXPECT_FALSE(true); } // Try invalid inputs (end with "*") @@ -352,7 +352,7 @@ TEST(KafkaMetrics, FailureCases) metrics.getInt({"brokers", "127.0.0.1:29003/2", "*"}); EXPECT_FALSE(true); } - catch (const std::invalid_argument& e) {} + catch (const std::invalid_argument& e) { std::cout << "Exception std::invalid_argument(" << e.what() << ") caught as expected!" << std::endl; } catch (...) { EXPECT_FALSE(true); } // Try invalid inputs (no keys) @@ -361,7 +361,7 @@ TEST(KafkaMetrics, FailureCases) metrics.getInt({}); EXPECT_FALSE(true); } - catch (const std::invalid_argument& e) {} + catch (const std::invalid_argument& e) { std::cout << "Exception std::invalid_argument(" << e.what() << ") caught as expected!" << std::endl; } catch (...) { EXPECT_FALSE(true); } // Try non-exist keys diff --git a/tests/utils/TestUtility.h b/tests/utils/TestUtility.h index 932ee8d56..dd6c49c28 100644 --- a/tests/utils/TestUtility.h +++ b/tests/utils/TestUtility.h @@ -208,7 +208,7 @@ CreateKafkaTopic(const kafka::Topic& topic, int numPartitions, int replicationFa class JoiningThread { public: template - explicit JoiningThread(F&& f, Args&&... args): _t(f, args...) {} + explicit JoiningThread(F&& f, Args&&... args): _t(std::forward(f), args...) {} ~JoiningThread() { if (_t.joinable()) _t.join(); } private: std::thread _t; diff --git a/tools/console_clients/KafkaConsoleConsumer.cc b/tools/console_clients/KafkaConsoleConsumer.cc index 3bf71e0e4..e6add97e5 100644 --- a/tools/console_clients/KafkaConsoleConsumer.cc +++ b/tools/console_clients/KafkaConsoleConsumer.cc @@ -6,6 +6,7 @@ #include #include +#include #include #include #include