Skip to content

Conversation

@Saumya-R
Copy link
Contributor

This PR adds comprehensive C++ test coverage for KVS (Key-Value Store) snapshot functionality, enabling the existing Python test suite to run against both Rust and C++ implementations. The changes introduce new C++ test scenarios that mirror the Python snapshot tests, along with necessary helper utilities for KVS parameter parsing and instance creation.

Key Changes:

  • Extended Python test parametrization to include C++ version alongside Rust
  • Added C++ implementations for four snapshot test scenarios (count, max_count, restore, paths)
  • Created helper utilities for KVS parameter parsing and instance management

@github-actions
Copy link

github-actions bot commented Dec 11, 2025

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run //:license-check

Status: ⚠️ Needs Review

Click to expand output
[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.3.0) and connecting to it...
INFO: Invocation ID: 1581027d-8dcf-48a3-915a-bf694e3a80f2
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
DEBUG: Rule 'rust_qnx8_toolchain+' indicated that a canonical reproducible form can be obtained by modifying arguments integrity = "sha256-oEubHgeZDdT0svMmBKJx7c3/2TdSI/vfwRUyDn+TPGA="
DEBUG: Repository rust_qnx8_toolchain+ instantiated at:
  <builtin>: in <toplevel>
Repository rule http_archive defined at:
  /home/runner/.bazel/external/bazel_tools/tools/build_defs/repo/http.bzl:394:31: in <toplevel>
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
WARNING: For repository 'rules_rust', the root module requires module version rules_rust@0.56.0, but got rules_rust@0.61.0 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'rules_cc', the root module requires module version rules_cc@0.1.1, but got rules_cc@0.1.2 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'platforms', the root module requires module version platforms@0.0.11, but got platforms@1.0.0 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
Computing main repo mapping: 
Loading: 
Loading: 5 packages loaded
Loading: 5 packages loaded
    currently loading: 
Analyzing: target //:license-check (6 packages loaded, 0 targets configured)
Analyzing: target //:license-check (6 packages loaded, 0 targets configured)

Analyzing: target //:license-check (90 packages loaded, 9 targets configured)

Analyzing: target //:license-check (93 packages loaded, 9 targets configured)

Analyzing: target //:license-check (142 packages loaded, 916 targets configured)

Analyzing: target //:license-check (163 packages loaded, 6982 targets configured)

Analyzing: target //:license-check (163 packages loaded, 6985 targets configured)

INFO: Analyzed target //:license-check (166 packages loaded, 9001 targets configured).
[13 / 14] [Prepa] Generating Dash formatted dependency file ...
INFO: From Generating Dash formatted dependency file ...:
INFO: Successfully converted 65 packages from Cargo.lock to bazel-out/k8-fastbuild/bin/formatted.txt
INFO: Found 1 target...
Target //:license.check.license_check up-to-date:
  bazel-bin/license.check.license_check
  bazel-bin/license.check.license_check.jar
INFO: Elapsed time: 25.413s, Critical Path: 0.39s
INFO: 14 processes: 5 disk cache hit, 9 internal.
INFO: Build completed successfully, 14 total actions
INFO: Running command line: bazel-bin/license.check.license_check ./formatted.txt <args omitted>
usage: org.eclipse.dash.licenses.cli.Main [-batch <int>] [-cd <url>]
       [-confidence <int>] [-ef <url>] [-excludeSources <sources>] [-help] [-lic
       <url>] [-project <shortname>] [-repo <url>] [-review] [-summary <file>]
       [-timeout <seconds>] [-token <token>]

@github-actions
Copy link

The created documentation from the pull request is available at: docu-html

@Saumya-R Saumya-R requested review from arkjedrz, Copilot and igorostrowskiq and removed request for Copilot and igorostrowskiq December 11, 2025 13:47
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR extends the existing KVS snapshot test coverage by enabling Python tests to run against both Rust and C++ implementations. The changes add C++ test scenarios that parallel the Python snapshot tests, along with helper utilities for parameter parsing and KVS instance creation.

Key Changes:

  • Extended Python test parametrization to include "cpp" alongside "rust"
  • Added C++ implementations for four snapshot test scenarios with known failing cases marked via pytest.xfail
  • Created helper utilities for parsing KVS parameters from JSON and managing KVS instances

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tests/python_test_cases/tests/test_cit_snapshots.py Added "cpp" to test parametrization and marked known failures with xfail references
tests/cpp_test_scenarios/src/main.cpp Integrated snapshot test group into the test harness
tests/cpp_test_scenarios/src/helpers/kvs_parameters.hpp New helper for parsing KVS parameters from JSON input
tests/cpp_test_scenarios/src/helpers/kvs_instance.hpp New helper for creating KVS instances from parsed parameters
tests/cpp_test_scenarios/src/cit/test_snapshots.hpp Declaration of four snapshot test scenario classes
tests/cpp_test_scenarios/src/cit/test_snapshots.cpp Implementation of snapshot test scenarios (count, max_count, restore, paths)
tests/cpp_test_scenarios/BUILD Updated build configuration to use glob for source files

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Saumya-R Saumya-R force-pushed the saumya_snapshot_test_cases branch from c920f42 to 3ff4b3d Compare December 12, 2025 09:38
@Saumya-R Saumya-R force-pushed the saumya_snapshot_test_cases branch from ad66d5e to d5afe29 Compare December 18, 2025 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants