Skip to content

chore: Add support for persistent store contract tests.#502

Open
kinyoklion wants to merge 6 commits intomainfrom
rlamb/persistence-contract-tests
Open

chore: Add support for persistent store contract tests.#502
kinyoklion wants to merge 6 commits intomainfrom
rlamb/persistence-contract-tests

Conversation

@kinyoklion
Copy link
Member

@kinyoklion kinyoklion commented Nov 3, 2025

Summary

Adds support for running persistent data store contract tests against the C++ server SDK's Redis integration.

Key changes:

  • CI action (.github/actions/ci/action.yml): Added use_redis input, passed as $4 to build.sh to explicitly control LD_BUILD_REDIS_SUPPORT.
  • scripts/build.sh: Added $4 parameter for explicit Redis support toggle (overrides automatic target-name detection when provided).
  • server-redis.yml: Added use_redis: true to all Redis workflow jobs to ensure Redis support is enabled now that $4=false (the default) would otherwise override auto-detection.
  • server.yml: Enabled use_redis: true and enable_persistence_tests: true for both contract test jobs.
  • Contract test data model: Added ConfigPersistentCache, ConfigPersistentStore, and ConfigPersistentDataStore structs; wired persistentDataStore into ConfigParams.
  • Contract test service (entity_manager.cpp): Implemented Redis persistent store creation and cache mode configuration (off, ttl, infinite) behind LD_REDIS_SUPPORT_ENABLED compile flag.
  • Capability registration (main.cpp): Advertises persistent-data-store-redis when Redis support is compiled in.
  • CMakeLists.txt: Conditionally links launchdarkly::server_redis_source and defines LD_REDIS_SUPPORT_ENABLED when LD_BUILD_REDIS_SUPPORT is ON.

Updates since last revision:

Review & Testing Checklist for Human

  • Verify the "infinite" cache mode implementation (24 * 365 hours TTL) matches what the contract test harness expects — confirm there isn't a dedicated "infinite" API on LazyLoadBuilder.
  • Verify the "off" cache mode (CacheRefresh(std::chrono::seconds(0))) actually disables caching in the LazyLoadBuilder API as intended (i.e., 0 seconds means "don't cache" rather than "always stale").
  • Confirm that the build.sh $4 parameter logic doesn't break any other callers of build.sh that may pass positional arguments differently. Note: when $4 is not provided, the variable is unset and the existing auto-detection logic is preserved.
  • Run the persistence contract tests end-to-end against a Redis instance to confirm they pass. CI builds the test binary with Redis support, but Mac/Windows Redis tests are still TODO (no Redis service available on those runners).

Notes

kinyoklion and others added 5 commits November 3, 2025 15:34
Co-Authored-By: rlamb@launchdarkly.com <kingdewman@gmail.com>
Co-Authored-By: rlamb@launchdarkly.com <kingdewman@gmail.com>
…s CI

Co-Authored-By: rlamb@launchdarkly.com <kingdewman@gmail.com>
@kinyoklion
Copy link
Member Author

Devin, please update with the latest changes from main.

@devin-ai-integration
Copy link
Contributor

Done — merged latest main (includes PR #505 and #506 changes) into this branch. Waiting for CI now.

@kinyoklion kinyoklion marked this pull request as ready for review March 10, 2026 21:29
@kinyoklion kinyoklion requested a review from a team as a code owner March 10, 2026 21:29
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

# Inform the test harness of test service's port.
test_service_port: ${{ env.TEST_SERVICE_PORT }}
token: ${{ secrets.GITHUB_TOKEN }}
enable_persistence_tests: true
Copy link

Choose a reason for hiding this comment

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

Missing Redis service container for persistence contract tests

High Severity

The contract-tests and contract-tests-curl jobs enable enable_persistence_tests: true and build with use_redis: true, but neither job defines a Redis services: container. Contrast with server-redis.yml, which properly includes a services: redis: block with the Redis image and port mapping. The persistence tests will attempt to connect to Redis at runtime and fail because no instance is running.

Additional Locations (1)
Fix in Cursor Fix in Web

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.

1 participant