seokjin0414 commented on code in PR #3554: URL: https://github.com/apache/iggy/pull/3554#discussion_r3521902066
########## bdd/cpp/scripts/entrypoint.sh: ########## @@ -0,0 +1,39 @@ +#!/bin/bash +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Copy the mounted shared feature files, start the cucumber-cpp wire server, then run the +# Ruby Cucumber driver against it. +set -euo pipefail + +if [ -d /app/features ]; then + cp -f /app/features/*.feature /workspace/bdd/cpp/features/ 2>/dev/null || true +fi + +bdd_wire_server & +server_pid=$! +sleep 2 + +cd /workspace/bdd/cpp || exit 1 + +set +e +bundle exec cucumber Review Comment: added --strict and a guard that exits 1 when no .feature files are present after the copy. verified both holes independently before fixing: an undefined step is exit 0 without --strict and exit 1 with it, and zero feature files is exit 0 even with --strict, so the guard covers the second one. full 17-step run stays green; with an extra undefined step appended the suite now fails (17 passed / 1 undefined, exit 1). ########## bdd/cpp/scripts/entrypoint.sh: ########## @@ -0,0 +1,39 @@ +#!/bin/bash +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Copy the mounted shared feature files, start the cucumber-cpp wire server, then run the +# Ruby Cucumber driver against it. +set -euo pipefail + +if [ -d /app/features ]; then + cp -f /app/features/*.feature /workspace/bdd/cpp/features/ 2>/dev/null || true +fi + +bdd_wire_server & +server_pid=$! +sleep 2 Review Comment: replaced with a poll loop (10s cap, exit 1 with a message on timeout). one catch found while testing: the wire server accepts exactly one connection, so a connect-based probe eats the slot cucumber needs and the run dies with 'unable to contact the wire server'. the probe reads /proc/net/tcp for port 3902 in LISTEN state instead, so nothing touches the socket. ########## bdd/cpp/features/step_definitions/messaging_steps.cpp: ########## @@ -0,0 +1,204 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// Unique step-class prefix so the generated CukeObject<n> symbols do not clash with the other +// step file when linked together (see background_steps.cpp for details). +#define CUKE_OBJECT_PREFIX IggyBddMessaging + +#include <gtest/gtest.h> + +#include <cucumber-cpp/autodetect.hpp> + +#include <cstddef> +#include <cstdint> +#include <string> +#include <utility> + +#include "iggy.hpp" +#include "world.hpp" + +GIVEN("^I have no streams in the system$") { + cucumber::ScenarioScope<bdd::GlobalContext> context; + ASSERT_NE(context->client, nullptr); + + const auto streams = context->client->get_streams(); + EXPECT_EQ(streams.size(), static_cast<std::size_t>(0)); +} + +WHEN("^I create a stream with name \"([^\"]{1,255})\"$") { + REGEX_PARAM(std::string, name); + cucumber::ScenarioScope<bdd::GlobalContext> context; + + context->client->create_stream(name); +} + +THEN("^the stream should be created successfully$") { + cucumber::ScenarioScope<bdd::GlobalContext> context; + + const auto streams = context->client->get_streams(); + EXPECT_EQ(streams.size(), static_cast<std::size_t>(1)); +} + +THEN("^the stream should have name \"([^\"]{1,255})\"$") { + REGEX_PARAM(std::string, name); + cucumber::ScenarioScope<bdd::GlobalContext> context; + + const auto stream = context->client->get_stream(bdd::make_numeric_identifier(0)); + EXPECT_EQ(std::string(stream.name), name); +} + +WHEN("^I create a topic with name \"([^\"]{1,255})\" in stream ([0-9]+) with ([0-9]+) partitions$") { + REGEX_PARAM(std::string, topic_name); + REGEX_PARAM(int, stream_id); + REGEX_PARAM(int, partitions_count); + cucumber::ScenarioScope<bdd::GlobalContext> context; + + // Drive the SDK through the typed helpers in iggy.hpp rather than raw strings. Options that + // still take plain strings below (partitioning kind, consumer kind) mark where the C++ SDK + // does not yet expose a typed wrapper. + const auto compression = iggy::CompressionAlgorithm::none(); + const auto message_expiry = iggy::Expiry::never_expire(); + const auto max_topic_size = iggy::MaxTopicSize::server_default(); + + context->client->create_topic(bdd::make_numeric_identifier(static_cast<std::uint32_t>(stream_id)), topic_name, + static_cast<std::uint32_t>(partitions_count), + std::string(compression.compression_algorithm_value()), 0, + std::string(message_expiry.expiry_kind()), message_expiry.expiry_value(), + std::string(max_topic_size.max_topic_size())); +} + +THEN("^the topic should be created successfully$") { + cucumber::ScenarioScope<bdd::GlobalContext> context; + + const auto stream = context->client->get_stream(bdd::make_numeric_identifier(0)); + EXPECT_FALSE(stream.topics.empty()); +} + +THEN("^the topic should have name \"([^\"]{1,255})\"$") { + REGEX_PARAM(std::string, name); + cucumber::ScenarioScope<bdd::GlobalContext> context; + + const auto stream = context->client->get_stream(bdd::make_numeric_identifier(0)); + ASSERT_FALSE(stream.topics.empty()); + EXPECT_EQ(std::string(stream.topics[0].name), name); +} + +THEN("^the topic should have ([0-9]+) partitions$") { + REGEX_PARAM(int, partitions_count); + cucumber::ScenarioScope<bdd::GlobalContext> context; + + const auto stream = context->client->get_stream(bdd::make_numeric_identifier(0)); + ASSERT_FALSE(stream.topics.empty()); + EXPECT_EQ(stream.topics[0].partitions_count, static_cast<std::uint32_t>(partitions_count)); +} + +WHEN("^I send ([0-9]+) messages to stream ([0-9]+), topic ([0-9]+), partition ([0-9]+)$") { + REGEX_PARAM(int, message_count); + REGEX_PARAM(int, stream_id); + REGEX_PARAM(int, topic_id); + REGEX_PARAM(int, partition_id); + cucumber::ScenarioScope<bdd::GlobalContext> context; + + rust::Vec<iggy::ffi::IggyMessageToSend> messages; + for (int index = 0; index < message_count; ++index) { + iggy::ffi::IggyMessageToSend message = + iggy::ffi::make_message(bdd::to_payload(bdd::expected_payload(static_cast<std::uint32_t>(index)))); + // Assign an explicit, 1-based id so the last-sent/last-polled comparison is meaningful. + message.id_lo = static_cast<std::uint64_t>(index + 1); + messages.push_back(std::move(message)); + } + + const auto sent = messages.size(); + context->client->send_messages(bdd::make_numeric_identifier(static_cast<std::uint32_t>(stream_id)), + bdd::make_numeric_identifier(static_cast<std::uint32_t>(topic_id)), "partition_id", + bdd::partition_id_bytes(static_cast<std::uint32_t>(partition_id)), + std::move(messages)); + + context->sent_count = static_cast<std::uint32_t>(sent); + context->messages_to_send = static_cast<std::uint32_t>(message_count); + context->last_sent_payload = bdd::expected_payload(static_cast<std::uint32_t>(message_count - 1)); + context->last_sent_id_lo = static_cast<std::uint64_t>(message_count); +} + +THEN("^all messages should be sent successfully$") { + cucumber::ScenarioScope<bdd::GlobalContext> context; + + EXPECT_EQ(context->sent_count, context->messages_to_send); Review Comment: dropped the bookkeeping — you're right, it could never fail. the then step now just documents that send_messages() throwing in the WHEN is the real signal, same shape as the rust suite. ########## bdd/cpp/features/step_definitions/world.hpp: ########## @@ -0,0 +1,111 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#pragma once + +#include <cstdint> +#include <string> +#include <vector> + +#include "lib.rs.h" + +namespace bdd { + +// Plain-C++ snapshot of a poll result so the scenario context never stores cxx types +// (rust::Vec) across steps. +struct PolledData { + std::uint32_t count = 0; + std::uint64_t current_offset = 0; Review Comment: dropped the field and the assignment. ########## bdd/cpp/Gemfile.lock: ########## @@ -0,0 +1,61 @@ +GEM + remote: https://rubygems.org/ + specs: + builder (3.3.0) + cucumber (7.1.0) + builder (~> 3.2, >= 3.2.4) + cucumber-core (~> 10.1, >= 10.1.0) + cucumber-create-meta (~> 6.0, >= 6.0.1) + cucumber-cucumber-expressions (~> 14.0, >= 14.0.0) + cucumber-gherkin (~> 22.0, >= 22.0.0) + cucumber-html-formatter (~> 17.0, >= 17.0.0) + cucumber-messages (~> 17.1, >= 17.1.1) + cucumber-wire (~> 6.2, >= 6.2.0) + diff-lcs (~> 1.4, >= 1.4.4) + mime-types (~> 3.3, >= 3.3.1) + multi_test (~> 0.1, >= 0.1.2) + sys-uname (~> 1.2, >= 1.2.2) + cucumber-core (10.1.1) + cucumber-gherkin (~> 22.0, >= 22.0.0) + cucumber-messages (~> 17.1, >= 17.1.1) + cucumber-tag-expressions (~> 4.1, >= 4.1.0) + cucumber-create-meta (6.0.4) + cucumber-messages (~> 17.1, >= 17.1.1) + sys-uname (~> 1.2, >= 1.2.2) + cucumber-cucumber-expressions (14.0.0) + cucumber-gherkin (22.0.0) + cucumber-messages (~> 17.1, >= 17.1.1) + cucumber-html-formatter (17.0.0) + cucumber-messages (~> 17.1, >= 17.1.0) + cucumber-messages (17.1.1) + cucumber-tag-expressions (4.1.0) + cucumber-wire (6.2.1) + cucumber-core (~> 10.1, >= 10.1.0) + cucumber-cucumber-expressions (~> 14.0, >= 14.0.0) + diff-lcs (1.6.2) + ffi (1.17.4) + ffi (1.17.4-arm64-darwin) + ffi (1.17.4-x86_64-darwin) + logger (1.7.0) + memoist3 (1.0.0) + mime-types (3.7.0) + logger + mime-types-data (~> 3.2025, >= 3.2025.0507) + mime-types-data (3.2026.0414) + multi_test (0.1.2) + sys-uname (1.5.1) + ffi (~> 1.1) + memoist3 (~> 1.0.0) + +PLATFORMS Review Comment: added x86_64-linux via bundle lock --add-platform (which also pins the precompiled ffi for the ci arch) and switched the deps stage to a frozen install, so gemfile/lock drift now fails the build instead of re-resolving. ########## bdd/cpp/.bazelrc: ########## @@ -0,0 +1,18 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +common --enable_bzlmod Review Comment: added a ci config mirroring foreign/cpp (build:ci --lockfile_mode=error) and the dockerfile stage-1 build now runs with --config=ci, so the committed lock is enforced where the image is built. verified the committed lock passes error mode on amd64 before wiring it in. ########## bdd/cpp/Dockerfile: ########## @@ -0,0 +1,71 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# C++ BDD harness, built in three stages so the runtime image carries only what the test run +# needs (~330 MB instead of ~10 GB for a single toolchain image): +# 1. build - Bazel builds the cucumber-cpp wire server. bdd/cpp is its own Bazel module and +# links the Iggy C++ SDK from foreign/cpp via local_path_override; rules_rust +# fetches its own Rust toolchain, so no system Rust is needed. +# 2. deps - Bundler installs the Ruby Cucumber runner (compiles the ffi native extension). +# 3. runtime - copies the wire server binary and the vendored gems, then runs cucumber. +# cucumber-cpp v0.8.0 is wire-only: the Ruby runner reads the shared feature files and drives +# the C++ wire server over the wire protocol. + +# Stage 1: build the wire server with Bazel. +FROM gcr.io/bazel-public/bazel:9.1.1 AS build + +USER root +WORKDIR /workspace +COPY . . + +RUN cd bdd/cpp \ + && bazel build //:bdd_wire_server \ + && cp bazel-bin/bdd_wire_server /tmp/bdd_wire_server + +# Stage 2: install the Ruby Cucumber runner. build-essential is needed to compile the ffi +# native extension and never reaches the runtime image. +FROM ruby:3.3-slim-trixie AS deps Review Comment: added the comment. verified it while at it: cucumber 7.1.0 on ruby 3.4.10 dies at require time with exactly that error — Hash.new(strict:, proc:) in multiline_argument/data_table.rb:78 — so the pin is indeed load-bearing. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
