From 901c0b02ccb433ae82db86210ed164dae797ad5f Mon Sep 17 00:00:00 2001 From: Torsten Kilias Date: Wed, 23 Sep 2026 00:38:26 +0200 Subject: [PATCH 01/11] docs: add public v2 coding style guide --- doc/developer_guide/v2/v2.md | 1 + doc/developer_guide/v2/v2_coding_style.md | 86 +++++++++++++++++++++++ 2 files changed, 87 insertions(+) create mode 100644 doc/developer_guide/v2/v2_coding_style.md diff --git a/doc/developer_guide/v2/v2.md b/doc/developer_guide/v2/v2.md index 7a51f04..c6099f1 100644 --- a/doc/developer_guide/v2/v2.md +++ b/doc/developer_guide/v2/v2.md @@ -4,6 +4,7 @@ The v2 developer documentation is split into focused guides: - [Build and test](v2_build_and_test.md) — Bazel setup and test execution. - [Code quality](v2_code_quality.md) — clang-tidy and clang-format checks. +- [Coding style](v2_coding_style.md) — public C++ conventions for v2 code. - [Fuzzing](v2_fuzzing.md) — libFuzzer targets, Nox campaigns, and regression runs. - [Dependency policy](v2_dependency_policy.md) — third-party isolation and diff --git a/doc/developer_guide/v2/v2_coding_style.md b/doc/developer_guide/v2/v2_coding_style.md new file mode 100644 index 0000000..2d65ac7 --- /dev/null +++ b/doc/developer_guide/v2/v2_coding_style.md @@ -0,0 +1,86 @@ +# v2 C++ Coding Style + +This guide defines the conventions for new and substantially changed C++ code +under [`udf-runner-cpp/v2`](../../../udf-runner-cpp/v2). The checked-in +[`clang-format` configuration](../../../udf-runner-cpp/v2/tools/clang-format/.clang-format) +and [`clang-tidy` configuration](../../../udf-runner-cpp/v2/tools/clang-tidy/.clang-tidy) +are authoritative for automatically checked rules. + +## Files and includes + +- Use UTF-8 source files. +- Use `.cc` for implementation files and `.h` or `.hpp` for headers, matching + the convention already used by the v2 module. +- Keep `#include` directives at the top of the file. Do not include headers + inside functions unless there is a documented, compelling reason. +- Include every header required by a file directly; do not rely on transitive + includes. +- Put non-template function definitions in implementation files unless there + is a measured performance reason to keep them inline. +- Keep public headers independent of private implementation details and avoid + conditional compilation in headers unless it is required by the public API. + +## Names and namespaces + +- Use ASCII identifiers and lower camel case for functions, variables, + parameters, and data members. +- Use upper camel case for classes and enum types. Use upper camel case for + scoped enum values as well. +- Name factory functions with a `create` prefix and getters/setters with + `get`/`set` prefixes. +- Put file-local functions and types in an unnamed namespace. +- Put project code in an appropriate `exasol::udf::v2` namespace rather than + importing a namespace with `using namespace`. +- Keep namespace aliases local and descriptive when they improve readability. + +## Functions and classes + +- Prefer free functions for behavior that does not depend on object state. +- Avoid operator overloading unless the type has a clear value-like meaning + and the overload is required for natural use of the public API. +- Mark a class `final` when it is not designed for inheritance. +- Mark overriding methods with `override`. +- Keep class declarations ordered, where practical, as public, protected, then + private; within each section, place types before methods and data members. +- Avoid ambiguity between constructor parameters and members. Use distinct, + descriptive parameter names or qualify member access explicitly. +- Separate function definitions with a blank line. + +## Types, control flow, and errors + +- Prefer fixed-width integer types such as `std::int32_t` and `std::uint64_t` + when the width is part of the interface or serialized representation. +- Use `enum class` for new enumerations. +- For `std::optional`, use `has_value()` when testing presence and `value()` + when explicitly retrieving the contained value. Name the variable after its + value, not after the fact that it is optional. +- Follow the repository formatter for braces and indentation. Keep all code + belonging to a `case`, including its terminating `break`, `return`, or + fallthrough marker, inside the case body when braces are needed. +- Prefer safe, expressive casts. If a lower-level cast is required for a + measured hot path or ABI boundary, document why it is safe. +- Report failures caused by external input or environment through the public + error mechanism, normally an exception. Use assertions for programmer + contract violations and impossible internal states. + +## Documentation and cleanup + +- Document design decisions close to the code they constrain. +- Put API documentation in public headers and implementation details near the + implementation. +- Use Doxygen commands with `@`. Prefer `@returns`, `@throws`, and `@see`. + Omit `@brief` when the first sentence already provides the brief. +- Remove commented-out code and avoid `#if 0` or `#if 1` except when a clear, + documented temporary or compatibility purpose requires it. +- Keep comments factual and explain why non-obvious code exists, not what an + immediately readable statement does. + +## Tests and review + +- Add or update tests when changing behavior, public interfaces, parsing, + serialization, concurrency, or dependency boundaries. +- Prefer small, focused tests that make failures easy to diagnose. +- Run the v2 build and tests, then the `clang-format` and `clang-tidy` checks + described in the [code quality guide](v2_code_quality.md). +- Do not suppress a static-analysis warning without documenting the reason at + the suppression site. From d6914e0d2c5c165fb4df66984f9f3a1538ac9c71 Mon Sep 17 00:00:00 2001 From: Torsten Kilias Date: Wed, 23 Sep 2026 01:45:06 +0200 Subject: [PATCH 02/11] docs: enforce v2 C++ naming style --- doc/developer_guide/v2/v2_coding_style.md | 26 +- udf-runner-cpp/v2/arrow_c_data_demo.cc | 25 +- udf-runner-cpp/v2/arrow_c_data_demo_test.cc | 8 +- udf-runner-cpp/v2/call_metadata_fuzz_test.cc | 9 +- .../v2/connection_information_fuzz_test.cc | 10 +- .../v2/export_specification_fuzz_test.cc | 10 +- udf-runner-cpp/v2/frame_fuzz_test.cc | 8 +- .../v2/import_specification_fuzz_test.cc | 10 +- .../include/exasol/udf/v2/waitable_queue.hpp | 44 +- .../v2/json_schema_symbol_leak_test.cc | 8 +- .../v2/moodycamel_symbol_leak_test.cc | 8 +- udf-runner-cpp/v2/queue_fuzz_test.cc | 426 ++++++++++-------- .../v2/test_utils/json_schema_fuzzing.hpp | 83 ++-- .../v2/tools/clang-tidy/.clang-tidy | 19 + udf-runner-cpp/v2/udf_protocol.cc | 2 +- udf-runner-cpp/v2/udf_protocol.hpp | 2 +- udf-runner-cpp/v2/udf_protocol_test.cc | 3 +- udf-runner-cpp/v2/waitable_queue_benchmark.cc | 36 +- 18 files changed, 411 insertions(+), 326 deletions(-) diff --git a/doc/developer_guide/v2/v2_coding_style.md b/doc/developer_guide/v2/v2_coding_style.md index 2d65ac7..cfd7556 100644 --- a/doc/developer_guide/v2/v2_coding_style.md +++ b/doc/developer_guide/v2/v2_coding_style.md @@ -1,7 +1,7 @@ # v2 C++ Coding Style -This guide defines the conventions for new and substantially changed C++ code -under [`udf-runner-cpp/v2`](../../../udf-runner-cpp/v2). The checked-in +This guide defines the conventions for C++ code under +[`udf-runner-cpp/v2`](../../../udf-runner-cpp/v2). The checked-in [`clang-format` configuration](../../../udf-runner-cpp/v2/tools/clang-format/.clang-format) and [`clang-tidy` configuration](../../../udf-runner-cpp/v2/tools/clang-tidy/.clang-tidy) are authoritative for automatically checked rules. @@ -22,17 +22,25 @@ are authoritative for automatically checked rules. ## Names and namespaces -- Use ASCII identifiers and lower camel case for functions, variables, - parameters, and data members. -- Use upper camel case for classes and enum types. Use upper camel case for - scoped enum values as well. +- Use ASCII identifiers and `lower_case` for functions, variables, parameters, + and data members. +- Use `CamelCase` for classes and enum types. Use `CamelCase` for scoped enum + values as well. - Name factory functions with a `create` prefix and getters/setters with - `get`/`set` prefixes. + `get_`/`set_` prefixes, for example `get_value()` and `set_value()`. - Put file-local functions and types in an unnamed namespace. - Put project code in an appropriate `exasol::udf::v2` namespace rather than importing a namespace with `using namespace`. - Keep namespace aliases local and descriptive when they improve readability. +The naming policy is enforced by clang-tidy’s +`readability-identifier-naming` check. Class members have no naming prefix or +suffix. When a member access would otherwise be ambiguous, qualify it with +`this->`, for example `this->value`. + +Names required by an external ABI or framework are exceptions. For example, +the libFuzzer entry point `LLVMFuzzerTestOneInput` keeps its required spelling. + ## Functions and classes - Prefer free functions for behavior that does not depend on object state. @@ -42,8 +50,8 @@ are authoritative for automatically checked rules. - Mark overriding methods with `override`. - Keep class declarations ordered, where practical, as public, protected, then private; within each section, place types before methods and data members. -- Avoid ambiguity between constructor parameters and members. Use distinct, - descriptive parameter names or qualify member access explicitly. +- Avoid ambiguity between constructor parameters and members. Prefer the same + descriptive name and qualify member access with `this->`. - Separate function definitions with a blank line. ## Types, control flow, and errors diff --git a/udf-runner-cpp/v2/arrow_c_data_demo.cc b/udf-runner-cpp/v2/arrow_c_data_demo.cc index 37777cd..61c7d59 100644 --- a/udf-runner-cpp/v2/arrow_c_data_demo.cc +++ b/udf-runner-cpp/v2/arrow_c_data_demo.cc @@ -18,12 +18,12 @@ namespace thread_local std::string g_last_error; -void SetLastError(const arrow::Status& status) +void set_last_error(const arrow::Status& status) { g_last_error = status.ToString(); } -arrow::Result> MakeDemoRecordBatch() +arrow::Result> make_demo_record_batch() { arrow::Int64Builder id_builder; arrow::StringBuilder name_builder; @@ -51,7 +51,7 @@ arrow::Result> MakeDemoRecordBatch() return arrow::RecordBatch::Make(schema, num_rows, {std::move(ids), std::move(names)}); } -arrow::Status ExportDemoRecordBatch(ArrowArray* out_array, ArrowSchema* out_schema) +arrow::Status export_demo_record_batch(ArrowArray* out_array, ArrowSchema* out_schema) { if (out_array == nullptr || out_schema == nullptr) { @@ -62,7 +62,7 @@ arrow::Status ExportDemoRecordBatch(ArrowArray* out_array, ArrowSchema* out_sche std::memset(out_array, 0, sizeof(*out_array)); std::memset(out_schema, 0, sizeof(*out_schema)); - auto maybe_batch = MakeDemoRecordBatch(); + auto maybe_batch = make_demo_record_batch(); if (!maybe_batch.ok()) { return maybe_batch.status(); @@ -71,10 +71,10 @@ arrow::Status ExportDemoRecordBatch(ArrowArray* out_array, ArrowSchema* out_sche return arrow::Status::OK(); } -arrow::Status ConsumeDemoRecordBatch(ArrowArray* array, - ArrowSchema* schema, - int64_t* out_row_count, - int64_t* out_id_sum) +arrow::Status consume_demo_record_batch(ArrowArray* array, + ArrowSchema* schema, + int64_t* out_row_count, + int64_t* out_id_sum) { if (array == nullptr || schema == nullptr || out_row_count == nullptr || out_id_sum == nullptr) { @@ -111,10 +111,10 @@ arrow::Status ConsumeDemoRecordBatch(ArrowArray* array, extern "C" UDF_RUNNER_CPP_V2_EXPORT int udf_runner_cpp_v2_demo_export_record_batch( ArrowArray* out_array, ArrowSchema* out_schema) { - const arrow::Status status = ExportDemoRecordBatch(out_array, out_schema); + const arrow::Status status = export_demo_record_batch(out_array, out_schema); if (!status.ok()) { - SetLastError(status); + set_last_error(status); return 1; } g_last_error.clear(); @@ -124,10 +124,11 @@ extern "C" UDF_RUNNER_CPP_V2_EXPORT int udf_runner_cpp_v2_demo_export_record_bat extern "C" UDF_RUNNER_CPP_V2_EXPORT int udf_runner_cpp_v2_demo_consume_record_batch( ArrowArray* array, ArrowSchema* schema, int64_t* out_row_count, int64_t* out_id_sum) { - const arrow::Status status = ConsumeDemoRecordBatch(array, schema, out_row_count, out_id_sum); + const arrow::Status status = + consume_demo_record_batch(array, schema, out_row_count, out_id_sum); if (!status.ok()) { - SetLastError(status); + set_last_error(status); return 1; } g_last_error.clear(); diff --git a/udf-runner-cpp/v2/arrow_c_data_demo_test.cc b/udf-runner-cpp/v2/arrow_c_data_demo_test.cc index d40584e..ed72381 100644 --- a/udf-runner-cpp/v2/arrow_c_data_demo_test.cc +++ b/udf-runner-cpp/v2/arrow_c_data_demo_test.cc @@ -25,8 +25,8 @@ using export_fn_t = int (*)(ArrowArray*, ArrowSchema*); using consume_fn_t = int (*)(ArrowArray*, ArrowSchema*, int64_t*, int64_t*); using error_fn_t = const char* (*)(); -constexpr std::string_view kArrowMangledPrefix = "_ZN5arrow"; -constexpr std::string_view kDemoExportedPrefix = "udf_runner_cpp_v2_demo_"; +constexpr std::string_view arrow_mangled_prefix = "_ZN5arrow"; +constexpr std::string_view demo_exported_prefix = "udf_runner_cpp_v2_demo_"; [[noreturn]] void fail(const std::string& message) { @@ -140,11 +140,11 @@ void verify_symbols(const std::string& library_path) const std::string name = read_string(file, string_table.sh_offset + symbol.st_name, string_table.sh_size - symbol.st_name); - if (name.compare(0, kArrowMangledPrefix.size(), kArrowMangledPrefix) == 0) + if (name.compare(0, arrow_mangled_prefix.size(), arrow_mangled_prefix) == 0) { fail("shared library exports an Arrow C++ symbol: " + name); } - if (name.compare(0, kDemoExportedPrefix.size(), kDemoExportedPrefix) == 0) + if (name.compare(0, demo_exported_prefix.size(), demo_exported_prefix) == 0) { found_demo_symbol = true; } diff --git a/udf-runner-cpp/v2/call_metadata_fuzz_test.cc b/udf-runner-cpp/v2/call_metadata_fuzz_test.cc index 96b3a15..443758f 100644 --- a/udf-runner-cpp/v2/call_metadata_fuzz_test.cc +++ b/udf-runner-cpp/v2/call_metadata_fuzz_test.cc @@ -1,8 +1,7 @@ #include "test_utils/json_schema_fuzzing.hpp" -extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t *data, - std::size_t size) { - exasol::udf::v2::fuzzing::FuzzJsonSchema( - data, size, "json_schema/call_metadata.schema.json"); - return 0; +extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t* data, std::size_t size) +{ + exasol::udf::v2::fuzzing::fuzz_json_schema(data, size, "json_schema/call_metadata.schema.json"); + return 0; } diff --git a/udf-runner-cpp/v2/connection_information_fuzz_test.cc b/udf-runner-cpp/v2/connection_information_fuzz_test.cc index 6de5ffd..0de6437 100644 --- a/udf-runner-cpp/v2/connection_information_fuzz_test.cc +++ b/udf-runner-cpp/v2/connection_information_fuzz_test.cc @@ -1,8 +1,8 @@ #include "test_utils/json_schema_fuzzing.hpp" -extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t *data, - std::size_t size) { - exasol::udf::v2::fuzzing::FuzzJsonSchema( - data, size, "json_schema/connection_information.schema.json"); - return 0; +extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t* data, std::size_t size) +{ + exasol::udf::v2::fuzzing::fuzz_json_schema(data, size, + "json_schema/connection_information.schema.json"); + return 0; } diff --git a/udf-runner-cpp/v2/export_specification_fuzz_test.cc b/udf-runner-cpp/v2/export_specification_fuzz_test.cc index 18e4944..a25e52a 100644 --- a/udf-runner-cpp/v2/export_specification_fuzz_test.cc +++ b/udf-runner-cpp/v2/export_specification_fuzz_test.cc @@ -1,8 +1,8 @@ #include "test_utils/json_schema_fuzzing.hpp" -extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t *data, - std::size_t size) { - exasol::udf::v2::fuzzing::FuzzJsonSchema( - data, size, "json_schema/export_specification.schema.json"); - return 0; +extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t* data, std::size_t size) +{ + exasol::udf::v2::fuzzing::fuzz_json_schema(data, size, + "json_schema/export_specification.schema.json"); + return 0; } diff --git a/udf-runner-cpp/v2/frame_fuzz_test.cc b/udf-runner-cpp/v2/frame_fuzz_test.cc index d1e6419..d5f19d4 100644 --- a/udf-runner-cpp/v2/frame_fuzz_test.cc +++ b/udf-runner-cpp/v2/frame_fuzz_test.cc @@ -3,8 +3,8 @@ #include "udf_protocol.hpp" -extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t *data, - std::size_t size) { - exasol::udf::protocol::VerifyFrameBuffer(data, size); - return 0; +extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t* data, std::size_t size) +{ + exasol::udf::protocol::verify_frame_buffer(data, size); + return 0; } diff --git a/udf-runner-cpp/v2/import_specification_fuzz_test.cc b/udf-runner-cpp/v2/import_specification_fuzz_test.cc index 5efed0f..8fcbd56 100644 --- a/udf-runner-cpp/v2/import_specification_fuzz_test.cc +++ b/udf-runner-cpp/v2/import_specification_fuzz_test.cc @@ -1,8 +1,8 @@ #include "test_utils/json_schema_fuzzing.hpp" -extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t *data, - std::size_t size) { - exasol::udf::v2::fuzzing::FuzzJsonSchema( - data, size, "json_schema/import_specification.schema.json"); - return 0; +extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t* data, std::size_t size) +{ + exasol::udf::v2::fuzzing::fuzz_json_schema(data, size, + "json_schema/import_specification.schema.json"); + return 0; } diff --git a/udf-runner-cpp/v2/include/exasol/udf/v2/waitable_queue.hpp b/udf-runner-cpp/v2/include/exasol/udf/v2/waitable_queue.hpp index 4f3ffca..f06f26d 100644 --- a/udf-runner-cpp/v2/include/exasol/udf/v2/waitable_queue.hpp +++ b/udf-runner-cpp/v2/include/exasol/udf/v2/waitable_queue.hpp @@ -28,18 +28,18 @@ class WaitableQueue public: using queue_type = Queue; - WaitableQueue() : notification_fd_(::eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC)) + WaitableQueue() : notification_fd(::eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC)) { - if (notification_fd_ == -1) + if (notification_fd == -1) { throw std::system_error(errno, std::generic_category(), "eventfd"); } } explicit WaitableQueue(Queue queue) - : queue_(std::move(queue)), notification_fd_(::eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC)) + : queue_storage(std::move(queue)), notification_fd(::eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC)) { - if (notification_fd_ == -1) + if (notification_fd == -1) { throw std::system_error(errno, std::generic_category(), "eventfd"); } @@ -47,9 +47,9 @@ class WaitableQueue ~WaitableQueue() { - if (notification_fd_ != -1) + if (notification_fd != -1) { - ::close(notification_fd_); + ::close(notification_fd); } } @@ -57,8 +57,8 @@ class WaitableQueue WaitableQueue& operator=(const WaitableQueue&) = delete; WaitableQueue(WaitableQueue&& other) noexcept - : queue_(std::move(other.queue_)), - notification_fd_(std::exchange(other.notification_fd_, -1)) + : queue_storage(std::move(other.queue_storage)), + notification_fd(std::exchange(other.notification_fd, -1)) { } @@ -66,25 +66,25 @@ class WaitableQueue { if (this != &other) { - if (notification_fd_ != -1) + if (notification_fd != -1) { - ::close(notification_fd_); + ::close(notification_fd); } - queue_ = std::move(other.queue_); - notification_fd_ = std::exchange(other.notification_fd_, -1); + queue_storage = std::move(other.queue_storage); + notification_fd = std::exchange(other.notification_fd, -1); } return *this; } [[nodiscard]] int native_handle() const noexcept { - return notification_fd_; + return notification_fd; } template [[nodiscard]] bool enqueue(T&& value) { - if (!queue_.enqueue(std::forward(value))) + if (!queue_storage.enqueue(std::forward(value))) { return false; } @@ -98,7 +98,7 @@ class WaitableQueue std::size_t enqueued = 0; for (; first != last; ++first) { - if (!queue_.enqueue(*first)) + if (!queue_storage.enqueue(*first)) { break; } @@ -114,7 +114,7 @@ class WaitableQueue template [[nodiscard]] bool try_dequeue(Output& value) { - return queue_.try_dequeue(value); + return queue_storage.try_dequeue(value); } // Drains all eventfd notifications and returns their accumulated count. @@ -126,7 +126,7 @@ class WaitableQueue for (;;) { std::uint64_t value = 0; - const ssize_t result = ::read(notification_fd_, &value, sizeof(value)); + const ssize_t result = ::read(notification_fd, &value, sizeof(value)); if (result == sizeof(value)) { total += value; @@ -150,11 +150,11 @@ class WaitableQueue Queue& queue() noexcept { - return queue_; + return queue_storage; } const Queue& queue() const noexcept { - return queue_; + return queue_storage; } private: @@ -163,7 +163,7 @@ class WaitableQueue constexpr std::uint64_t signal = 1; for (;;) { - const ssize_t result = ::write(notification_fd_, &signal, sizeof(signal)); + const ssize_t result = ::write(notification_fd, &signal, sizeof(signal)); if (result == sizeof(signal)) { return; @@ -186,8 +186,8 @@ class WaitableQueue } } - Queue queue_; - int notification_fd_; + Queue queue_storage; + int notification_fd; }; template diff --git a/udf-runner-cpp/v2/json_schema_symbol_leak_test.cc b/udf-runner-cpp/v2/json_schema_symbol_leak_test.cc index ab11565..927ffad 100644 --- a/udf-runner-cpp/v2/json_schema_symbol_leak_test.cc +++ b/udf-runner-cpp/v2/json_schema_symbol_leak_test.cc @@ -18,8 +18,8 @@ namespace isolated_nlohmann = exasol::udf::v2::third_party::nlohmann; namespace { -constexpr std::string_view kGlobalNamespacePrefix = "_ZN8nlohmann"; -constexpr std::string_view kIsolatedNamespacePrefix = "_ZN6exasol3udf2v211third_party8nlohmann"; +constexpr std::string_view global_namespace_prefix = "_ZN8nlohmann"; +constexpr std::string_view isolated_namespace_prefix = "_ZN6exasol3udf2v211third_party8nlohmann"; [[noreturn]] void fail(const std::string& message) { @@ -133,11 +133,11 @@ void verify_symbols(const std::string& library_path) const std::string name = read_string(file, string_table.sh_offset + symbol.st_name, string_table.sh_size - symbol.st_name); - if (name.compare(0, kGlobalNamespacePrefix.size(), kGlobalNamespacePrefix) == 0) + if (name.compare(0, global_namespace_prefix.size(), global_namespace_prefix) == 0) { fail("validator exports a global nlohmann symbol: " + name); } - if (name.compare(0, kIsolatedNamespacePrefix.size(), kIsolatedNamespacePrefix) == 0) + if (name.compare(0, isolated_namespace_prefix.size(), isolated_namespace_prefix) == 0) { found_isolated_symbol = true; } diff --git a/udf-runner-cpp/v2/moodycamel_symbol_leak_test.cc b/udf-runner-cpp/v2/moodycamel_symbol_leak_test.cc index c21e17c..b3e74d1 100644 --- a/udf-runner-cpp/v2/moodycamel_symbol_leak_test.cc +++ b/udf-runner-cpp/v2/moodycamel_symbol_leak_test.cc @@ -15,8 +15,8 @@ namespace { -constexpr std::string_view kGlobalNamespacePrefix = "_ZN10moodycamel"; -constexpr std::string_view kIsolatedNamespacePrefix = "_ZN6exasol3udf2v211third_party10moodycamel"; +constexpr std::string_view global_namespace_prefix = "_ZN10moodycamel"; +constexpr std::string_view isolated_namespace_prefix = "_ZN6exasol3udf2v211third_party10moodycamel"; [[noreturn]] void fail(const std::string& message) { @@ -97,11 +97,11 @@ void verify_symbols(const std::string& path) } const std::string name = read_string(file, string_table.sh_offset + symbol.st_name, string_table.sh_size - symbol.st_name); - if (name.starts_with(kGlobalNamespacePrefix)) + if (name.starts_with(global_namespace_prefix)) { fail("queue library exports a global moodycamel symbol: " + name); } - if (name.starts_with(kIsolatedNamespacePrefix)) + if (name.starts_with(isolated_namespace_prefix)) { found_isolated_symbol = true; } diff --git a/udf-runner-cpp/v2/queue_fuzz_test.cc b/udf-runner-cpp/v2/queue_fuzz_test.cc index 1a5989a..23936ea 100644 --- a/udf-runner-cpp/v2/queue_fuzz_test.cc +++ b/udf-runner-cpp/v2/queue_fuzz_test.cc @@ -14,145 +14,184 @@ #include #include -namespace { +namespace +{ constexpr std::size_t kMaxOperations = 32; -constexpr std::size_t kMaxBatchSize = 4; +constexpr std::size_t kMaxBatchSize = 4; -using Byte = std::byte; +using Byte = std::byte; using Value = std::uint64_t; -[[noreturn]] void fuzz_failure() { - // Queue corruption, loss, duplication, or reordering is a finding. Keep - // this independent from assert(), which may be disabled in fuzz builds. - std::abort(); +[[noreturn]] void fuzz_failure() +{ + // Queue corruption, loss, duplication, or reordering is a finding. Keep + // this independent from assert(), which may be disabled in fuzz builds. + std::abort(); } -class StartGate { +class StartGate +{ public: - explicit StartGate(const std::size_t participant_count) - : participant_count_(participant_count) {} + explicit StartGate(const std::size_t participant_count) : participant_count(participant_count) + { + } - void arrive_and_wait() { - ready_.fetch_add(1, std::memory_order::seq_cst); - while (!go_.load(std::memory_order::seq_cst)) { - std::this_thread::yield(); + void arrive_and_wait() + { + ready.fetch_add(1, std::memory_order::seq_cst); + while (!go.load(std::memory_order::seq_cst)) + { + std::this_thread::yield(); + } } - } - void release() { - while (ready_.load(std::memory_order::seq_cst) != participant_count_) { - std::this_thread::yield(); + void release() + { + while (ready.load(std::memory_order::seq_cst) != participant_count) + { + std::this_thread::yield(); + } + go.store(true, std::memory_order::seq_cst); } - go_.store(true, std::memory_order::seq_cst); - } private: - const std::size_t participant_count_; - std::atomic ready_{0}; - std::atomic go_{false}; + const std::size_t participant_count; + std::atomic ready{0}; + std::atomic go{false}; }; -Value make_value(const std::size_t producer, const std::size_t sequence) { - return (static_cast(producer) << 32U) | static_cast(sequence); +Value make_value(const std::size_t producer, const std::size_t sequence) +{ + return (static_cast(producer) << 32U) | static_cast(sequence); } -void verify_results(const std::vector> &produced, - const std::vector> &consumed, - const bool preserve_order) { - std::vector expected; - std::vector actual; - for (const auto &values : produced) { - expected.insert(expected.end(), values.begin(), values.end()); - } - for (const auto &values : consumed) { - actual.insert(actual.end(), values.begin(), values.end()); - } +void verify_results(const std::vector>& produced, + const std::vector>& consumed, + const bool preserve_order) +{ + std::vector expected; + std::vector actual; + for (const auto& values : produced) + { + expected.insert(expected.end(), values.begin(), values.end()); + } + for (const auto& values : consumed) + { + actual.insert(actual.end(), values.begin(), values.end()); + } - if (preserve_order) { - if (produced.size() != 1 || consumed.size() != 1 || expected != actual) { - fuzz_failure(); + if (preserve_order) + { + if (produced.size() != 1 || consumed.size() != 1 || expected != actual) + { + fuzz_failure(); + } + return; } - return; - } - std::ranges::sort(expected); - std::ranges::sort(actual); - if (expected != actual) { - fuzz_failure(); - } + std::ranges::sort(expected); + std::ranges::sort(actual); + if (expected != actual) + { + fuzz_failure(); + } } template -void produce_operation(Queue &queue, const Byte operation_byte, - const std::size_t producer, std::size_t &sequence, - std::vector &produced) { - const unsigned int operation = std::to_integer(operation_byte); - if constexpr (Waitable) { - if ((operation & 1U) != 0) { - std::array batch{}; - const std::size_t batch_size = 1 + ((operation >> 1U) % kMaxBatchSize); - for (std::size_t item = 0; item < batch_size; ++item) { - batch[item] = make_value(producer, sequence++); - } - const std::size_t enqueued = - queue.enqueue_batch(batch.begin(), batch.begin() + batch_size); - produced.insert(produced.end(), batch.begin(), batch.begin() + enqueued); - } else { - const Value value = make_value(producer, sequence++); - if (queue.enqueue(value)) { - produced.push_back(value); - } +void produce_operation(Queue& queue, + const Byte operation_byte, + const std::size_t producer, + std::size_t& sequence, + std::vector& produced) +{ + const unsigned int operation = std::to_integer(operation_byte); + if constexpr (Waitable) + { + if ((operation & 1U) != 0) + { + std::array batch{}; + const std::size_t batch_size = 1 + ((operation >> 1U) % kMaxBatchSize); + for (std::size_t item = 0; item < batch_size; ++item) + { + batch[item] = make_value(producer, sequence++); + } + const std::size_t enqueued = + queue.enqueue_batch(batch.begin(), batch.begin() + batch_size); + produced.insert(produced.end(), batch.begin(), batch.begin() + enqueued); + } + else + { + const Value value = make_value(producer, sequence++); + if (queue.enqueue(value)) + { + produced.push_back(value); + } + } } - - } else { - const Value value = make_value(producer, sequence++); - if (queue.enqueue(value)) { - produced.push_back(value); + else + { + const Value value = make_value(producer, sequence++); + if (queue.enqueue(value)) + { + produced.push_back(value); + } } - } } template -void produce(Queue &queue, StartGate &start_gate, - const std::span operations, const std::size_t producer, - std::vector &produced, - std::atomic &producers_remaining, - std::atomic &producers_done) { - start_gate.arrive_and_wait(); - std::size_t sequence = 0; - for (const Byte operation_byte : operations) { - produce_operation(queue, operation_byte, producer, - sequence, produced); - if ((std::to_integer(operation_byte) & 4U) != 0) { - std::this_thread::yield(); +void produce(Queue& queue, + StartGate& start_gate, + const std::span operations, + const std::size_t producer, + std::vector& produced, + std::atomic& producers_remaining, + std::atomic& producers_done) +{ + start_gate.arrive_and_wait(); + std::size_t sequence = 0; + for (const Byte operation_byte : operations) + { + produce_operation(queue, operation_byte, producer, sequence, produced); + if ((std::to_integer(operation_byte) & 4U) != 0) + { + std::this_thread::yield(); + } + } + if (producers_remaining.fetch_sub(1, std::memory_order::seq_cst) == 1) + { + producers_done.store(true, std::memory_order::seq_cst); } - } - if (producers_remaining.fetch_sub(1, std::memory_order::seq_cst) == 1) { - producers_done.store(true, std::memory_order::seq_cst); - } } template -bool consume_one(Queue &queue, std::vector &consumed, - const std::atomic &producers_done) { - if constexpr (Waitable) { - queue.drain_notifications(); - } - if (Value value = 0; queue.try_dequeue(value)) { - consumed.push_back(value); - return true; - } - return !producers_done.load(std::memory_order::seq_cst); +bool consume_one(Queue& queue, + std::vector& consumed, + const std::atomic& producers_done) +{ + if constexpr (Waitable) + { + queue.drain_notifications(); + } + if (Value value = 0; queue.try_dequeue(value)) + { + consumed.push_back(value); + return true; + } + return !producers_done.load(std::memory_order::seq_cst); } template -void consume(Queue &queue, StartGate &start_gate, std::vector &consumed, - const std::atomic &producers_done) { - start_gate.arrive_and_wait(); - while (consume_one(queue, consumed, producers_done)) { - std::this_thread::yield(); - } +void consume(Queue& queue, + StartGate& start_gate, + std::vector& consumed, + const std::atomic& producers_done) +{ + start_gate.arrive_and_wait(); + while (consume_one(queue, consumed, producers_done)) + { + std::this_thread::yield(); + } } template @@ -160,101 +199,110 @@ void run_queue(const std::type_identity, const std::bool_constant, const std::span operations, const std::size_t producer_count, - const std::size_t consumer_count, const bool preserve_order) { - Queue queue; - std::vector> produced(producer_count); - std::vector> consumed(consumer_count); - for (auto &values : produced) { - values.reserve(operations.size() * (Waitable ? kMaxBatchSize : 1)); - } - for (auto &values : consumed) { - values.reserve(operations.size() * producer_count * - (Waitable ? kMaxBatchSize : 1)); - } + const std::size_t consumer_count, + const bool preserve_order) +{ + Queue queue; + std::vector> produced(producer_count); + std::vector> consumed(consumer_count); + for (auto& values : produced) + { + values.reserve(operations.size() * (Waitable ? kMaxBatchSize : 1)); + } + for (auto& values : consumed) + { + values.reserve(operations.size() * producer_count * (Waitable ? kMaxBatchSize : 1)); + } - StartGate start_gate(producer_count + consumer_count); - std::atomic producers_done{false}; - std::atomic producers_remaining{producer_count}; - std::vector producers; - std::vector consumers; - producers.reserve(producer_count); - consumers.reserve(consumer_count); + StartGate start_gate(producer_count + consumer_count); + std::atomic producers_done{false}; + std::atomic producers_remaining{producer_count}; + std::vector producers; + std::vector consumers; + producers.reserve(producer_count); + consumers.reserve(consumer_count); - for (std::size_t producer = 0; producer < producer_count; ++producer) { - producers.emplace_back( - produce, std::ref(queue), std::ref(start_gate), - operations, producer, std::ref(produced[producer]), - std::ref(producers_remaining), std::ref(producers_done)); - } + for (std::size_t producer = 0; producer < producer_count; ++producer) + { + producers.emplace_back(produce, std::ref(queue), std::ref(start_gate), + operations, producer, std::ref(produced[producer]), + std::ref(producers_remaining), std::ref(producers_done)); + } - for (std::size_t consumer = 0; consumer < consumer_count; ++consumer) { - consumers.emplace_back(consume, std::ref(queue), - std::ref(start_gate), std::ref(consumed[consumer]), - std::cref(producers_done)); - } + for (std::size_t consumer = 0; consumer < consumer_count; ++consumer) + { + consumers.emplace_back(consume, std::ref(queue), std::ref(start_gate), + std::ref(consumed[consumer]), std::cref(producers_done)); + } - start_gate.release(); - for (auto &producer : producers) { - producer.join(); - } - for (auto &consumer : consumers) { - consumer.join(); - } + start_gate.release(); + for (auto& producer : producers) + { + producer.join(); + } + for (auto& consumer : consumers) + { + consumer.join(); + } - if constexpr (Waitable) { - // Notifications are hints rather than item counts. Drain any final - // counter value after all queue elements have been consumed. - queue.drain_notifications(); - } - // A consumer can observe an empty queue while another consumer is - // completing its final dequeue. Once all consumers have joined, there is no - // longer a race, so perform a final nonblocking drain before checking the - // accounting. This also makes termination independent of scheduling. - Value value = 0; - while (queue.try_dequeue(value)) { - consumed.front().push_back(value); - } - verify_results(produced, consumed, preserve_order); + if constexpr (Waitable) + { + // Notifications are hints rather than item counts. Drain any final + // counter value after all queue elements have been consumed. + queue.drain_notifications(); + } + // A consumer can observe an empty queue while another consumer is + // completing its final dequeue. Once all consumers have joined, there is no + // longer a race, so perform a final nonblocking drain before checking the + // accounting. This also makes termination independent of scheduling. + Value value = 0; + while (queue.try_dequeue(value)) + { + consumed.front().push_back(value); + } + verify_results(produced, consumed, preserve_order); } } // namespace -extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t *data, - const std::size_t size) { - if (size < 2) { - return 0; - } +extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t* data, const std::size_t size) +{ + if (size < 2) + { + return 0; + } - const std::size_t operation_count = std::min(kMaxOperations, size - 1); - const auto *input = reinterpret_cast(data); - const unsigned int input_byte = std::to_integer(input[0]); - const unsigned int mode = input_byte & 3U; - const unsigned int producer_bit = (input_byte >> 2U) & 1U; - const unsigned int consumer_bit = (input_byte >> 3U) & 1U; - const auto *operations = input + 1; - const std::span operation_span(operations, operation_count); + const std::size_t operation_count = std::min(kMaxOperations, size - 1); + const auto* input = reinterpret_cast(data); + const unsigned int input_byte = std::to_integer(input[0]); + const unsigned int mode = input_byte & 3U; + const unsigned int producer_bit = (input_byte >> 2U) & 1U; + const unsigned int consumer_bit = (input_byte >> 3U) & 1U; + const auto* operations = input + 1; + const std::span operation_span(operations, operation_count); - switch (mode) { - case 0: - run_queue(std::type_identity>{}, - std::bool_constant{}, operation_span, 1, 1, true); - break; - case 1: - run_queue(std::type_identity>{}, - std::bool_constant{}, operation_span, 2 + producer_bit, - 2 + consumer_bit, false); - break; - case 2: - run_queue(std::type_identity>{}, - std::bool_constant{}, operation_span, 1, 1, true); - break; - case 3: - run_queue(std::type_identity>{}, - std::bool_constant{}, operation_span, 2 + producer_bit, - 2 + consumer_bit, false); - break; - default: - fuzz_failure(); - } - return 0; + switch (mode) + { + case 0: + run_queue(std::type_identity>{}, + std::bool_constant{}, operation_span, 1, 1, true); + break; + case 1: + run_queue(std::type_identity>{}, + std::bool_constant{}, operation_span, 2 + producer_bit, + 2 + consumer_bit, false); + break; + case 2: + run_queue(std::type_identity>{}, + std::bool_constant{}, operation_span, 1, 1, true); + break; + case 3: + run_queue(std::type_identity>{}, + std::bool_constant{}, operation_span, 2 + producer_bit, + 2 + consumer_bit, false); + break; + default: + fuzz_failure(); + } + return 0; } diff --git a/udf-runner-cpp/v2/test_utils/json_schema_fuzzing.hpp b/udf-runner-cpp/v2/test_utils/json_schema_fuzzing.hpp index bfe0bbd..1c17f5c 100644 --- a/udf-runner-cpp/v2/test_utils/json_schema_fuzzing.hpp +++ b/udf-runner-cpp/v2/test_utils/json_schema_fuzzing.hpp @@ -10,61 +10,70 @@ #include -namespace exasol::udf::v2::fuzzing { +namespace exasol::udf::v2::fuzzing +{ namespace isolated_nlohmann = exasol::udf::v2::third_party::nlohmann; -using Json = isolated_nlohmann::json; -using JsonValidator = isolated_nlohmann::json_schema::json_validator; +using Json = isolated_nlohmann::json; +using JsonValidator = isolated_nlohmann::json_schema::json_validator; -class SchemaReadError final : public std::runtime_error { +class SchemaReadError final : public std::runtime_error +{ public: - using std::runtime_error::runtime_error; + using std::runtime_error::runtime_error; }; -inline Json ReadSchema(const char *path) { - std::ifstream input(path); - if (!input.good()) { - throw SchemaReadError(std::string("Unable to read JSON schema: ") + path); - } - return Json::parse(input); +inline Json read_schema(const char* path) +{ + std::ifstream input(path); + if (!input.good()) + { + throw SchemaReadError(std::string("Unable to read JSON schema: ") + path); + } + return Json::parse(input); } -class SchemaValidator { +class SchemaValidator +{ public: - explicit SchemaValidator(const char *root_schema_path) { - validator_.set_root_schema(ReadSchema(root_schema_path)); - } - - void Validate(const uint8_t *data, std::size_t size) const noexcept { - if (size == 0) { - return; + explicit SchemaValidator(const char* root_schema_path) + { + this->validator.set_root_schema(read_schema(root_schema_path)); } - try { - const auto input = - Json::parse(std::string(reinterpret_cast(data), size)); - validator_.validate(input); - } catch (const std::exception &exception) { - // Parse and schema-validation failures are expected input - // outcomes. Sanitizer findings and other process failures still - // terminate the fuzz target. - static_cast(exception); + void validate(const uint8_t* data, std::size_t size) const noexcept + { + if (size == 0) + { + return; + } + + try + { + const auto input = Json::parse(std::string(reinterpret_cast(data), size)); + this->validator.validate(input); + } + catch (const std::exception& exception) + { + // Parse and schema-validation failures are expected input + // outcomes. Sanitizer findings and other process failures still + // terminate the fuzz target. + static_cast(exception); + } } - } private: - JsonValidator validator_{ - [](const isolated_nlohmann::json_uri &, Json &schema) { + JsonValidator validator{[](const isolated_nlohmann::json_uri&, Json& schema) { // The import and export schemas are the only schemas with an external // reference, and both reference this schema. - schema = ReadSchema("json_schema/connection_information.schema.json"); - }}; + schema = read_schema("json_schema/connection_information.schema.json"); + }}; }; -inline void FuzzJsonSchema(const uint8_t *data, std::size_t size, - const char *schema_path) { - static const auto validator = std::make_unique(schema_path); - validator->Validate(data, size); +inline void fuzz_json_schema(const uint8_t* data, std::size_t size, const char* schema_path) +{ + static const auto validator = std::make_unique(schema_path); + validator->validate(data, size); } } // namespace exasol::udf::v2::fuzzing diff --git a/udf-runner-cpp/v2/tools/clang-tidy/.clang-tidy b/udf-runner-cpp/v2/tools/clang-tidy/.clang-tidy index ccc41d9..2c04585 100644 --- a/udf-runner-cpp/v2/tools/clang-tidy/.clang-tidy +++ b/udf-runner-cpp/v2/tools/clang-tidy/.clang-tidy @@ -463,6 +463,7 @@ Checks: > # -hicpp-no-malloc, # cppcoreguidelines-no-malloc # -hicpp-noexcept-move, # performance-noexcept-move-constructor # -hicpp-special-member-functions, # cppcoreguidelines-special-member-functions + # -hicpp-static-assert, # misc-static-assert # -hicpp-undelegated-constructor, # bugprone-undelegated-constructor # -hicpp-uppercase-literal-suffix, # readability-uppercase-literal-suffix @@ -558,6 +559,24 @@ CheckOptions: value: 'true' - key: readability-magic-numbers.IgnoredIntegerValues value: '1;2;3;4;10' + - key: readability-identifier-naming.ClassCase + value: CamelCase + - key: readability-identifier-naming.FunctionCase + value: lower_case + - key: readability-identifier-naming.FunctionIgnoredRegexp + value: '^(LLVMFuzzerTestOneInput)$' + - key: readability-identifier-naming.VariableCase + value: lower_case + - key: readability-identifier-naming.ParameterCase + value: lower_case + - key: readability-identifier-naming.ClassMemberCase + value: lower_case + - key: readability-identifier-naming.ClassMemberPrefix + value: '' + - key: readability-identifier-naming.ClassMemberSuffix + value: '' + - key: readability-identifier-naming.NamespaceCase + value: lower_case ExtraArgsBefore: - "-std=c++17" - "-xc++" diff --git a/udf-runner-cpp/v2/udf_protocol.cc b/udf-runner-cpp/v2/udf_protocol.cc index e122ae1..13d2882 100644 --- a/udf-runner-cpp/v2/udf_protocol.cc +++ b/udf-runner-cpp/v2/udf_protocol.cc @@ -3,7 +3,7 @@ namespace exasol::udf::protocol { -bool VerifyFrameBuffer(const void* data, std::size_t size) +bool verify_frame_buffer(const void* data, std::size_t size) { using IsolatedVerifier = exasol::udf::v2::third_party::flatbuffers::Verifier; IsolatedVerifier verifier(static_cast(data), size); diff --git a/udf-runner-cpp/v2/udf_protocol.hpp b/udf-runner-cpp/v2/udf_protocol.hpp index b98ed73..603a96f 100644 --- a/udf-runner-cpp/v2/udf_protocol.hpp +++ b/udf-runner-cpp/v2/udf_protocol.hpp @@ -13,7 +13,7 @@ namespace exasol::udf::protocol { -bool VerifyFrameBuffer(const void* data, std::size_t size); +bool verify_frame_buffer(const void* data, std::size_t size); } // namespace exasol::udf::protocol diff --git a/udf-runner-cpp/v2/udf_protocol_test.cc b/udf-runner-cpp/v2/udf_protocol_test.cc index 88b8d83..abcec30 100644 --- a/udf-runner-cpp/v2/udf_protocol_test.cc +++ b/udf-runner-cpp/v2/udf_protocol_test.cc @@ -12,7 +12,8 @@ int main() const auto frame = exasol::udf::protocol::CreateFrame(builder, 7, message); builder.Finish(frame); - assert(exasol::udf::protocol::VerifyFrameBuffer(builder.GetBufferPointer(), builder.GetSize())); + assert( + exasol::udf::protocol::verify_frame_buffer(builder.GetBufferPointer(), builder.GetSize())); const auto* decoded = exasol::udf::protocol::GetFrame(builder.GetBufferPointer()); assert(decoded->stream_id() == 7); assert(decoded->message()->open_call()->call_name()->str() == "example"); diff --git a/udf-runner-cpp/v2/waitable_queue_benchmark.cc b/udf-runner-cpp/v2/waitable_queue_benchmark.cc index bb74473..9d22d5c 100644 --- a/udf-runner-cpp/v2/waitable_queue_benchmark.cc +++ b/udf-runner-cpp/v2/waitable_queue_benchmark.cc @@ -40,7 +40,7 @@ struct TimedItem // Includes raw enqueue and dequeue only; this is the queue-operation baseline // for the waitable round-trip benchmark. -void BM_RawSpscRoundTrip(benchmark::State& state) +void bm_raw_spsc_round_trip(benchmark::State& state) { exasol::udf::v2::SpscQueue queue(1024); for (const auto iteration : state) @@ -56,7 +56,7 @@ void BM_RawSpscRoundTrip(benchmark::State& state) // Includes enqueue, eventfd notification draining, and dequeue. The eventfd // write is part of the measured round trip. -void BM_WaitableSpscRoundTrip(benchmark::State& state) +void bm_waitable_spsc_round_trip(benchmark::State& state) { exasol::udf::v2::WaitableSpscQueue queue(exasol::udf::v2::SpscQueue(1024)); for (const auto iteration : state) @@ -73,7 +73,7 @@ void BM_WaitableSpscRoundTrip(benchmark::State& state) // Includes wait_enqueue and dequeue on a non-full blocking SPSC queue. The // benchmark measures the uncontended fast path rather than intentional waits. -void BM_BlockingSpscRoundTrip(benchmark::State& state) +void bm_blocking_spsc_round_trip(benchmark::State& state) { exasol::udf::v2::SpscCircularBuffer queue(1024); for (const auto iteration : state) @@ -90,7 +90,7 @@ void BM_BlockingSpscRoundTrip(benchmark::State& state) // Enqueue-only benchmarks pause timing while removing the item so the queue // remains empty for the next iteration. The waitable case includes its // eventfd write; notification draining is cleanup and is not timed. -void BM_RawSpscEnqueueLatency(benchmark::State& state) +void bm_raw_spsc_enqueue_latency(benchmark::State& state) { exasol::udf::v2::SpscQueue queue(1024); for (const auto iteration : state) @@ -106,7 +106,7 @@ void BM_RawSpscEnqueueLatency(benchmark::State& state) state.SetItemsProcessed(state.iterations()); } -void BM_WaitableSpscEnqueueLatency(benchmark::State& state) +void bm_waitable_spsc_enqueue_latency(benchmark::State& state) { exasol::udf::v2::WaitableSpscQueue queue(exasol::udf::v2::SpscQueue(1024)); for (const auto iteration : state) @@ -123,7 +123,7 @@ void BM_WaitableSpscEnqueueLatency(benchmark::State& state) state.SetItemsProcessed(state.iterations()); } -void BM_BlockingSpscEnqueueLatency(benchmark::State& state) +void bm_blocking_spsc_enqueue_latency(benchmark::State& state) { exasol::udf::v2::SpscCircularBuffer queue(1024); for (const auto iteration : state) @@ -142,7 +142,7 @@ void BM_BlockingSpscEnqueueLatency(benchmark::State& state) // Raw and waitable batch benchmarks use the same batch sizes. The waitable // queue emits one eventfd notification after the entire batch, exposing how // batching amortizes notification overhead. -void BM_RawSpscBatch(benchmark::State& state) +void bm_raw_spsc_batch(benchmark::State& state) { const auto batch_size = static_cast(state.range(0)); const std::vector batch(batch_size, 1); @@ -165,7 +165,7 @@ void BM_RawSpscBatch(benchmark::State& state) state.SetItemsProcessed(state.iterations() * static_cast(batch_size)); } -void BM_WaitableSpscBatch(benchmark::State& state) +void bm_waitable_spsc_batch(benchmark::State& state) { const auto batch_size = static_cast(state.range(0)); const std::vector batch(batch_size, 1); @@ -190,7 +190,7 @@ void BM_WaitableSpscBatch(benchmark::State& state) // and dequeue. One item is outstanding at a time, so the result measures // wakeup latency rather than latency caused by queue backlog. The producer // handshake is outside the manually recorded interval. -void BM_WaitableSpscEpollLatency(benchmark::State& state) +void bm_waitable_spsc_epoll_latency(benchmark::State& state) { exasol::udf::v2::WaitableSpscQueue queue{exasol::udf::v2::SpscQueue(8)}; const int epoll_fd = ::epoll_create1(EPOLL_CLOEXEC); @@ -264,12 +264,12 @@ void BM_WaitableSpscEpollLatency(benchmark::State& state) } // namespace -BENCHMARK(BM_RawSpscRoundTrip); -BENCHMARK(BM_WaitableSpscRoundTrip); -BENCHMARK(BM_BlockingSpscRoundTrip); -BENCHMARK(BM_RawSpscEnqueueLatency); -BENCHMARK(BM_WaitableSpscEnqueueLatency); -BENCHMARK(BM_BlockingSpscEnqueueLatency); -BENCHMARK(BM_RawSpscBatch)->Args({1})->Args({8})->Args({64})->Args({256}); -BENCHMARK(BM_WaitableSpscBatch)->Args({1})->Args({8})->Args({64})->Args({256}); -BENCHMARK(BM_WaitableSpscEpollLatency)->UseManualTime(); +BENCHMARK(bm_raw_spsc_round_trip); +BENCHMARK(bm_waitable_spsc_round_trip); +BENCHMARK(bm_blocking_spsc_round_trip); +BENCHMARK(bm_raw_spsc_enqueue_latency); +BENCHMARK(bm_waitable_spsc_enqueue_latency); +BENCHMARK(bm_blocking_spsc_enqueue_latency); +BENCHMARK(bm_raw_spsc_batch)->Args({1})->Args({8})->Args({64})->Args({256}); +BENCHMARK(bm_waitable_spsc_batch)->Args({1})->Args({8})->Args({64})->Args({256}); +BENCHMARK(bm_waitable_spsc_epoll_latency)->UseManualTime(); From 4c2333f3e7e83eb7238dd63b600177c0d3b26473 Mon Sep 17 00:00:00 2001 From: Torsten Kilias Date: Wed, 23 Sep 2026 09:22:06 +0200 Subject: [PATCH 03/11] docs: track v2 coding style issue --- doc/changes/unreleased.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/changes/unreleased.md b/doc/changes/unreleased.md index 7f9cd9b..3acf681 100644 --- a/doc/changes/unreleased.md +++ b/doc/changes/unreleased.md @@ -18,6 +18,7 @@ n/a ## Internal +* #57: Defined and enforced public v2 C++ coding style * #51: Added agent and contributor guidance for v1/v2 development, SLC workflows, CI testing, and PR conventions * #56: Restructured the developer guide and synchronized agent guidance * Updated Poetry dependencies and added developer guide and added .gitignore From eb3845db1f36980c4d2c8060f51fcb0e09e25287 Mon Sep 17 00:00:00 2001 From: Torsten Kilias Date: Thu, 24 Sep 2026 19:33:30 +0200 Subject: [PATCH 04/11] fix: address coding style review feedback --- udf-runner-cpp/v2/arrow_c_data_demo_test.cc | 6 ++++++ udf-runner-cpp/v2/include/exasol/udf/v2/waitable_queue.hpp | 7 +++---- udf-runner-cpp/v2/sonar-project.properties | 2 +- udf-runner-cpp/v2/tools/clang-tidy/.clang-tidy | 7 ++++++- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/udf-runner-cpp/v2/arrow_c_data_demo_test.cc b/udf-runner-cpp/v2/arrow_c_data_demo_test.cc index ed72381..dbc62ca 100644 --- a/udf-runner-cpp/v2/arrow_c_data_demo_test.cc +++ b/udf-runner-cpp/v2/arrow_c_data_demo_test.cc @@ -227,6 +227,9 @@ int main(int argc, char** argv) assert(names->GetString(0) == "alpha"); assert(names->GetString(3) == "delta"); + assert(export_batch(nullptr, nullptr) != 0); + assert(std::string(last_error()).find("must not be null") != std::string::npos); + ArrowArray second_array{}; ArrowSchema second_schema{}; if (export_batch(&second_array, &second_schema) != 0) @@ -242,6 +245,9 @@ int main(int argc, char** argv) } assert(row_count == 4); assert(id_sum == 10); + + assert(consume_batch(nullptr, nullptr, nullptr, nullptr) != 0); + assert(std::string(last_error()).find("must not be null") != std::string::npos); } catch (const std::exception& error) { diff --git a/udf-runner-cpp/v2/include/exasol/udf/v2/waitable_queue.hpp b/udf-runner-cpp/v2/include/exasol/udf/v2/waitable_queue.hpp index f06f26d..6837f5e 100644 --- a/udf-runner-cpp/v2/include/exasol/udf/v2/waitable_queue.hpp +++ b/udf-runner-cpp/v2/include/exasol/udf/v2/waitable_queue.hpp @@ -28,7 +28,7 @@ class WaitableQueue public: using queue_type = Queue; - WaitableQueue() : notification_fd(::eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC)) + WaitableQueue() { if (notification_fd == -1) { @@ -36,8 +36,7 @@ class WaitableQueue } } - explicit WaitableQueue(Queue queue) - : queue_storage(std::move(queue)), notification_fd(::eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC)) + explicit WaitableQueue(Queue queue) : queue_storage(std::move(queue)) { if (notification_fd == -1) { @@ -187,7 +186,7 @@ class WaitableQueue } Queue queue_storage; - int notification_fd; + int notification_fd = ::eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC); }; template diff --git a/udf-runner-cpp/v2/sonar-project.properties b/udf-runner-cpp/v2/sonar-project.properties index 4698e97..1a3006b 100644 --- a/udf-runner-cpp/v2/sonar-project.properties +++ b/udf-runner-cpp/v2/sonar-project.properties @@ -5,7 +5,7 @@ sonar.projectKey=udf-runner-cpp # in v2 while excluding vendored third-party sources. sonar.sources=. sonar.exclusions=third_party/**,bazel-*/** -sonar.coverage.exclusions=**/*_fuzz_test.cc,**/json_schema_fuzzing.hpp +sonar.coverage.exclusions=**/*_test.cc,**/*_fuzz_test.cc,**/json_schema_fuzzing.hpp # Bazel's SonarQube coverage generator writes this generic coverage report. sonar.coverageReportPaths=bazel-out/_coverage/_coverage_report.dat diff --git a/udf-runner-cpp/v2/tools/clang-tidy/.clang-tidy b/udf-runner-cpp/v2/tools/clang-tidy/.clang-tidy index 2c04585..aa62fa2 100644 --- a/udf-runner-cpp/v2/tools/clang-tidy/.clang-tidy +++ b/udf-runner-cpp/v2/tools/clang-tidy/.clang-tidy @@ -552,7 +552,8 @@ Checks: > # -openmp-exception-escape, # -openmp-use-default-none, # -zircon-temporary-objects, -HeaderFilterRegex: "" +HeaderFilterRegex: '(^|/)(udf-runner-cpp/v2|_main)/.*' +ExcludeHeaderFilterRegex: '(^|/)(external|third_party)/.*' WarningsAsErrors: "*" CheckOptions: - key: hicpp-signed-bitwise.IgnorePositiveIntegerLiterals @@ -561,6 +562,10 @@ CheckOptions: value: '1;2;3;4;10' - key: readability-identifier-naming.ClassCase value: CamelCase + - key: readability-identifier-naming.EnumCase + value: CamelCase + - key: readability-identifier-naming.EnumConstantCase + value: CamelCase - key: readability-identifier-naming.FunctionCase value: lower_case - key: readability-identifier-naming.FunctionIgnoredRegexp From e60de7cf592052fd654743ad70b0f6921e2ec961 Mon Sep 17 00:00:00 2001 From: Torsten Kilias Date: Wed, 23 Sep 2026 10:40:30 +0200 Subject: [PATCH 05/11] test: add gtest support to v2 --- doc/developer_guide/v2/v2_build_and_test.md | 13 ++ doc/developer_guide/v2/v2_coding_style.md | 12 ++ udf-runner-cpp/v2/BUILD.bazel | 25 +++- udf-runner-cpp/v2/MODULE.bazel | 1 + udf-runner-cpp/v2/arrow_core_test.cc | 10 +- .../v2/json_schema_validation_test.cc | 79 ++++++------ udf-runner-cpp/v2/moodycamel_queues_test.cc | 42 +++--- udf-runner-cpp/v2/udf_protocol_test.cc | 14 +- udf-runner-cpp/v2/waitable_queue_test.cc | 122 +++++++----------- 9 files changed, 173 insertions(+), 145 deletions(-) diff --git a/doc/developer_guide/v2/v2_build_and_test.md b/doc/developer_guide/v2/v2_build_and_test.md index ed3b8cd..233ecb9 100644 --- a/doc/developer_guide/v2/v2_build_and_test.md +++ b/doc/developer_guide/v2/v2_build_and_test.md @@ -24,5 +24,18 @@ The test suite covers the FlatBuffers protocol, Arrow support, JSON schemas, queue implementations, and fuzz-target regression tests. For fuzzing-specific commands, see the [v2 fuzzing guide](v2_fuzzing.md). +## Benchmarks + +Build and run the waitable-queue benchmark with: + +```bash +cd udf-runner-cpp/v2 +bazel run //:waitable_queue_benchmark +``` + +Benchmark results are diagnostic measurements. CPU frequency, scheduler +activity, build mode, and system load can affect the results, so benchmarks +must not be used as deterministic pass/fail tests. + For static analysis and formatting checks, see the [v2 code-quality guide](v2_code_quality.md). diff --git a/doc/developer_guide/v2/v2_coding_style.md b/doc/developer_guide/v2/v2_coding_style.md index cfd7556..4e36142 100644 --- a/doc/developer_guide/v2/v2_coding_style.md +++ b/doc/developer_guide/v2/v2_coding_style.md @@ -88,6 +88,18 @@ the libFuzzer entry point `LLVMFuzzerTestOneInput` keeps its required spelling. - Add or update tests when changing behavior, public interfaces, parsing, serialization, concurrency, or dependency boundaries. - Prefer small, focused tests that make failures easy to diagnose. +- Write functional unit tests with GoogleTest `TEST` or `TEST_F` cases. Use + `ASSERT_*` for prerequisites and `EXPECT_*` for independent checks; use + `@googletest//:gtest_main` instead of a hand-written `main()`. +- GoogleMock is available through the GoogleTest dependency. Use it only to + verify meaningful interactions with collaborators, callbacks, or failure + boundaries. Do not add production abstractions solely to create a mock. +- Keep custom entry points for ELF inspection, dynamic-loading, include-order, + and other specialized tests where they make the test's purpose clearer. +- Use Google Benchmark for performance tests. Exclude setup and cleanup from + measured regions when appropriate, use `benchmark::DoNotOptimize` for values + that must remain observable, and do not make benchmarks depend on fixed + timing thresholds or a particular machine. - Run the v2 build and tests, then the `clang-format` and `clang-tidy` checks described in the [code quality guide](v2_code_quality.md). - Do not suppress a static-analysis warning without documenting the reason at diff --git a/udf-runner-cpp/v2/BUILD.bazel b/udf-runner-cpp/v2/BUILD.bazel index 4b1a309..921a9fc 100644 --- a/udf-runner-cpp/v2/BUILD.bazel +++ b/udf-runner-cpp/v2/BUILD.bazel @@ -97,7 +97,10 @@ cc_test( name = "udf_protocol_test", srcs = ["udf_protocol_test.cc"], copts = ["-std=c++20"], - deps = [":udf_protocol"], + deps = [ + ":udf_protocol", + "@googletest//:gtest_main", + ], ) cc_binary( @@ -179,7 +182,10 @@ cc_test( name = "arrow_core_test", srcs = ["arrow_core_test.cc"], copts = ["-std=c++20"], - deps = [":arrow_core"], + deps = [ + ":arrow_core", + "@googletest//:gtest_main", + ], ) cc_test( @@ -205,7 +211,10 @@ cc_test( srcs = ["json_schema_validation_test.cc"], data = ["//json_schema:all_schemas"], copts = ["-std=c++17"], - deps = [":json_schema"], + deps = [ + ":json_schema", + "@googletest//:gtest_main", + ], ) cc_test( @@ -288,7 +297,10 @@ cc_test( name = "moodycamel_queues_test", srcs = ["moodycamel_queues_test.cc"], copts = ["-std=c++20"], - deps = [":moodycamel_queues"], + deps = [ + ":moodycamel_queues", + "@googletest//:gtest_main", + ], ) cc_binary( @@ -321,7 +333,10 @@ cc_test( name = "waitable_queue_test", srcs = ["waitable_queue_test.cc"], copts = ["-std=c++20"], - deps = [":waitable_queue"], + deps = [ + ":waitable_queue", + "@googletest//:gtest_main", + ], target_compatible_with = ["@platforms//os:linux"], ) diff --git a/udf-runner-cpp/v2/MODULE.bazel b/udf-runner-cpp/v2/MODULE.bazel index 05def17..a616297 100644 --- a/udf-runner-cpp/v2/MODULE.bazel +++ b/udf-runner-cpp/v2/MODULE.bazel @@ -6,6 +6,7 @@ module( bazel_dep(name = "rules_cc", version = "0.2.17") bazel_dep(name = "platforms", version = "1.0.0") bazel_dep(name = "flatbuffers", version = "25.2.10") +bazel_dep(name = "googletest", version = "1.15.0") bazel_dep(name = "google_benchmark", version = "1.9.5") bazel_dep(name = "rules_fuzzing", version = "0.8.0", dev_dependency = True) bazel_dep(name = "bazel_sonarqube", version = "1.0.5") diff --git a/udf-runner-cpp/v2/arrow_core_test.cc b/udf-runner-cpp/v2/arrow_core_test.cc index 0c2c207..94c1ed0 100644 --- a/udf-runner-cpp/v2/arrow_core_test.cc +++ b/udf-runner-cpp/v2/arrow_core_test.cc @@ -1,15 +1,15 @@ -#include #include #include #include +#include -int main() +TEST(ArrowCoreTest, BuildsInt64Array) { arrow::Int64Builder builder; - assert(builder.Append(int64_t{42}).ok()); + ASSERT_TRUE(builder.Append(int64_t{42}).ok()); std::shared_ptr array; - assert(builder.Finish(&array).ok()); - assert(array->length() == 1); + ASSERT_TRUE(builder.Finish(&array).ok()); + EXPECT_EQ(array->length(), 1); } diff --git a/udf-runner-cpp/v2/json_schema_validation_test.cc b/udf-runner-cpp/v2/json_schema_validation_test.cc index 6a57450..fd6391f 100644 --- a/udf-runner-cpp/v2/json_schema_validation_test.cc +++ b/udf-runner-cpp/v2/json_schema_validation_test.cc @@ -1,11 +1,9 @@ -#include -#include -#include #include #include #include #include +#include namespace isolated_nlohmann = exasol::udf::v2::third_party::nlohmann; @@ -15,49 +13,46 @@ namespace isolated_nlohmann::json read_json(const std::string& path) { std::ifstream input(path); - assert(input.good()); + if (!input.good()) + { + throw std::runtime_error("cannot open JSON schema: " + path); + } return isolated_nlohmann::json::parse(input); } } // namespace -int main() +TEST(JsonSchemaValidationTest, AcceptsValidImportSpecification) { - try - { - const auto import_schema = read_json("json_schema/import_specification.schema.json"); - isolated_nlohmann::json_schema::json_validator validator( - [](const isolated_nlohmann::json_uri&, isolated_nlohmann::json& schema) { - schema = read_json("json_schema/connection_information.schema.json"); - }); - validator.set_root_schema(import_schema); - - const isolated_nlohmann::json valid = { - {"is_subselect", true}, - {"connection_information", - { - {"kind", "JDBC"}, - {"address", "jdbc:example://host/database"}, - {"user", "user"}, - {"password", "secret"}, - }}, - }; - validator.validate(valid); - - bool rejected = false; - try - { - validator.validate(isolated_nlohmann::json::object()); - } - catch (const std::exception&) - { - rejected = true; - } - assert(rejected); - } - catch (const std::exception& error) - { - std::fprintf(stderr, "%s\n", error.what()); - return 1; - } + const auto import_schema = read_json("json_schema/import_specification.schema.json"); + isolated_nlohmann::json_schema::json_validator validator( + [](const isolated_nlohmann::json_uri&, isolated_nlohmann::json& schema) { + schema = read_json("json_schema/connection_information.schema.json"); + }); + validator.set_root_schema(import_schema); + + const isolated_nlohmann::json valid = { + {"is_subselect", true}, + {"connection_information", + { + {"kind", "JDBC"}, + {"address", "jdbc:example://host/database"}, + {"user", "user"}, + {"password", "secret"}, + }}, + }; + + EXPECT_NO_THROW(validator.validate(valid)); +} + +TEST(JsonSchemaValidationTest, RejectsInvalidImportSpecification) +{ + const auto import_schema = read_json("json_schema/import_specification.schema.json"); + isolated_nlohmann::json_schema::json_validator validator( + [](const isolated_nlohmann::json_uri&, isolated_nlohmann::json& schema) { + schema = read_json("json_schema/connection_information.schema.json"); + }); + validator.set_root_schema(import_schema); + + EXPECT_THROW(validator.validate(isolated_nlohmann::json::object()), std::exception); } diff --git a/udf-runner-cpp/v2/moodycamel_queues_test.cc b/udf-runner-cpp/v2/moodycamel_queues_test.cc index 2ac0378..91f95a1 100644 --- a/udf-runner-cpp/v2/moodycamel_queues_test.cc +++ b/udf-runner-cpp/v2/moodycamel_queues_test.cc @@ -1,28 +1,40 @@ -#include - #include #include -int main() +#include + +TEST(MoodycamelQueuesTest, SupportsSpscQueue) { exasol::udf::v2::SpscQueue spsc; - assert(spsc.enqueue(7)); + ASSERT_TRUE(spsc.enqueue(7)); int value = 0; - assert(spsc.try_dequeue(value)); - assert(value == 7); + ASSERT_TRUE(spsc.try_dequeue(value)); + EXPECT_EQ(value, 7); +} +TEST(MoodycamelQueuesTest, SupportsSpscCircularBuffer) +{ exasol::udf::v2::SpscCircularBuffer circular(2); - assert(circular.try_enqueue(8)); - assert(circular.try_dequeue(value)); - assert(value == 8); + ASSERT_TRUE(circular.try_enqueue(8)); + int value = 0; + ASSERT_TRUE(circular.try_dequeue(value)); + EXPECT_EQ(value, 8); +} +TEST(MoodycamelQueuesTest, SupportsMpmcQueue) +{ exasol::udf::v2::MpmcQueue mpmc; - assert(mpmc.enqueue(9)); - assert(mpmc.try_dequeue(value)); - assert(value == 9); + ASSERT_TRUE(mpmc.enqueue(9)); + int value = 0; + ASSERT_TRUE(mpmc.try_dequeue(value)); + EXPECT_EQ(value, 9); +} +TEST(MoodycamelQueuesTest, SupportsBlockingMpmcQueue) +{ exasol::udf::v2::BlockingMpmcQueue blocking; - assert(blocking.enqueue(10)); - assert(blocking.try_dequeue(value)); - assert(value == 10); + ASSERT_TRUE(blocking.enqueue(10)); + int value = 0; + ASSERT_TRUE(blocking.try_dequeue(value)); + EXPECT_EQ(value, 10); } diff --git a/udf-runner-cpp/v2/udf_protocol_test.cc b/udf-runner-cpp/v2/udf_protocol_test.cc index abcec30..c9aa19d 100644 --- a/udf-runner-cpp/v2/udf_protocol_test.cc +++ b/udf-runner-cpp/v2/udf_protocol_test.cc @@ -1,9 +1,8 @@ #include "udf_protocol.hpp" -#include -#include +#include -int main() +TEST(UdfProtocolTest, EncodesAndDecodesOpenCallFrame) { exasol::udf::v2::third_party::flatbuffers::FlatBufferBuilder builder; const auto call_name = builder.CreateString("example"); @@ -12,9 +11,12 @@ int main() const auto frame = exasol::udf::protocol::CreateFrame(builder, 7, message); builder.Finish(frame); - assert( + ASSERT_TRUE( exasol::udf::protocol::verify_frame_buffer(builder.GetBufferPointer(), builder.GetSize())); const auto* decoded = exasol::udf::protocol::GetFrame(builder.GetBufferPointer()); - assert(decoded->stream_id() == 7); - assert(decoded->message()->open_call()->call_name()->str() == "example"); + ASSERT_NE(decoded, nullptr); + EXPECT_EQ(decoded->stream_id(), 7); + ASSERT_NE(decoded->message(), nullptr); + ASSERT_NE(decoded->message()->open_call(), nullptr); + EXPECT_EQ(decoded->message()->open_call()->call_name()->str(), "example"); } diff --git a/udf-runner-cpp/v2/waitable_queue_test.cc b/udf-runner-cpp/v2/waitable_queue_test.cc index fad0d68..d420430 100644 --- a/udf-runner-cpp/v2/waitable_queue_test.cc +++ b/udf-runner-cpp/v2/waitable_queue_test.cc @@ -3,34 +3,22 @@ #include #include -#include #include -#include -#include #include -#include #include #include +#include namespace { -void test_check(bool condition, const char* message) -{ - if (!condition) - { - std::fprintf(stderr, "waitable queue test failure: %s\n", message); - std::abort(); - } -} - -void add_to_epoll(int epoll_fd, int fd, std::uint32_t events) +bool add_to_epoll(int epoll_fd, int fd, std::uint32_t events) { epoll_event event{}; event.events = events; event.data.fd = fd; - test_check(::epoll_ctl(epoll_fd, EPOLL_CTL_ADD, fd, &event) == 0, "epoll_ctl failed"); + return ::epoll_ctl(epoll_fd, EPOLL_CTL_ADD, fd, &event) == 0; } void close_pair(const std::array& sockets) @@ -41,66 +29,56 @@ void close_pair(const std::array& sockets) } // namespace -int main() +TEST(WaitableQueueTest, NotifiesEpollAndSupportsBatchOperations) { - try + exasol::udf::v2::WaitableSpscQueue queue; + const int epoll_fd = ::epoll_create1(EPOLL_CLOEXEC); + ASSERT_NE(epoll_fd, -1); + + std::array sockets{}; + ASSERT_EQ(::socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, sockets.data()), 0); + ASSERT_TRUE(add_to_epoll(epoll_fd, queue.native_handle(), EPOLLIN)); + ASSERT_TRUE(add_to_epoll(epoll_fd, sockets[1], EPOLLIN)); + + ASSERT_TRUE(queue.enqueue(42)); + const char byte = 'x'; + ASSERT_EQ(::write(sockets[0], &byte, sizeof(byte)), sizeof(byte)); + + std::array events{}; + const int event_count = ::epoll_wait(epoll_fd, events.data(), events.size(), 1000); + ASSERT_EQ(event_count, 2); + + bool queue_ready = false; + bool socket_ready = false; + for (const auto& event : std::span(events).first(static_cast(event_count))) { - exasol::udf::v2::WaitableSpscQueue queue; - const int epoll_fd = ::epoll_create1(EPOLL_CLOEXEC); - test_check(epoll_fd != -1, "epoll_create1 failed"); - - std::array sockets{}; - test_check(::socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, sockets.data()) == 0, - "socketpair failed"); - add_to_epoll(epoll_fd, queue.native_handle(), EPOLLIN); - add_to_epoll(epoll_fd, sockets[1], EPOLLIN); - - test_check(queue.enqueue(42), "queue enqueue failed"); - const char byte = 'x'; - test_check(::write(sockets[0], &byte, sizeof(byte)) == sizeof(byte), "socket write failed"); - - std::array events{}; - const int event_count = ::epoll_wait(epoll_fd, events.data(), events.size(), 1000); - test_check(event_count == 2, "epoll_wait did not report both descriptors"); - - bool queue_ready = false; - bool socket_ready = false; - for (const auto& event : std::span(events).first(static_cast(event_count))) - { - queue_ready |= event.data.fd == queue.native_handle(); - socket_ready |= event.data.fd == sockets[1]; - } - test_check(queue_ready, "queue descriptor was not ready"); - test_check(socket_ready, "socket descriptor was not ready"); - - test_check(queue.drain_notifications() == 1, "unexpected queue notification count"); - int value = 0; - test_check(queue.try_dequeue(value), "queue dequeue failed"); - test_check(value == 42, "unexpected dequeued value"); - - const std::vector batch{1, 2, 3}; - test_check(queue.enqueue_batch(batch.begin(), batch.end()) == batch.size(), - "batch enqueue failed"); - test_check(queue.drain_notifications() == 1, "unexpected batch notification count"); - for (int expected : batch) - { - test_check(queue.try_dequeue(value), "batch dequeue failed"); - test_check(value == expected, "unexpected batch value"); - } - test_check(!queue.try_dequeue(value), "queue should be empty"); - - exasol::udf::v2::WaitableMpmcQueue mpmc; - test_check(mpmc.enqueue(7), "MPMC queue enqueue failed"); - test_check(mpmc.drain_notifications() == 1, "unexpected MPMC notification count"); - test_check(mpmc.try_dequeue(value), "MPMC queue dequeue failed"); - test_check(value == 7, "unexpected MPMC value"); - - close_pair(sockets); - ::close(epoll_fd); + queue_ready |= event.data.fd == queue.native_handle(); + socket_ready |= event.data.fd == sockets[1]; } - catch (const std::exception& error) + EXPECT_TRUE(queue_ready); + EXPECT_TRUE(socket_ready); + + EXPECT_EQ(queue.drain_notifications(), 1); + int value = 0; + ASSERT_TRUE(queue.try_dequeue(value)); + EXPECT_EQ(value, 42); + + const std::vector batch{1, 2, 3}; + EXPECT_EQ(queue.enqueue_batch(batch.begin(), batch.end()), batch.size()); + EXPECT_EQ(queue.drain_notifications(), 1); + for (int expected : batch) { - std::fprintf(stderr, "waitable queue test failure: %s\n", error.what()); - return 1; + ASSERT_TRUE(queue.try_dequeue(value)); + EXPECT_EQ(value, expected); } + EXPECT_FALSE(queue.try_dequeue(value)); + + exasol::udf::v2::WaitableMpmcQueue mpmc; + ASSERT_TRUE(mpmc.enqueue(7)); + EXPECT_EQ(mpmc.drain_notifications(), 1); + ASSERT_TRUE(mpmc.try_dequeue(value)); + EXPECT_EQ(value, 7); + + close_pair(sockets); + ::close(epoll_fd); } From c82cd4a116982d60d8b9c0f679d57fae7fd8ebf5 Mon Sep 17 00:00:00 2001 From: Torsten Kilias Date: Thu, 24 Sep 2026 18:34:14 +0200 Subject: [PATCH 06/11] fix: address PR review findings --- doc/changes/unreleased.md | 1 + udf-runner-cpp/v2/json_schema_validation_test.cc | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/doc/changes/unreleased.md b/doc/changes/unreleased.md index 3acf681..fba29f7 100644 --- a/doc/changes/unreleased.md +++ b/doc/changes/unreleased.md @@ -18,6 +18,7 @@ n/a ## Internal +* #64: Added GoogleTest, GoogleMock, and Google Benchmark support for v2 tests * #57: Defined and enforced public v2 C++ coding style * #51: Added agent and contributor guidance for v1/v2 development, SLC workflows, CI testing, and PR conventions * #56: Restructured the developer guide and synchronized agent guidance diff --git a/udf-runner-cpp/v2/json_schema_validation_test.cc b/udf-runner-cpp/v2/json_schema_validation_test.cc index fd6391f..d47255e 100644 --- a/udf-runner-cpp/v2/json_schema_validation_test.cc +++ b/udf-runner-cpp/v2/json_schema_validation_test.cc @@ -10,12 +10,18 @@ namespace isolated_nlohmann = exasol::udf::v2::third_party::nlohmann; namespace { +class JsonSchemaTestError final : public std::runtime_error +{ +public: + using std::runtime_error::runtime_error; +}; + isolated_nlohmann::json read_json(const std::string& path) { std::ifstream input(path); if (!input.good()) { - throw std::runtime_error("cannot open JSON schema: " + path); + throw JsonSchemaTestError("cannot open JSON schema: " + path); } return isolated_nlohmann::json::parse(input); } From c7754f65816b39706de78ccbe8856fff8cc07752 Mon Sep 17 00:00:00 2001 From: Torsten Kilias Date: Fri, 25 Sep 2026 17:24:37 +0200 Subject: [PATCH 07/11] test: migrate EventFd integration tests to GoogleTest --- udf-runner-cpp/v2/BUILD.bazel | 10 +- udf-runner-cpp/v2/event_fd_test.cc | 88 +++++-------- .../v2/waitable_queue_integration_test.cc | 120 +++++++----------- 3 files changed, 86 insertions(+), 132 deletions(-) diff --git a/udf-runner-cpp/v2/BUILD.bazel b/udf-runner-cpp/v2/BUILD.bazel index f020023..1210ddf 100644 --- a/udf-runner-cpp/v2/BUILD.bazel +++ b/udf-runner-cpp/v2/BUILD.bazel @@ -333,7 +333,10 @@ cc_test( name = "event_fd_test", srcs = ["event_fd_test.cc"], copts = ["-std=c++20"], - deps = [":event_fd"], + deps = [ + ":event_fd", + "@googletest//:gtest_main", + ], target_compatible_with = ["@platforms//os:linux"], ) @@ -363,7 +366,10 @@ cc_test( name = "waitable_queue_integration_test", srcs = ["waitable_queue_integration_test.cc"], copts = ["-std=c++20"], - deps = [":waitable_queue"], + deps = [ + ":waitable_queue", + "@googletest//:gtest_main", + ], tags = ["no-coverage"], target_compatible_with = ["@platforms//os:linux"], ) diff --git a/udf-runner-cpp/v2/event_fd_test.cc b/udf-runner-cpp/v2/event_fd_test.cc index 70bb5eb..cefb8fb 100644 --- a/udf-runner-cpp/v2/event_fd_test.cc +++ b/udf-runner-cpp/v2/event_fd_test.cc @@ -1,36 +1,26 @@ #include #include -#include -#include #include #include #include +#include namespace { -void test_check(bool condition, const char* message) -{ - if (!condition) - { - std::cerr << "event fd test failure: " << message << '\n'; - std::abort(); - } -} - template void expect_system_error(Function&& function, std::errc expected, const char* message) { try { function(); - test_check(false, message); + ADD_FAILURE() << message; } catch (const std::system_error& error) { - test_check(error.code() == std::make_error_code(expected), "unexpected system error"); + EXPECT_EQ(error.code(), std::make_error_code(expected)); } } @@ -44,52 +34,44 @@ void self_move_assign(Type& value) } // namespace -int main() +TEST(EventFdTest, SupportsNotificationsAndMoves) { - try - { - exasol::udf::v2::LinuxEventFd event_fd; - test_check(event_fd.native_handle() != -1, "eventfd construction failed"); - - event_fd.write_notification(); - event_fd.write_notification(); - test_check(event_fd.read_notification() == 2, "eventfd did not accumulate notifications"); - expect_system_error([&event_fd] { event_fd.read_notification(); }, - std::errc::resource_unavailable_try_again, - "empty eventfd read should report EAGAIN"); + exasol::udf::v2::LinuxEventFd event_fd; + ASSERT_NE(event_fd.native_handle(), -1); - const int moved_handle = event_fd.native_handle(); - exasol::udf::v2::LinuxEventFd move_constructed(std::move(event_fd)); - test_check(move_constructed.native_handle() == moved_handle, - "move construction changed the handle"); + event_fd.write_notification(); + event_fd.write_notification(); + EXPECT_EQ(event_fd.read_notification(), 2); + expect_system_error([&event_fd] { event_fd.read_notification(); }, + std::errc::resource_unavailable_try_again, + "empty eventfd read should report EAGAIN"); - exasol::udf::v2::LinuxEventFd move_assigned; - move_assigned = std::move(move_constructed); - test_check(move_assigned.native_handle() == moved_handle, - "move assignment changed the handle"); - self_move_assign(move_assigned); - test_check(move_assigned.native_handle() == moved_handle, - "self move assignment changed the handle"); + const int moved_handle = event_fd.native_handle(); + exasol::udf::v2::LinuxEventFd move_constructed(std::move(event_fd)); + EXPECT_EQ(move_constructed.native_handle(), moved_handle); - { - exasol::udf::v2::LinuxEventFd closed_event_fd; - ::close(closed_event_fd.native_handle()); - expect_system_error([&closed_event_fd] { closed_event_fd.read_notification(); }, - std::errc::bad_file_descriptor, - "closed eventfd read should be rejected"); - } + exasol::udf::v2::LinuxEventFd move_assigned; + move_assigned = std::move(move_constructed); + EXPECT_EQ(move_assigned.native_handle(), moved_handle); + self_move_assign(move_assigned); + EXPECT_EQ(move_assigned.native_handle(), moved_handle); +} - { - exasol::udf::v2::LinuxEventFd closed_event_fd; - ::close(closed_event_fd.native_handle()); - expect_system_error([&closed_event_fd] { closed_event_fd.write_notification(); }, - std::errc::bad_file_descriptor, - "closed eventfd write should be rejected"); - } +TEST(EventFdTest, ReportsErrorsForClosedDescriptor) +{ + { + exasol::udf::v2::LinuxEventFd closed_event_fd; + ASSERT_EQ(::close(closed_event_fd.native_handle()), 0); + expect_system_error([&closed_event_fd] { closed_event_fd.read_notification(); }, + std::errc::bad_file_descriptor, + "closed eventfd read should be rejected"); } - catch (const std::exception& error) + { - std::cerr << "event fd test failure: " << error.what() << '\n'; - return 1; + exasol::udf::v2::LinuxEventFd closed_event_fd; + ASSERT_EQ(::close(closed_event_fd.native_handle()), 0); + expect_system_error([&closed_event_fd] { closed_event_fd.write_notification(); }, + std::errc::bad_file_descriptor, + "closed eventfd write should be rejected"); } } diff --git a/udf-runner-cpp/v2/waitable_queue_integration_test.cc b/udf-runner-cpp/v2/waitable_queue_integration_test.cc index e811d9e..cfbcde8 100644 --- a/udf-runner-cpp/v2/waitable_queue_integration_test.cc +++ b/udf-runner-cpp/v2/waitable_queue_integration_test.cc @@ -3,34 +3,23 @@ #include #include -#include #include -#include -#include #include #include #include #include +#include namespace { -void test_check(bool condition, const char* message) -{ - if (!condition) - { - std::cerr << "waitable queue integration test failure: " << message << '\n'; - std::abort(); - } -} - -void add_to_epoll(int epoll_fd, int fd, std::uint32_t events) +bool add_to_epoll(int epoll_fd, int fd, std::uint32_t events) { epoll_event event{}; event.events = events; event.data.fd = fd; - test_check(::epoll_ctl(epoll_fd, EPOLL_CTL_ADD, fd, &event) == 0, "epoll_ctl failed"); + return ::epoll_ctl(epoll_fd, EPOLL_CTL_ADD, fd, &event) == 0; } void close_pair(const std::array& sockets) @@ -47,25 +36,26 @@ void self_move_assign(Queue& queue) (queue.*assign)(std::move(queue)); } -void test_spsc_epoll_and_queue_operations() +} // namespace + +TEST(WaitableQueueIntegrationTest, SpscEpollAndQueueOperations) { exasol::udf::v2::WaitableSpscQueue queue; const int epoll_fd = ::epoll_create1(EPOLL_CLOEXEC); - test_check(epoll_fd != -1, "epoll_create1 failed"); + ASSERT_NE(epoll_fd, -1); std::array sockets{}; - test_check(::socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, sockets.data()) == 0, - "socketpair failed"); - add_to_epoll(epoll_fd, queue.native_handle(), EPOLLIN); - add_to_epoll(epoll_fd, sockets[1], EPOLLIN); + ASSERT_EQ(::socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, sockets.data()), 0); + ASSERT_TRUE(add_to_epoll(epoll_fd, queue.native_handle(), EPOLLIN)); + ASSERT_TRUE(add_to_epoll(epoll_fd, sockets[1], EPOLLIN)); - test_check(queue.enqueue(42), "SPSC queue enqueue failed"); + ASSERT_TRUE(queue.enqueue(42)); const char byte = 'x'; - test_check(::write(sockets[0], &byte, sizeof(byte)) == sizeof(byte), "socket write failed"); + ASSERT_EQ(::write(sockets[0], &byte, sizeof(byte)), sizeof(byte)); std::array events{}; const int event_count = ::epoll_wait(epoll_fd, events.data(), events.size(), 1000); - test_check(event_count == 2, "epoll_wait did not report both descriptors"); + ASSERT_EQ(event_count, 2); bool queue_ready = false; bool socket_ready = false; @@ -74,100 +64,76 @@ void test_spsc_epoll_and_queue_operations() queue_ready |= event.data.fd == queue.native_handle(); socket_ready |= event.data.fd == sockets[1]; } - test_check(queue_ready, "queue descriptor was not ready"); - test_check(socket_ready, "socket descriptor was not ready"); + EXPECT_TRUE(queue_ready); + EXPECT_TRUE(socket_ready); - test_check(queue.drain_notifications() == 1, "unexpected queue notification count"); + EXPECT_EQ(queue.drain_notifications(), 1); int value = 0; - test_check(queue.try_dequeue(value), "SPSC queue dequeue failed"); - test_check(value == 42, "unexpected SPSC queue value"); + ASSERT_TRUE(queue.try_dequeue(value)); + EXPECT_EQ(value, 42); const std::vector batch{1, 2, 3}; - test_check(queue.enqueue_batch(batch.begin(), batch.end()) == batch.size(), - "SPSC batch enqueue failed"); - test_check(queue.drain_notifications() == 1, "unexpected SPSC batch notification count"); + EXPECT_EQ(queue.enqueue_batch(batch.begin(), batch.end()), batch.size()); + EXPECT_EQ(queue.drain_notifications(), 1); for (int expected : batch) { - test_check(queue.try_dequeue(value), "SPSC batch dequeue failed"); - test_check(value == expected, "unexpected SPSC batch value"); + ASSERT_TRUE(queue.try_dequeue(value)); + EXPECT_EQ(value, expected); } - test_check(!queue.try_dequeue(value), "SPSC queue should be empty"); + EXPECT_FALSE(queue.try_dequeue(value)); const std::array empty_batch{}; - test_check(queue.enqueue_batch(empty_batch.begin(), empty_batch.end()) == 0, - "empty SPSC batch should not enqueue values"); - test_check(queue.drain_notifications() == 0, "empty SPSC batch should not notify"); + EXPECT_EQ(queue.enqueue_batch(empty_batch.begin(), empty_batch.end()), 0); + EXPECT_EQ(queue.drain_notifications(), 0); const auto& const_queue = queue; - test_check(&const_queue.queue() == &queue.queue(), "const SPSC queue access failed"); + EXPECT_EQ(&const_queue.queue(), &queue.queue()); exasol::udf::v2::WaitableSpscQueue moved_queue; const int moved_handle = moved_queue.native_handle(); exasol::udf::v2::WaitableSpscQueue move_constructed(std::move(moved_queue)); - test_check(move_constructed.native_handle() == moved_handle, - "SPSC move construction changed handle"); + EXPECT_EQ(move_constructed.native_handle(), moved_handle); exasol::udf::v2::WaitableSpscQueue move_assigned; move_assigned = std::move(move_constructed); - test_check(move_assigned.native_handle() == moved_handle, - "SPSC move assignment changed handle"); + EXPECT_EQ(move_assigned.native_handle(), moved_handle); self_move_assign(move_assigned); close_pair(sockets); - ::close(epoll_fd); + ASSERT_EQ(::close(epoll_fd), 0); } -void test_mpmc_queue_operations() +TEST(WaitableQueueIntegrationTest, MpmcQueueOperations) { exasol::udf::v2::WaitableMpmcQueue queue; - test_check(queue.enqueue(7), "MPMC queue enqueue failed"); - test_check(queue.drain_notifications() == 1, "unexpected MPMC notification count"); + ASSERT_TRUE(queue.enqueue(7)); + EXPECT_EQ(queue.drain_notifications(), 1); int value = 0; - test_check(queue.try_dequeue(value), "MPMC queue dequeue failed"); - test_check(value == 7, "unexpected MPMC queue value"); + ASSERT_TRUE(queue.try_dequeue(value)); + EXPECT_EQ(value, 7); const std::vector batch{8, 9}; - test_check(queue.enqueue_batch(batch.begin(), batch.end()) == batch.size(), - "MPMC batch enqueue failed"); - test_check(queue.drain_notifications() == 1, "unexpected MPMC batch notification count"); + EXPECT_EQ(queue.enqueue_batch(batch.begin(), batch.end()), batch.size()); + EXPECT_EQ(queue.drain_notifications(), 1); for (int expected : batch) { - test_check(queue.try_dequeue(value), "MPMC batch dequeue failed"); - test_check(value == expected, "unexpected MPMC batch value"); + ASSERT_TRUE(queue.try_dequeue(value)); + EXPECT_EQ(value, expected); } const std::array empty_batch{}; - test_check(queue.enqueue_batch(empty_batch.begin(), empty_batch.end()) == 0, - "empty MPMC batch should not enqueue values"); - test_check(queue.drain_notifications() == 0, "empty MPMC batch should not notify"); + EXPECT_EQ(queue.enqueue_batch(empty_batch.begin(), empty_batch.end()), 0); + EXPECT_EQ(queue.drain_notifications(), 0); const auto& const_queue = queue; - test_check(&const_queue.queue() == &queue.queue(), "const MPMC queue access failed"); + EXPECT_EQ(&const_queue.queue(), &queue.queue()); exasol::udf::v2::WaitableMpmcQueue moved_queue; const int moved_handle = moved_queue.native_handle(); exasol::udf::v2::WaitableMpmcQueue move_constructed(std::move(moved_queue)); - test_check(move_constructed.native_handle() == moved_handle, - "MPMC move construction changed handle"); + EXPECT_EQ(move_constructed.native_handle(), moved_handle); exasol::udf::v2::WaitableMpmcQueue move_assigned; move_assigned = std::move(move_constructed); - test_check(move_assigned.native_handle() == moved_handle, - "MPMC move assignment changed handle"); + EXPECT_EQ(move_assigned.native_handle(), moved_handle); self_move_assign(move_assigned); } - -} // namespace - -int main() -{ - try - { - test_spsc_epoll_and_queue_operations(); - test_mpmc_queue_operations(); - } - catch (const std::exception& error) - { - std::cerr << "waitable queue integration test failure: " << error.what() << '\n'; - return 1; - } -} From 347a07a6192057f40ae02bcca2f7a32082cb524d Mon Sep 17 00:00:00 2001 From: Torsten Kilias Date: Fri, 25 Sep 2026 17:36:48 +0200 Subject: [PATCH 08/11] test: remove generic exception catch --- udf-runner-cpp/v2/waitable_queue_test.cc | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/udf-runner-cpp/v2/waitable_queue_test.cc b/udf-runner-cpp/v2/waitable_queue_test.cc index e2ed547..3f8f13b 100644 --- a/udf-runner-cpp/v2/waitable_queue_test.cc +++ b/udf-runner-cpp/v2/waitable_queue_test.cc @@ -239,15 +239,8 @@ void test_mock_capacity_and_moves() TEST(WaitableQueueTest, MockEventFdAndQueueOperations) { - try - { - test_mock_queue_operations(); - test_mock_notification_paths(); - test_mock_capacity_and_moves(); - expect_invalid_argument(); - } - catch (const std::exception& error) - { - FAIL() << error.what(); - } + test_mock_queue_operations(); + test_mock_notification_paths(); + test_mock_capacity_and_moves(); + expect_invalid_argument(); } From d626361bcb83267425cca45d4ab3b5315df5399c Mon Sep 17 00:00:00 2001 From: Torsten Kilias Date: Fri, 25 Sep 2026 17:47:34 +0200 Subject: [PATCH 09/11] test: split v2 GoogleTest cases --- udf-runner-cpp/v2/event_fd_test.cc | 45 ++++--- .../v2/waitable_queue_integration_test.cc | 83 ++++++++---- udf-runner-cpp/v2/waitable_queue_test.cc | 124 +++++++++++++++--- 3 files changed, 192 insertions(+), 60 deletions(-) diff --git a/udf-runner-cpp/v2/event_fd_test.cc b/udf-runner-cpp/v2/event_fd_test.cc index cefb8fb..613cd63 100644 --- a/udf-runner-cpp/v2/event_fd_test.cc +++ b/udf-runner-cpp/v2/event_fd_test.cc @@ -34,7 +34,7 @@ void self_move_assign(Type& value) } // namespace -TEST(EventFdTest, SupportsNotificationsAndMoves) +TEST(EventFdTest, AccumulatesNotifications) { exasol::udf::v2::LinuxEventFd event_fd; ASSERT_NE(event_fd.native_handle(), -1); @@ -42,36 +42,47 @@ TEST(EventFdTest, SupportsNotificationsAndMoves) event_fd.write_notification(); event_fd.write_notification(); EXPECT_EQ(event_fd.read_notification(), 2); +} + +TEST(EventFdTest, RejectsReadWhenEmpty) +{ + exasol::udf::v2::LinuxEventFd event_fd; expect_system_error([&event_fd] { event_fd.read_notification(); }, std::errc::resource_unavailable_try_again, "empty eventfd read should report EAGAIN"); +} +TEST(EventFdTest, SupportsMoveConstruction) +{ + exasol::udf::v2::LinuxEventFd event_fd; const int moved_handle = event_fd.native_handle(); exasol::udf::v2::LinuxEventFd move_constructed(std::move(event_fd)); EXPECT_EQ(move_constructed.native_handle(), moved_handle); +} +TEST(EventFdTest, SupportsMoveAssignmentAndSelfMove) +{ + exasol::udf::v2::LinuxEventFd source; + const int moved_handle = source.native_handle(); exasol::udf::v2::LinuxEventFd move_assigned; - move_assigned = std::move(move_constructed); + move_assigned = std::move(source); EXPECT_EQ(move_assigned.native_handle(), moved_handle); self_move_assign(move_assigned); EXPECT_EQ(move_assigned.native_handle(), moved_handle); } -TEST(EventFdTest, ReportsErrorsForClosedDescriptor) +TEST(EventFdTest, RejectsReadOnClosedDescriptor) { - { - exasol::udf::v2::LinuxEventFd closed_event_fd; - ASSERT_EQ(::close(closed_event_fd.native_handle()), 0); - expect_system_error([&closed_event_fd] { closed_event_fd.read_notification(); }, - std::errc::bad_file_descriptor, - "closed eventfd read should be rejected"); - } + exasol::udf::v2::LinuxEventFd closed_event_fd; + ASSERT_EQ(::close(closed_event_fd.native_handle()), 0); + expect_system_error([&closed_event_fd] { closed_event_fd.read_notification(); }, + std::errc::bad_file_descriptor, "closed eventfd read should be rejected"); +} - { - exasol::udf::v2::LinuxEventFd closed_event_fd; - ASSERT_EQ(::close(closed_event_fd.native_handle()), 0); - expect_system_error([&closed_event_fd] { closed_event_fd.write_notification(); }, - std::errc::bad_file_descriptor, - "closed eventfd write should be rejected"); - } +TEST(EventFdTest, RejectsWriteOnClosedDescriptor) +{ + exasol::udf::v2::LinuxEventFd closed_event_fd; + ASSERT_EQ(::close(closed_event_fd.native_handle()), 0); + expect_system_error([&closed_event_fd] { closed_event_fd.write_notification(); }, + std::errc::bad_file_descriptor, "closed eventfd write should be rejected"); } diff --git a/udf-runner-cpp/v2/waitable_queue_integration_test.cc b/udf-runner-cpp/v2/waitable_queue_integration_test.cc index cfbcde8..54784e4 100644 --- a/udf-runner-cpp/v2/waitable_queue_integration_test.cc +++ b/udf-runner-cpp/v2/waitable_queue_integration_test.cc @@ -14,20 +14,6 @@ namespace { -bool add_to_epoll(int epoll_fd, int fd, std::uint32_t events) -{ - epoll_event event{}; - event.events = events; - event.data.fd = fd; - return ::epoll_ctl(epoll_fd, EPOLL_CTL_ADD, fd, &event) == 0; -} - -void close_pair(const std::array& sockets) -{ - ::close(sockets[0]); - ::close(sockets[1]); -} - template void self_move_assign(Queue& queue) { @@ -36,19 +22,41 @@ void self_move_assign(Queue& queue) (queue.*assign)(std::move(queue)); } -} // namespace - -TEST(WaitableQueueIntegrationTest, SpscEpollAndQueueOperations) +class SpscEpollTest : public testing::Test { - exasol::udf::v2::WaitableSpscQueue queue; - const int epoll_fd = ::epoll_create1(EPOLL_CLOEXEC); - ASSERT_NE(epoll_fd, -1); +protected: + void SetUp() override + { + ASSERT_NE(epoll_fd, -1); + ASSERT_EQ(::socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, sockets.data()), 0); + ASSERT_TRUE(add_to_epoll(queue.native_handle(), EPOLLIN)); + ASSERT_TRUE(add_to_epoll(sockets[1], EPOLLIN)); + } + void TearDown() override + { + ::close(sockets[0]); + ::close(sockets[1]); + ::close(epoll_fd); + } + + bool add_to_epoll(int fd, std::uint32_t events) + { + epoll_event event{}; + event.events = events; + event.data.fd = fd; + return ::epoll_ctl(epoll_fd, EPOLL_CTL_ADD, fd, &event) == 0; + } + + exasol::udf::v2::WaitableSpscQueue queue; + int epoll_fd = ::epoll_create1(EPOLL_CLOEXEC); std::array sockets{}; - ASSERT_EQ(::socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, sockets.data()), 0); - ASSERT_TRUE(add_to_epoll(epoll_fd, queue.native_handle(), EPOLLIN)); - ASSERT_TRUE(add_to_epoll(epoll_fd, sockets[1], EPOLLIN)); +}; +} // namespace + +TEST_F(SpscEpollTest, ReportsQueueAndSocketReadiness) +{ ASSERT_TRUE(queue.enqueue(42)); const char byte = 'x'; ASSERT_EQ(::write(sockets[0], &byte, sizeof(byte)), sizeof(byte)); @@ -66,8 +74,14 @@ TEST(WaitableQueueIntegrationTest, SpscEpollAndQueueOperations) } EXPECT_TRUE(queue_ready); EXPECT_TRUE(socket_ready); + EXPECT_EQ(queue.drain_notifications(), 1); +} +TEST_F(SpscEpollTest, SupportsQueueOperationsAndBatches) +{ + ASSERT_TRUE(queue.enqueue(42)); EXPECT_EQ(queue.drain_notifications(), 1); + int value = 0; ASSERT_TRUE(queue.try_dequeue(value)); EXPECT_EQ(value, 42); @@ -88,21 +102,22 @@ TEST(WaitableQueueIntegrationTest, SpscEpollAndQueueOperations) const auto& const_queue = queue; EXPECT_EQ(&const_queue.queue(), &queue.queue()); +} +TEST_F(SpscEpollTest, SupportsMoves) +{ exasol::udf::v2::WaitableSpscQueue moved_queue; const int moved_handle = moved_queue.native_handle(); exasol::udf::v2::WaitableSpscQueue move_constructed(std::move(moved_queue)); EXPECT_EQ(move_constructed.native_handle(), moved_handle); + exasol::udf::v2::WaitableSpscQueue move_assigned; move_assigned = std::move(move_constructed); EXPECT_EQ(move_assigned.native_handle(), moved_handle); self_move_assign(move_assigned); - - close_pair(sockets); - ASSERT_EQ(::close(epoll_fd), 0); } -TEST(WaitableQueueIntegrationTest, MpmcQueueOperations) +TEST(WaitableQueueIntegrationTest, MpmcSingleValueOperations) { exasol::udf::v2::WaitableMpmcQueue queue; ASSERT_TRUE(queue.enqueue(7)); @@ -111,10 +126,16 @@ TEST(WaitableQueueIntegrationTest, MpmcQueueOperations) int value = 0; ASSERT_TRUE(queue.try_dequeue(value)); EXPECT_EQ(value, 7); +} +TEST(WaitableQueueIntegrationTest, MpmcBatchAndEmptyBatchOperations) +{ + exasol::udf::v2::WaitableMpmcQueue queue; const std::vector batch{8, 9}; EXPECT_EQ(queue.enqueue_batch(batch.begin(), batch.end()), batch.size()); EXPECT_EQ(queue.drain_notifications(), 1); + + int value = 0; for (int expected : batch) { ASSERT_TRUE(queue.try_dequeue(value)); @@ -124,14 +145,22 @@ TEST(WaitableQueueIntegrationTest, MpmcQueueOperations) const std::array empty_batch{}; EXPECT_EQ(queue.enqueue_batch(empty_batch.begin(), empty_batch.end()), 0); EXPECT_EQ(queue.drain_notifications(), 0); +} +TEST(WaitableQueueIntegrationTest, MpmcProvidesQueueAccess) +{ + exasol::udf::v2::WaitableMpmcQueue queue; const auto& const_queue = queue; EXPECT_EQ(&const_queue.queue(), &queue.queue()); +} +TEST(WaitableQueueIntegrationTest, MpmcSupportsMoves) +{ exasol::udf::v2::WaitableMpmcQueue moved_queue; const int moved_handle = moved_queue.native_handle(); exasol::udf::v2::WaitableMpmcQueue move_constructed(std::move(moved_queue)); EXPECT_EQ(move_constructed.native_handle(), moved_handle); + exasol::udf::v2::WaitableMpmcQueue move_assigned; move_assigned = std::move(move_constructed); EXPECT_EQ(move_assigned.native_handle(), moved_handle); diff --git a/udf-runner-cpp/v2/waitable_queue_test.cc b/udf-runner-cpp/v2/waitable_queue_test.cc index 3f8f13b..eb00286 100644 --- a/udf-runner-cpp/v2/waitable_queue_test.cc +++ b/udf-runner-cpp/v2/waitable_queue_test.cc @@ -1,8 +1,8 @@ #include #include -#include #include #include +#include #include #include #include @@ -151,7 +151,7 @@ void expect_invalid_argument() } } -void test_mock_queue_operations() +void test_mock_single_value_operations() { using WaitableQueue = exasol::udf::v2::WaitableQueue; @@ -166,6 +166,13 @@ void test_mock_queue_operations() test_check(queue.try_dequeue(value), "mock queue dequeue failed"); test_check(value == 1, "unexpected mock queue value"); test_check(!queue.try_dequeue(value), "mock queue should be empty"); +} + +void test_mock_batch_operations() +{ + using WaitableQueue = exasol::udf::v2::WaitableQueue; + auto event_fd = std::make_unique(); + auto queue = WaitableQueue(MockQueue{}, std::move(event_fd)); const std::vector batch{2, 3}; test_check(queue.enqueue_batch(batch.begin(), batch.end()) == batch.size(), @@ -173,43 +180,60 @@ void test_mock_queue_operations() test_check(queue.queue().size() == batch.size(), "mock queue accessor returned wrong queue"); const auto& const_queue = queue; test_check(&const_queue.queue() == &queue.queue(), "const queue access failed"); +} +void test_mock_empty_batch() +{ + using WaitableQueue = exasol::udf::v2::WaitableQueue; + auto queue = WaitableQueue(MockQueue{}, std::make_unique()); const std::vector empty_batch; test_check(queue.enqueue_batch(empty_batch.begin(), empty_batch.end()) == 0, "empty mock batch should not enqueue values"); } -void test_mock_notification_paths() +void test_mock_interrupted_read() { - using enum std::errc; using WaitableQueue = exasol::udf::v2::WaitableQueue; auto read_event_fd = std::make_unique(); read_event_fd->set_read_actions( - {interrupted, std::uint64_t{7}, resource_unavailable_try_again}); + {std::errc::interrupted, std::uint64_t{7}, std::errc::resource_unavailable_try_again}); auto read_queue = WaitableQueue(MockQueue{}, std::move(read_event_fd)); test_check(read_queue.drain_notifications() == 7, "interrupted read was not retried"); +} +void test_mock_interrupted_write_and_saturation() +{ + using WaitableQueue = exasol::udf::v2::WaitableQueue; auto write_event_fd = std::make_unique(); write_event_fd->set_write_actions( - {interrupted, std::monostate{}, resource_unavailable_try_again}); + {std::errc::interrupted, std::monostate{}, std::errc::resource_unavailable_try_again}); auto write_queue = WaitableQueue(MockQueue{}, std::move(write_event_fd)); test_check(write_queue.enqueue(1), "interrupted write was not retried"); test_check(write_queue.enqueue(2), "saturated write was not ignored"); +} +void test_mock_read_error() +{ + using WaitableQueue = exasol::udf::v2::WaitableQueue; auto read_error_event_fd = std::make_unique(); - read_error_event_fd->set_read_actions({io_error}); + read_error_event_fd->set_read_actions({std::errc::io_error}); auto read_error_queue = WaitableQueue(MockQueue{}, std::move(read_error_event_fd)); - expect_system_error([&read_error_queue] { read_error_queue.drain_notifications(); }, io_error); + expect_system_error([&read_error_queue] { read_error_queue.drain_notifications(); }, + std::errc::io_error); +} +void test_mock_write_error() +{ + using WaitableQueue = exasol::udf::v2::WaitableQueue; auto write_error_event_fd = std::make_unique(); - write_error_event_fd->set_write_actions({io_error}); + write_error_event_fd->set_write_actions({std::errc::io_error}); auto write_error_queue = WaitableQueue(MockQueue{}, std::move(write_error_event_fd)); expect_system_error([&write_error_queue] { static_cast(write_error_queue.enqueue(1)); }, - io_error); + std::errc::io_error); } -void test_mock_capacity_and_moves() +void test_mock_limited_capacity() { using WaitableQueue = exasol::udf::v2::WaitableQueue; @@ -218,16 +242,32 @@ void test_mock_capacity_and_moves() test_check(limited_queue.enqueue_batch(batch.begin(), batch.end()) == 1, "mock queue should stop at capacity"); test_check(!limited_queue.enqueue(3), "full mock queue should reject enqueue"); +} - auto empty_queue = WaitableQueue(MockQueue{0}, std::make_unique()); +void test_mock_zero_capacity() +{ + using WaitableQueue = exasol::udf::v2::WaitableQueue; + auto empty_queue = WaitableQueue(MockQueue{0}, std::make_unique()); + const std::vector batch{1, 2}; test_check(empty_queue.enqueue_batch(batch.begin(), batch.end()) == 0, "empty mock queue should reject a batch"); +} +void test_mock_move_construction() +{ + using WaitableQueue = exasol::udf::v2::WaitableQueue; WaitableQueue moved_queue(MockQueue{}, std::make_unique()); const int moved_handle = moved_queue.native_handle(); WaitableQueue move_constructed(std::move(moved_queue)); test_check(move_constructed.native_handle() == moved_handle, "mock move construction changed handle"); +} + +void test_mock_move_assignment_and_self_move() +{ + using WaitableQueue = exasol::udf::v2::WaitableQueue; + WaitableQueue move_constructed(MockQueue{}, std::make_unique()); + const int moved_handle = move_constructed.native_handle(); WaitableQueue move_assigned(MockQueue{}, std::make_unique()); move_assigned = std::move(move_constructed); test_check(move_assigned.native_handle() == moved_handle, @@ -237,10 +277,62 @@ void test_mock_capacity_and_moves() } // namespace -TEST(WaitableQueueTest, MockEventFdAndQueueOperations) +TEST(WaitableQueueTest, EnqueuesDequeuesAndAccessesMockQueue) +{ + test_mock_single_value_operations(); +} + +TEST(WaitableQueueTest, EnqueuesMockBatchesAndProvidesAccessors) +{ + test_mock_batch_operations(); +} + +TEST(WaitableQueueTest, HandlesEmptyMockBatches) +{ + test_mock_empty_batch(); +} + +TEST(WaitableQueueTest, RetriesInterruptedMockReads) +{ + test_mock_interrupted_read(); +} + +TEST(WaitableQueueTest, RetriesInterruptedWritesAndIgnoresSaturation) +{ + test_mock_interrupted_write_and_saturation(); +} + +TEST(WaitableQueueTest, PropagatesMockReadErrors) +{ + test_mock_read_error(); +} + +TEST(WaitableQueueTest, PropagatesMockWriteErrors) +{ + test_mock_write_error(); +} + +TEST(WaitableQueueTest, StopsAtMockCapacity) +{ + test_mock_limited_capacity(); +} + +TEST(WaitableQueueTest, HandlesZeroMockCapacity) +{ + test_mock_zero_capacity(); +} + +TEST(WaitableQueueTest, SupportsMockMoveConstruction) +{ + test_mock_move_construction(); +} + +TEST(WaitableQueueTest, SupportsMockMoveAssignmentAndSelfMove) +{ + test_mock_move_assignment_and_self_move(); +} + +TEST(WaitableQueueTest, RejectsNullEventFd) { - test_mock_queue_operations(); - test_mock_notification_paths(); - test_mock_capacity_and_moves(); expect_invalid_argument(); } From 91c6af7e1bb54d98189538a9a5b9579af7ef2c91 Mon Sep 17 00:00:00 2001 From: Torsten Kilias Date: Fri, 25 Sep 2026 18:01:26 +0200 Subject: [PATCH 10/11] test: use GoogleMock for waitable queue tests --- udf-runner-cpp/v2/BUILD.bazel | 1 + udf-runner-cpp/v2/waitable_queue_test.cc | 342 ++++++++--------------- 2 files changed, 121 insertions(+), 222 deletions(-) diff --git a/udf-runner-cpp/v2/BUILD.bazel b/udf-runner-cpp/v2/BUILD.bazel index 1210ddf..4f2fbec 100644 --- a/udf-runner-cpp/v2/BUILD.bazel +++ b/udf-runner-cpp/v2/BUILD.bazel @@ -357,6 +357,7 @@ cc_test( copts = ["-std=c++20"], deps = [ ":waitable_queue", + "@googletest//:gtest", "@googletest//:gtest_main", ], target_compatible_with = ["@platforms//os:linux"], diff --git a/udf-runner-cpp/v2/waitable_queue_test.cc b/udf-runner-cpp/v2/waitable_queue_test.cc index eb00286..a241985 100644 --- a/udf-runner-cpp/v2/waitable_queue_test.cc +++ b/udf-runner-cpp/v2/waitable_queue_test.cc @@ -5,20 +5,15 @@ #include #include #include -#include #include #include +#include #include namespace { -void test_check(bool condition, const char* message) -{ - EXPECT_TRUE(condition) << message; -} - class MockQueue { public: @@ -58,281 +53,184 @@ class MockQueue std::deque values; }; -class MockEventFd final : public exasol::udf::v2::EventFd +class MockEventFd : public exasol::udf::v2::EventFd { public: - using read_action = std::variant; - using write_action = std::variant; - - void set_read_actions(std::deque actions) - { - read_actions = std::move(actions); - } - - void set_write_actions(std::deque actions) - { - write_actions = std::move(actions); - } - - [[nodiscard]] int native_handle() const noexcept override - { - return 42; - } - - std::uint64_t read_notification() override - { - const auto action = read_actions.empty() - ? read_action{std::errc::resource_unavailable_try_again} - : read_actions.front(); - if (!read_actions.empty()) - { - read_actions.pop_front(); - } - if (const auto* error = std::get_if(&action)) - { - throw std::system_error(std::make_error_code(*error), "mock eventfd read"); - } - return std::get(action); - } - - void write_notification() override - { - const auto action = - write_actions.empty() ? write_action{std::monostate{}} : write_actions.front(); - if (!write_actions.empty()) - { - write_actions.pop_front(); - } - if (const auto* error = std::get_if(&action)) - { - throw std::system_error(std::make_error_code(*error), "mock eventfd write"); - } - } - -private: - std::deque read_actions; - std::deque write_actions; + MOCK_METHOD(int, native_handle, (), (const, noexcept, override)); + MOCK_METHOD(std::uint64_t, read_notification, (), (override)); + MOCK_METHOD(void, write_notification, (), (override)); }; -template -void expect_system_error(Function&& function, std::errc expected) -{ - try - { - function(); - test_check(false, "expected system error"); - } - catch (const std::system_error& error) - { - test_check(error.code() == std::make_error_code(expected), "unexpected system error"); - } -} +using testing::Return; +using testing::StrictMock; +using testing::Throw; -template -void self_move_assign(Queue& queue) -{ - using move_assignment = Queue& (Queue::*)(Queue&&) noexcept; - const move_assignment assign = &Queue::operator=; - (queue.*assign)(std::move(queue)); -} +} // namespace -void expect_invalid_argument() +TEST(WaitableQueueTest, EnqueuesDequeuesAndDrainsNotifications) { - try - { - auto null_event_fd = std::unique_ptr{}; - auto queue = exasol::udf::v2::WaitableQueue(MockQueue{}, std::move(null_event_fd)); - static_cast(queue); - test_check(false, "null eventfd implementation was accepted"); - } - catch (const std::invalid_argument& error) - { - test_check(error.what() != nullptr, "invalid-argument error did not contain a message"); - } -} + auto event_fd = std::make_unique>(); + EXPECT_CALL(*event_fd, native_handle()).WillOnce(Return(42)); + EXPECT_CALL(*event_fd, write_notification()).WillOnce(Return()); + EXPECT_CALL(*event_fd, read_notification()) + .WillOnce(Return(3)) + .WillOnce(Throw( + std::system_error(std::make_error_code(std::errc::resource_unavailable_try_again)))); -void test_mock_single_value_operations() -{ using WaitableQueue = exasol::udf::v2::WaitableQueue; - - auto event_fd = std::make_unique(); - event_fd->set_read_actions({std::uint64_t{3}, std::errc::resource_unavailable_try_again}); - auto queue = WaitableQueue(MockQueue{}, std::move(event_fd)); - test_check(queue.native_handle() == 42, "mock eventfd handle was not retained"); - test_check(queue.enqueue(1), "mock queue enqueue failed"); - test_check(queue.drain_notifications() == 3, "mock notification drain failed"); + auto queue = WaitableQueue(MockQueue{}, std::move(event_fd)); + EXPECT_EQ(queue.native_handle(), 42); + ASSERT_TRUE(queue.enqueue(1)); + EXPECT_EQ(queue.drain_notifications(), 3); int value = 0; - test_check(queue.try_dequeue(value), "mock queue dequeue failed"); - test_check(value == 1, "unexpected mock queue value"); - test_check(!queue.try_dequeue(value), "mock queue should be empty"); + ASSERT_TRUE(queue.try_dequeue(value)); + EXPECT_EQ(value, 1); + EXPECT_FALSE(queue.try_dequeue(value)); } -void test_mock_batch_operations() +TEST(WaitableQueueTest, EnqueuesBatchesAndProvidesQueueAccessors) { + auto event_fd = std::make_unique>(); + EXPECT_CALL(*event_fd, write_notification()).WillOnce(Return()); + using WaitableQueue = exasol::udf::v2::WaitableQueue; - auto event_fd = std::make_unique(); auto queue = WaitableQueue(MockQueue{}, std::move(event_fd)); - const std::vector batch{2, 3}; - test_check(queue.enqueue_batch(batch.begin(), batch.end()) == batch.size(), - "mock batch enqueue failed"); - test_check(queue.queue().size() == batch.size(), "mock queue accessor returned wrong queue"); + EXPECT_EQ(queue.enqueue_batch(batch.begin(), batch.end()), batch.size()); + EXPECT_EQ(queue.queue().size(), batch.size()); const auto& const_queue = queue; - test_check(&const_queue.queue() == &queue.queue(), "const queue access failed"); + EXPECT_EQ(&const_queue.queue(), &queue.queue()); } -void test_mock_empty_batch() +TEST(WaitableQueueTest, DoesNotNotifyForEmptyBatch) { + auto event_fd = std::make_unique>(); using WaitableQueue = exasol::udf::v2::WaitableQueue; - auto queue = WaitableQueue(MockQueue{}, std::make_unique()); + auto queue = WaitableQueue(MockQueue{}, std::move(event_fd)); const std::vector empty_batch; - test_check(queue.enqueue_batch(empty_batch.begin(), empty_batch.end()) == 0, - "empty mock batch should not enqueue values"); + EXPECT_EQ(queue.enqueue_batch(empty_batch.begin(), empty_batch.end()), 0); } -void test_mock_interrupted_read() +TEST(WaitableQueueTest, RetriesInterruptedReads) { - using WaitableQueue = exasol::udf::v2::WaitableQueue; - - auto read_event_fd = std::make_unique(); - read_event_fd->set_read_actions( - {std::errc::interrupted, std::uint64_t{7}, std::errc::resource_unavailable_try_again}); - auto read_queue = WaitableQueue(MockQueue{}, std::move(read_event_fd)); - test_check(read_queue.drain_notifications() == 7, "interrupted read was not retried"); -} + auto event_fd = std::make_unique>(); + EXPECT_CALL(*event_fd, read_notification()) + .WillOnce(Throw(std::system_error(std::make_error_code(std::errc::interrupted)))) + .WillOnce(Return(7)) + .WillOnce(Throw( + std::system_error(std::make_error_code(std::errc::resource_unavailable_try_again)))); -void test_mock_interrupted_write_and_saturation() -{ using WaitableQueue = exasol::udf::v2::WaitableQueue; - auto write_event_fd = std::make_unique(); - write_event_fd->set_write_actions( - {std::errc::interrupted, std::monostate{}, std::errc::resource_unavailable_try_again}); - auto write_queue = WaitableQueue(MockQueue{}, std::move(write_event_fd)); - test_check(write_queue.enqueue(1), "interrupted write was not retried"); - test_check(write_queue.enqueue(2), "saturated write was not ignored"); -} - -void test_mock_read_error() -{ - using WaitableQueue = exasol::udf::v2::WaitableQueue; - auto read_error_event_fd = std::make_unique(); - read_error_event_fd->set_read_actions({std::errc::io_error}); - auto read_error_queue = WaitableQueue(MockQueue{}, std::move(read_error_event_fd)); - expect_system_error([&read_error_queue] { read_error_queue.drain_notifications(); }, - std::errc::io_error); -} - -void test_mock_write_error() -{ - using WaitableQueue = exasol::udf::v2::WaitableQueue; - auto write_error_event_fd = std::make_unique(); - write_error_event_fd->set_write_actions({std::errc::io_error}); - auto write_error_queue = WaitableQueue(MockQueue{}, std::move(write_error_event_fd)); - expect_system_error([&write_error_queue] { static_cast(write_error_queue.enqueue(1)); }, - std::errc::io_error); + auto queue = WaitableQueue(MockQueue{}, std::move(event_fd)); + EXPECT_EQ(queue.drain_notifications(), 7); } -void test_mock_limited_capacity() +TEST(WaitableQueueTest, RetriesInterruptedWritesAndIgnoresSaturation) { - using WaitableQueue = exasol::udf::v2::WaitableQueue; + auto event_fd = std::make_unique>(); + EXPECT_CALL(*event_fd, write_notification()) + .WillOnce(Throw(std::system_error(std::make_error_code(std::errc::interrupted)))) + .WillOnce(Return()) + .WillOnce(Throw( + std::system_error(std::make_error_code(std::errc::resource_unavailable_try_again)))); - auto limited_queue = WaitableQueue(MockQueue{1}, std::make_unique()); - const std::vector batch{1, 2}; - test_check(limited_queue.enqueue_batch(batch.begin(), batch.end()) == 1, - "mock queue should stop at capacity"); - test_check(!limited_queue.enqueue(3), "full mock queue should reject enqueue"); -} - -void test_mock_zero_capacity() -{ using WaitableQueue = exasol::udf::v2::WaitableQueue; - auto empty_queue = WaitableQueue(MockQueue{0}, std::make_unique()); - const std::vector batch{1, 2}; - test_check(empty_queue.enqueue_batch(batch.begin(), batch.end()) == 0, - "empty mock queue should reject a batch"); + auto queue = WaitableQueue(MockQueue{}, std::move(event_fd)); + EXPECT_TRUE(queue.enqueue(1)); + EXPECT_TRUE(queue.enqueue(2)); } -void test_mock_move_construction() +TEST(WaitableQueueTest, PropagatesReadErrors) { - using WaitableQueue = exasol::udf::v2::WaitableQueue; - WaitableQueue moved_queue(MockQueue{}, std::make_unique()); - const int moved_handle = moved_queue.native_handle(); - WaitableQueue move_constructed(std::move(moved_queue)); - test_check(move_constructed.native_handle() == moved_handle, - "mock move construction changed handle"); -} + auto event_fd = std::make_unique>(); + EXPECT_CALL(*event_fd, read_notification()) + .WillOnce(Throw(std::system_error(std::make_error_code(std::errc::io_error)))); -void test_mock_move_assignment_and_self_move() -{ using WaitableQueue = exasol::udf::v2::WaitableQueue; - WaitableQueue move_constructed(MockQueue{}, std::make_unique()); - const int moved_handle = move_constructed.native_handle(); - WaitableQueue move_assigned(MockQueue{}, std::make_unique()); - move_assigned = std::move(move_constructed); - test_check(move_assigned.native_handle() == moved_handle, - "mock move assignment changed handle"); - self_move_assign(move_assigned); -} - -} // namespace - -TEST(WaitableQueueTest, EnqueuesDequeuesAndAccessesMockQueue) -{ - test_mock_single_value_operations(); -} - -TEST(WaitableQueueTest, EnqueuesMockBatchesAndProvidesAccessors) -{ - test_mock_batch_operations(); + auto queue = WaitableQueue(MockQueue{}, std::move(event_fd)); + try + { + static_cast(queue.drain_notifications()); + ADD_FAILURE() << "expected read error"; + } + catch (const std::system_error& error) + { + EXPECT_EQ(error.code(), std::make_error_code(std::errc::io_error)); + } } -TEST(WaitableQueueTest, HandlesEmptyMockBatches) +TEST(WaitableQueueTest, PropagatesWriteErrors) { - test_mock_empty_batch(); -} + auto event_fd = std::make_unique>(); + EXPECT_CALL(*event_fd, write_notification()) + .WillOnce(Throw(std::system_error(std::make_error_code(std::errc::io_error)))); -TEST(WaitableQueueTest, RetriesInterruptedMockReads) -{ - test_mock_interrupted_read(); + using WaitableQueue = exasol::udf::v2::WaitableQueue; + auto queue = WaitableQueue(MockQueue{}, std::move(event_fd)); + try + { + static_cast(queue.enqueue(1)); + ADD_FAILURE() << "expected write error"; + } + catch (const std::system_error& error) + { + EXPECT_EQ(error.code(), std::make_error_code(std::errc::io_error)); + } } -TEST(WaitableQueueTest, RetriesInterruptedWritesAndIgnoresSaturation) +TEST(WaitableQueueTest, StopsAtLimitedCapacity) { - test_mock_interrupted_write_and_saturation(); -} + auto event_fd = std::make_unique>(); + EXPECT_CALL(*event_fd, write_notification()).WillOnce(Return()); -TEST(WaitableQueueTest, PropagatesMockReadErrors) -{ - test_mock_read_error(); + using WaitableQueue = exasol::udf::v2::WaitableQueue; + auto queue = WaitableQueue(MockQueue{1}, std::move(event_fd)); + const std::vector batch{1, 2}; + EXPECT_EQ(queue.enqueue_batch(batch.begin(), batch.end()), 1); + EXPECT_FALSE(queue.enqueue(3)); } -TEST(WaitableQueueTest, PropagatesMockWriteErrors) +TEST(WaitableQueueTest, HandlesZeroCapacity) { - test_mock_write_error(); + auto event_fd = std::make_unique>(); + using WaitableQueue = exasol::udf::v2::WaitableQueue; + auto queue = WaitableQueue(MockQueue{0}, std::move(event_fd)); + const std::vector batch{1, 2}; + EXPECT_EQ(queue.enqueue_batch(batch.begin(), batch.end()), 0); } -TEST(WaitableQueueTest, StopsAtMockCapacity) +TEST(WaitableQueueTest, SupportsMoveConstruction) { - test_mock_limited_capacity(); -} + auto event_fd = std::make_unique>(); + EXPECT_CALL(*event_fd, native_handle()).WillOnce(Return(42)); -TEST(WaitableQueueTest, HandlesZeroMockCapacity) -{ - test_mock_zero_capacity(); + using WaitableQueue = exasol::udf::v2::WaitableQueue; + WaitableQueue moved_queue(MockQueue{}, std::move(event_fd)); + WaitableQueue move_constructed(std::move(moved_queue)); + EXPECT_EQ(move_constructed.native_handle(), 42); } -TEST(WaitableQueueTest, SupportsMockMoveConstruction) +TEST(WaitableQueueTest, SupportsMoveAssignmentAndSelfMove) { - test_mock_move_construction(); -} + auto source_event_fd = std::make_unique>(); + EXPECT_CALL(*source_event_fd, native_handle()).WillOnce(Return(42)); + auto target_event_fd = std::make_unique>(); -TEST(WaitableQueueTest, SupportsMockMoveAssignmentAndSelfMove) -{ - test_mock_move_assignment_and_self_move(); + using WaitableQueue = exasol::udf::v2::WaitableQueue; + WaitableQueue source(MockQueue{}, std::move(source_event_fd)); + WaitableQueue target(MockQueue{}, std::move(target_event_fd)); + target = std::move(source); + EXPECT_EQ(target.native_handle(), 42); + using move_assignment = WaitableQueue& (WaitableQueue::*)(WaitableQueue&&) noexcept; + const move_assignment assign = &WaitableQueue::operator=; + (target.*assign)(std::move(target)); } TEST(WaitableQueueTest, RejectsNullEventFd) { - expect_invalid_argument(); + auto null_event_fd = std::unique_ptr{}; + EXPECT_THROW( + { auto queue = exasol::udf::v2::WaitableQueue(MockQueue{}, std::move(null_event_fd)); }, + std::invalid_argument); } From e2391ea0dd985030f617848a646a4f3dd7c7ddf2 Mon Sep 17 00:00:00 2001 From: Torsten Kilias Date: Fri, 25 Sep 2026 18:16:14 +0200 Subject: [PATCH 11/11] fix: resolve v2 clang-tidy CI findings --- .../v2/waitable_queue_integration_test.cc | 74 +++++++++++-------- udf-runner-cpp/v2/waitable_queue_test.cc | 9 ++- 2 files changed, 50 insertions(+), 33 deletions(-) diff --git a/udf-runner-cpp/v2/waitable_queue_integration_test.cc b/udf-runner-cpp/v2/waitable_queue_integration_test.cc index 54784e4..0bff948 100644 --- a/udf-runner-cpp/v2/waitable_queue_integration_test.cc +++ b/udf-runner-cpp/v2/waitable_queue_integration_test.cc @@ -27,81 +27,97 @@ class SpscEpollTest : public testing::Test protected: void SetUp() override { - ASSERT_NE(epoll_fd, -1); - ASSERT_EQ(::socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, sockets.data()), 0); - ASSERT_TRUE(add_to_epoll(queue.native_handle(), EPOLLIN)); - ASSERT_TRUE(add_to_epoll(sockets[1], EPOLLIN)); + ASSERT_NE(epoll_fd_storage, -1); + ASSERT_EQ(::socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, sockets_storage.data()), 0); + ASSERT_TRUE(add_to_epoll(queue_storage.native_handle(), EPOLLIN)); + ASSERT_TRUE(add_to_epoll(sockets_storage[1], EPOLLIN)); } void TearDown() override { - ::close(sockets[0]); - ::close(sockets[1]); - ::close(epoll_fd); + ::close(sockets_storage[0]); + ::close(sockets_storage[1]); + ::close(epoll_fd_storage); } - bool add_to_epoll(int fd, std::uint32_t events) + bool add_to_epoll(int fd, std::uint32_t events) const { epoll_event event{}; event.events = events; event.data.fd = fd; - return ::epoll_ctl(epoll_fd, EPOLL_CTL_ADD, fd, &event) == 0; + return ::epoll_ctl(epoll_fd_storage, EPOLL_CTL_ADD, fd, &event) == 0; } - exasol::udf::v2::WaitableSpscQueue queue; - int epoll_fd = ::epoll_create1(EPOLL_CLOEXEC); - std::array sockets{}; + exasol::udf::v2::WaitableSpscQueue& queue() noexcept + { + return queue_storage; + } + + [[nodiscard]] int epoll_fd() const noexcept + { + return epoll_fd_storage; + } + + [[nodiscard]] const std::array& sockets() const noexcept + { + return sockets_storage; + } + +private: + exasol::udf::v2::WaitableSpscQueue queue_storage; + int epoll_fd_storage = ::epoll_create1(EPOLL_CLOEXEC); + std::array sockets_storage{}; }; } // namespace TEST_F(SpscEpollTest, ReportsQueueAndSocketReadiness) { - ASSERT_TRUE(queue.enqueue(42)); + ASSERT_TRUE(queue().enqueue(42)); const char byte = 'x'; - ASSERT_EQ(::write(sockets[0], &byte, sizeof(byte)), sizeof(byte)); + ASSERT_EQ(::write(sockets()[0], &byte, sizeof(byte)), sizeof(byte)); std::array events{}; - const int event_count = ::epoll_wait(epoll_fd, events.data(), events.size(), 1000); + const int event_count = ::epoll_wait(epoll_fd(), events.data(), events.size(), 1000); ASSERT_EQ(event_count, 2); bool queue_ready = false; bool socket_ready = false; for (const auto& event : std::span(events).first(static_cast(event_count))) { - queue_ready |= event.data.fd == queue.native_handle(); - socket_ready |= event.data.fd == sockets[1]; + queue_ready |= event.data.fd == queue().native_handle(); + socket_ready |= event.data.fd == sockets()[1]; } EXPECT_TRUE(queue_ready); EXPECT_TRUE(socket_ready); - EXPECT_EQ(queue.drain_notifications(), 1); + EXPECT_EQ(queue().drain_notifications(), 1); } TEST_F(SpscEpollTest, SupportsQueueOperationsAndBatches) { - ASSERT_TRUE(queue.enqueue(42)); - EXPECT_EQ(queue.drain_notifications(), 1); + ASSERT_TRUE(queue().enqueue(42)); + EXPECT_EQ(queue().drain_notifications(), 1); int value = 0; - ASSERT_TRUE(queue.try_dequeue(value)); + ASSERT_TRUE(queue().try_dequeue(value)); EXPECT_EQ(value, 42); const std::vector batch{1, 2, 3}; - EXPECT_EQ(queue.enqueue_batch(batch.begin(), batch.end()), batch.size()); - EXPECT_EQ(queue.drain_notifications(), 1); + EXPECT_EQ(queue().enqueue_batch(batch.begin(), batch.end()), batch.size()); + EXPECT_EQ(queue().drain_notifications(), 1); for (int expected : batch) { - ASSERT_TRUE(queue.try_dequeue(value)); + ASSERT_TRUE(queue().try_dequeue(value)); EXPECT_EQ(value, expected); } - EXPECT_FALSE(queue.try_dequeue(value)); + EXPECT_FALSE(queue().try_dequeue(value)); const std::array empty_batch{}; - EXPECT_EQ(queue.enqueue_batch(empty_batch.begin(), empty_batch.end()), 0); - EXPECT_EQ(queue.drain_notifications(), 0); + EXPECT_EQ(queue().enqueue_batch(empty_batch.begin(), empty_batch.end()), 0); + EXPECT_EQ(queue().drain_notifications(), 0); - const auto& const_queue = queue; - EXPECT_EQ(&const_queue.queue(), &queue.queue()); + const auto& const_queue = queue(); + EXPECT_EQ(&const_queue.queue(), &queue().queue()); } TEST_F(SpscEpollTest, SupportsMoves) diff --git a/udf-runner-cpp/v2/waitable_queue_test.cc b/udf-runner-cpp/v2/waitable_queue_test.cc index a241985..ac60f9d 100644 --- a/udf-runner-cpp/v2/waitable_queue_test.cc +++ b/udf-runner-cpp/v2/waitable_queue_test.cc @@ -56,7 +56,11 @@ class MockQueue class MockEventFd : public exasol::udf::v2::EventFd { public: - MOCK_METHOD(int, native_handle, (), (const, noexcept, override)); + [[nodiscard]] int native_handle() const noexcept override + { + return 42; + } + MOCK_METHOD(std::uint64_t, read_notification, (), (override)); MOCK_METHOD(void, write_notification, (), (override)); }; @@ -70,7 +74,6 @@ using testing::Throw; TEST(WaitableQueueTest, EnqueuesDequeuesAndDrainsNotifications) { auto event_fd = std::make_unique>(); - EXPECT_CALL(*event_fd, native_handle()).WillOnce(Return(42)); EXPECT_CALL(*event_fd, write_notification()).WillOnce(Return()); EXPECT_CALL(*event_fd, read_notification()) .WillOnce(Return(3)) @@ -203,7 +206,6 @@ TEST(WaitableQueueTest, HandlesZeroCapacity) TEST(WaitableQueueTest, SupportsMoveConstruction) { auto event_fd = std::make_unique>(); - EXPECT_CALL(*event_fd, native_handle()).WillOnce(Return(42)); using WaitableQueue = exasol::udf::v2::WaitableQueue; WaitableQueue moved_queue(MockQueue{}, std::move(event_fd)); @@ -214,7 +216,6 @@ TEST(WaitableQueueTest, SupportsMoveConstruction) TEST(WaitableQueueTest, SupportsMoveAssignmentAndSelfMove) { auto source_event_fd = std::make_unique>(); - EXPECT_CALL(*source_event_fd, native_handle()).WillOnce(Return(42)); auto target_event_fd = std::make_unique>(); using WaitableQueue = exasol::udf::v2::WaitableQueue;