Skip to content

test(bdd): add C++ SDK BDD tests for basic messaging#3554

Open
seokjin0414 wants to merge 3 commits into
apache:masterfrom
seokjin0414:2965-cpp-bdd-tests
Open

test(bdd): add C++ SDK BDD tests for basic messaging#3554
seokjin0414 wants to merge 3 commits into
apache:masterfrom
seokjin0414:2965-cpp-bdd-tests

Conversation

@seokjin0414

@seokjin0414 seokjin0414 commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

What

Adds BDD coverage for the C++ SDK by implementing the shared
bdd/scenarios/basic_messaging.feature with cucumber-cpp and wiring it into the
Docker-based BDD harness and CI, the same way the other SDKs run. This is the
second part of #2965; the messaging FFI it relies on landed in #3046.

How it works

cucumber-cpp only supports the Cucumber wire protocol, so the step definitions
(bdd/cpp/features/step_definitions) build with CMake into a wire server, and a
Ruby Cucumber runner (cucumber + cucumber-wire, pinned to the versions
cucumber-cpp itself tests against) drives the feature file over the wire against a
real iggy-server. The step logic and assertions are all C++ and exercise the
SDK's FFI (get_streams, create_stream, create_topic, send_messages,
poll_messages). The Iggy C++ SDK is built with cargo and linked into the wire
server as a staticlib.

Heads-up for reviewers

Because cucumber-cpp is wire-protocol-only, the C++ BDD image pulls in a Ruby
runtime (cucumber + cucumber-wire). Flagging this explicitly in case it
changes how you'd like the harness shaped.

Notes

Everything lives under bdd/cpp, next to the other SDK BDD suites; foreign/cpp
is unchanged apart from the reserve() fix to the existing test.

The TearDown fixture and the magic-number cleanup discussed on Discord are tracked
separately by @slbotbm.

Refs #2965

@seokjin0414 seokjin0414 marked this pull request as ready for review June 23, 2026 09:57
@github-actions github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Jun 23, 2026
@slbotbm

slbotbm commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

@seokjin0414 the test should be in he root bdd/ folder, not inside of foreign/cpp

@slbotbm

slbotbm commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

ignore the above, i didn't read the description😅

@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.00%. Comparing base (307fdb1) to head (023d49d).

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3554      +/-   ##
============================================
- Coverage     74.07%   74.00%   -0.07%     
  Complexity      937      937              
============================================
  Files          1249     1249              
  Lines        128248   128248              
  Branches     104116   104162      +46     
============================================
- Hits          94994    94912      -82     
- Misses        30219    30250      +31     
- Partials       3035     3086      +51     
Components Coverage Δ
Rust Core 74.67% <ø> (-0.05%) ⬇️
Java SDK 62.44% <ø> (ø)
C# SDK 71.43% <ø> (-0.63%) ⬇️
Python SDK 88.88% <ø> (ø)
PHP SDK 84.29% <ø> (ø)
Node SDK 91.35% <ø> (ø)
Go SDK 40.14% <ø> (ø)
see 40 files with indirect coverage changes
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@seokjin0414

Copy link
Copy Markdown
Contributor Author

@slbotbm
no worries! ended up moving it to bdd/cpp anyway. i kept it out of foreign/cpp by building the wire server with CMake (FetchContent cucumber-cpp + the cxx staticlib) instead of Bazel, so it's self-contained like the other bdd dirs and
foreign/cpp now only carries the reserve() fix.
can switch it to a Bazel target under foreign/cpp if you'd prefer.

@slbotbm

slbotbm commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

@seokjin0414 I didn't understand why did you use cmake instead of bazel. I think it would be possible to use bazel in bdd/cpp. you could define iggy_cpp as a bazel dependency in bdd/cpp/MODULE.bazel, and then locally define the path to iggy_cpp as ../../foreign/cpp.

@slbotbm

slbotbm commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

/author

@github-actions github-actions Bot added S-waiting-on-author PR is waiting on author response and removed S-waiting-on-review PR is waiting on a reviewer labels Jun 24, 2026
@seokjin0414

Copy link
Copy Markdown
Contributor Author

@slbotbm the cmake route was just to keep the test self-contained in bdd/cpp without touching foreign/cpp — i didn't realize local_path_override could pull the sdk in cleanly. it can, so i switched to bazel as you suggested: bdd/cpp is its own module now, cucumber-cpp from the registry + iggy_cpp via local_path_override(../../foreign/cpp). cmake is gone.

the only change to foreign/cpp is one line — making the iggy-cpp target public so the harness can depend on it. build + the full wire e2e (17 steps) pass.

@slbotbm slbotbm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a .clang-format as well that copies the one from foreign/cpp

You can use /ready when your PR is ready for review again

Comment thread bdd/cpp/BUILD.bazel
Comment on lines +25 to +47
cc_binary(
name = "bdd_wire_server",
srcs = glob([
"features/step_definitions/*.cpp",
"features/step_definitions/*.hpp",
]),
copts = select({
"@platforms//os:windows": ["/utf-8"],
"//conditions:default": [
"-finput-charset=UTF-8",
"-fexec-charset=UTF-8",
],
}),
includes = [
"features/step_definitions",
],
testonly = True,
deps = [
"@cucumber-cpp//:cucumber-cpp",
"@googletest//:gtest",
"@iggy_cpp//:iggy-cpp",
],
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add the following flags: -Wall, -Wpedantic, -Werror, -Wextra, -g3, -O0, -fno-omit-frame-pointer, -DDEBUG

Also linkopts = ["-g"]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added all of those to the cc_binary copts, plus linkopts = ["-g"]. compiles clean under -Werror -Wpedantic.

Comment thread bdd/cpp/Dockerfile Outdated
Comment on lines +57 to +73
RUN printf '%s\n' \
'#!/bin/bash' \
'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' \
'set +e' \
'bundle exec cucumber' \
'status=$?' \
'kill "$server_pid" 2>/dev/null || true' \
'exit $status' \
> /entrypoint.sh \
&& chmod +x /entrypoint.sh

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a real scripts/entrypoint.sh instead of creating it on the fly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done — it's now bdd/cpp/scripts/entrypoint.sh (COPY'd in, shellcheck-clean).

Comment thread foreign/cpp/BUILD.bazel
Comment on lines +132 to 133
visibility = ["//visibility:public"],
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a version to iggy_cpp (0.1.0)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set version = "0.1.0" on the iggy_cpp module, and matched it in the bdd/cpp/MODULE.bazel bazel_dep.

Comment thread bdd/cpp/.bazelrc Outdated
Comment on lines +24 to +26
# cucumber-cpp and its transitive deps (asio, tclap, ...) are not clean under strict warnings.
# Silence warnings for external sources so they never fail the wire-server build.
build --per_file_copt=external/.*@-w

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By adding the flags to BUILD.bazel, this is not required anymore.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right — removed it. the warning flags are on the target copts now, so external deps aren't affected.

Comment thread bdd/cpp/.bazelrc Outdated
Comment on lines +20 to +22
# This harness resolves its module graph fresh in the build environment (Docker), so it does
# not commit a MODULE.bazel.lock.
common --lockfile_mode=off

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this and commit the lockfile

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed lockfile_mode=off and committed MODULE.bazel.lock.

Comment thread bdd/cpp/MODULE.bazel Outdated
Comment on lines +22 to +25
bazel_dep(name = "rules_cc", version = "0.2.18")
bazel_dep(name = "platforms", version = "1.1.0")
bazel_dep(name = "googletest", version = "1.17.0.bcr.2")
bazel_dep(name = "cucumber-cpp", version = "0.8.0.bcr.1")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is platforms required?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no — it was only used by the windows copts select, and the wire server is linux-only, so i dropped both the select and the platforms dep.

@slbotbm

slbotbm commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Let's also commit the ruby lockfile as well.

The oversized-payload test grew its 64 MB buffer one byte at a time and a comment claimed cxx::Vec had no public reserve API. It does, so reserve the capacity once before filling it.

Signed-off-by: seokjin0414 <sars21@hanmail.net>
The C++ SDK had no BDD coverage. Add cucumber-cpp step definitions under
bdd/cpp that drive the shared basic_messaging.feature through the C++ FFI
client.

cucumber-cpp uses the wire protocol, so bdd/cpp is a self-contained Bazel
module: it builds the step definitions into a wire server (pulling
cucumber-cpp from the Bazel registry and the Iggy C++ SDK from foreign/cpp
via local_path_override) that a Ruby Cucumber runner connects to. The SDK's
iggy-cpp target is made public so the harness can depend on it. A per-file
CUKE_OBJECT_PREFIX keeps the generated step classes from clashing across
translation units. The Gemfile and cucumber.wire comment styles are
registered with the license checker.

Signed-off-by: seokjin0414 <sars21@hanmail.net>
A Ruby cucumber runner with cucumber-wire drives the wire server over the
wire protocol against a real iggy-server. Register the cpp-bdd compose
service, the run-bdd-tests entry, and the bdd-cpp CI component so it runs
like the other SDK BDD suites.

Signed-off-by: seokjin0414 <sars21@hanmail.net>
@seokjin0414

seokjin0414 commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

@slbotbm
also addressed the two non-inline ones: added bdd/cpp/.clang-format (copied from foreign/cpp), and committed bdd/cpp/Gemfile.lock for the ruby side. rebased onto master — CI is green.

@seokjin0414

Copy link
Copy Markdown
Contributor Author

/ready

@seokjin0414 seokjin0414 requested a review from slbotbm June 28, 2026 15:45
@github-actions github-actions Bot added S-waiting-on-review PR is waiting on a reviewer and removed S-waiting-on-author PR is waiting on author response labels Jun 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review PR is waiting on a reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants