slbotbm commented on code in PR #3554: URL: https://github.com/apache/iggy/pull/3554#discussion_r3506932843
########## 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 + +RUN apt-get update && apt-get install --yes --no-install-recommends \ + build-essential \ + && rm -rf /var/lib/apt/lists/* + +WORKDIR /workspace/bdd/cpp +COPY bdd/cpp/Gemfile bdd/cpp/Gemfile.lock ./ +RUN gem install bundler --no-document \ Review Comment: Let's pin bundler to a specific version ########## bdd/cpp/features/step_definitions/background_steps.cpp: ########## @@ -0,0 +1,62 @@ +/* + * 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. + */ + +// cucumber-cpp names its generated step classes CukeObject<__COUNTER__>, and the counter +// restarts in every translation unit. A unique prefix per step file keeps those names from +// colliding when several step files are linked into one wire server. +#define CUKE_OBJECT_PREFIX IggyBddBackground + +#include <gtest/gtest.h> + +#include <cucumber-cpp/autodetect.hpp> + +#include <cstdlib> +#include <string> + +#include "world.hpp" + +using cucumber::ScenarioScope; Review Comment: Let's remove this line and explicitly use `cucumber::ScenarioScope` whever it is called. ########## bdd/cpp/features/step_definitions/messaging_steps.cpp: ########## @@ -0,0 +1,192 @@ +/* + * 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 "world.hpp" + +using cucumber::ScenarioScope; + +GIVEN("^I have no streams in the system$") { + 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 \"(.*)\"$") { Review Comment: Let's change the regex from `(.*)` to `([^"]{1,255})` to reflect actual stream name restrictions ########## bdd/cpp/features/step_definitions/messaging_steps.cpp: ########## @@ -0,0 +1,192 @@ +/* + * 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 "world.hpp" + +using cucumber::ScenarioScope; + +GIVEN("^I have no streams in the system$") { + 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 \"(.*)\"$") { Review Comment: Also do for topic as well. ########## bdd/cpp/features/step_definitions/messaging_steps.cpp: ########## @@ -0,0 +1,192 @@ +/* + * 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 "world.hpp" + +using cucumber::ScenarioScope; + +GIVEN("^I have no streams in the system$") { + 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 \"(.*)\"$") { + REGEX_PARAM(std::string, name); + ScenarioScope<bdd::GlobalContext> context; + + context->client->create_stream(name); +} + +THEN("^the stream should be created successfully$") { + 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 \"(.*)\"$") { + REGEX_PARAM(std::string, name); + 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 \"(.*)\" in stream ([0-9]+) with ([0-9]+) partitions$") { + REGEX_PARAM(std::string, topic_name); + REGEX_PARAM(int, stream_id); + REGEX_PARAM(int, partitions_count); + ScenarioScope<bdd::GlobalContext> context; + + context->client->create_topic(bdd::make_numeric_identifier(static_cast<std::uint32_t>(stream_id)), topic_name, + static_cast<std::uint32_t>(partitions_count), "none", 0, "never_expire", 0, + "server_default"); Review Comment: Instead of passing strings like "server_default" and "never_expire" here, I think we can use the functions and classes in `iggy.hpp` ########## bdd/cpp/features/step_definitions/messaging_steps.cpp: ########## @@ -0,0 +1,192 @@ +/* + * 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 "world.hpp" + +using cucumber::ScenarioScope; + +GIVEN("^I have no streams in the system$") { + 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 \"(.*)\"$") { + REGEX_PARAM(std::string, name); + ScenarioScope<bdd::GlobalContext> context; + + context->client->create_stream(name); +} + +THEN("^the stream should be created successfully$") { + 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 \"(.*)\"$") { + REGEX_PARAM(std::string, name); + 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 \"(.*)\" in stream ([0-9]+) with ([0-9]+) partitions$") { + REGEX_PARAM(std::string, topic_name); + REGEX_PARAM(int, stream_id); + REGEX_PARAM(int, partitions_count); + ScenarioScope<bdd::GlobalContext> context; + + context->client->create_topic(bdd::make_numeric_identifier(static_cast<std::uint32_t>(stream_id)), topic_name, + static_cast<std::uint32_t>(partitions_count), "none", 0, "never_expire", 0, + "server_default"); Review Comment: can also be done for the rest of the functions. This will help us understand what the sdk is not providing right now. ########## bdd/cpp/features/step_definitions/messaging_steps.cpp: ########## @@ -0,0 +1,192 @@ +/* + * 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 "world.hpp" + +using cucumber::ScenarioScope; + +GIVEN("^I have no streams in the system$") { + 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 \"(.*)\"$") { + REGEX_PARAM(std::string, name); + ScenarioScope<bdd::GlobalContext> context; + + context->client->create_stream(name); +} + +THEN("^the stream should be created successfully$") { + 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 \"(.*)\"$") { + REGEX_PARAM(std::string, name); + 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 \"(.*)\" in stream ([0-9]+) with ([0-9]+) partitions$") { + REGEX_PARAM(std::string, topic_name); + REGEX_PARAM(int, stream_id); + REGEX_PARAM(int, partitions_count); + ScenarioScope<bdd::GlobalContext> context; + + context->client->create_topic(bdd::make_numeric_identifier(static_cast<std::uint32_t>(stream_id)), topic_name, + static_cast<std::uint32_t>(partitions_count), "none", 0, "never_expire", 0, + "server_default"); +} + +THEN("^the topic should be created successfully$") { + 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 \"(.*)\"$") { + REGEX_PARAM(std::string, name); + 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); + 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); + 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)); + } + + 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>(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$") { + ScenarioScope<bdd::GlobalContext> context; + + EXPECT_GT(context->sent_count, static_cast<std::uint32_t>(0)); Review Comment: can we tighten this to expect the exact message count? -- 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]
