[C++] Expand test coverage: unit, concurrency, and integration tests#499
Open
zlata-stefanovic-db wants to merge 3 commits into
Open
[C++] Expand test coverage: unit, concurrency, and integration tests#499zlata-stefanovic-db wants to merge 3 commits into
zlata-stefanovic-db wants to merge 3 commits into
Conversation
zlata-stefanovic-db
changed the title
Cover the API surface the current suite left untested, in the same dependency-free, network-free style as the existing tests (each a plain executable registered with CTest): - error_test: ZerobusException message + is_retryable + std::exception catch - record_test: UnackedRecord byte/flag preservation (incl. embedded NUL) - version_test: version() pinned to ZEROBUS_CPP_VERSION - proto_schema_test: UC-JSON round trip, error paths, move semantics - headers_callback_test: headers trampoline marshalling, empty / embedded-NUL / throwing / null-user_data guards, freed via zerobus_free_headers - sdk_test: offline build, move, create_stream / create_arrow_stream argument validation Mirrors the parity set the other SDKs test (cf. Java's unit tests) and the suite intended in #421. Full suite passes, including under AddressSanitizer. Signed-off-by: Zlata Stefanovic <zlata.stefanovic@databricks.com>
Add concurrency_test.cpp and integration_test.cpp, refine existing test suites, wire new tests into CMake, extend the C++ CI workflow, and update NEXT_CHANGELOG.md. Signed-off-by: Zlata Stefanovic <zlata.stefanovic@databricks.com>
Add a 'Running the tests' section to the C++ README (sanitizer runs and the env-var-gated integration_test), note it in NEXT_CHANGELOG, and make integration_test explain its flush()/close() sequence and warn when the default record is used. Signed-off-by: Zlata Stefanovic <zlata.stefanovic@databricks.com>
zlata-stefanovic-db
force-pushed
the
cpp-comprehensive-tests
branch
from
c67a894 to
1fc1569
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Expands the C++ SDK test suite to cover the API surface the current tests left uncovered, matching the parity set the other SDKs test (cf. Java's unit tests) and the suite intended in the closed #421. Also adds concurrency and live-integration coverage, plus a ThreadSanitizer CI job. All hermetic tests are plain, dependency-free, network-free executables registered with CTest, in the same style as the existing tests.
Unit tests (API surface)
ZerobusException: message,is_retryable(), catchable asstd::exceptionUnackedRecord: byte/flag preservation, embedded NUL, empty payloadversion()pinned toZEROBUS_CPP_VERSIONProtoSchema: UC-JSON round trip, invalid JSON, missing required column, move semantics (moved-from null-handle contract)user_dataguards; results freed viazerobus_free_headersSdk/SdkBuilder: offline build (no network I/O), move, andcreate_stream/create_arrow_streamargument validationConcurrency
ProtoSchema" contract (descriptor_bytes/encode_jsonareconst) across many threads. Hermetic, so it runs in CI. Aimed at ThreadSanitizer to catch data races the AddressSanitizer job cannot. No concurrent-Streamcase (aStreamis not safe for concurrent use, so that would test misuse).make test SANITIZE=thread) toci-cpp.yml.Live integration
create_stream -> ingest -> flush -> closepath against a real endpoint, mirroring the Java/TypeScript integration suites. Uses the loop-then-flush()pattern (never waits per record). Skips (exits 0) when credentials are unset, somake testand CI stay hermetic. RequiresZEROBUS_SERVER_ENDPOINT,DATABRICKS_WORKSPACE_URL,ZEROBUS_TABLE_NAME,DATABRICKS_CLIENT_ID,DATABRICKS_CLIENT_SECRET; optionalZEROBUS_TEST_RECORD_JSON.Testing
make test— full suite passes (13 tests;integration_testskips without credentials)make test SANITIZE=address— passes under AddressSanitizer (validates the FFI alloc/free pairing in the headers/proto-schema tests)make test SANITIZE=thread— passes under ThreadSanitizer (validates the shared-ProtoSchemaconcurrency contract)make fmt-check— cleanMerge order
Stacked on #484 (
cpp-ack-callback). Base iscpp-ack-callback; retarget tomainonce #484 merges. Draft until then.