Skip to content

ci: Run the persistence contract tests - #392

Merged
kinyoklion merged 2 commits into
dotnet-server-sdk-v9from
rlamb/contract-tests-persistence-v9
Nov 6, 2026
Merged

kinyoklion merged 2 commits into
dotnet-server-sdk-v9from
rlamb/contract-tests-persistence-v9

Conversation

@kinyoklion

@kinyoklion kinyoklion commented Nov 2, 2026 •

Copy link
Copy Markdown
Member

Summary

The server SDK's contract test service had no persistent store support: the data system store configuration in SdkClientEntity.cs was commented out with a TODO about the store packages, and the FDv1 persistentDataStore configuration was not handled. CI therefore ran both harness versions with enable_persistence_tests: 'false'. This change makes the service drive real Redis, DynamoDB, and Consul stores and runs the persistence suites in CI.

It is separate from the persistence fix in #390 (and #389 on main), which these tests do not cover yet; the harness gains tests for that behavior in a separate change.

Test service

  • The FDv2 dataSystem.store and storeMode configuration (harness v3) and the FDv1 top-level persistentDataStore configuration (harness v2) build the store with Components.PersistentDataStore, with the requested prefix and cache mode.
  • An FDv1 configuration with a store and no streaming, polling, or service endpoint configuration runs in daemon mode (Components.ExternalUpdatesOnly), which the v2 suite expects. Without this, the client tried to reach LaunchDarkly and failed to initialize.
  • An empty prefix means the store's default prefix, and a store configuration without a DSN is rejected.
  • The Redis and DynamoDB clients fail fast (500 ms timeouts, no DynamoDB retries), as the store recovery tests require. The Consul client does not retry.
  • The service advertises persistent-data-store-redis, persistent-data-store-dynamodb, persistent-data-store-consul, and persistent-data-store-recovery.
  • The service already takes its port as an optional first argument; CI now passes one. Its default stays 8000.

Building against the local SDK

The store projects in pkgs/ reference the published LaunchDarkly.ServerSdk package, so a reference to them would bring a second copy of the SDK into the service. Instead, a small project per store under contract-tests/Stores/ compiles that store's sources against the SDK project, with the same client library and version range as the store's own project. There is one project per store because the Redis and DynamoDB stores share internal type names. The store projects themselves are not changed, and neither are their packages or their builds.

On this branch the SDK is 9.0.0-alpha, outside the stores' published range ([8.0.0, 9.0.0)); the stores build and pass against it from their sources.

CI

  • .github/actions/contract-tests gains enable_persistence_tests (default false) and test_service_port (default 8000). With persistence enabled, it starts the stores with launchdarkly/gh-actions/actions/persistent-stores, waits until they accept connections and Consul has a leader, and enables the tests in both harness runs.
  • The server SDK workflow enables the persistence tests on the Linux runner only, where the stores start in Docker, and passes port 9000 on every runner: the harness expects DynamoDB on port 8000, the service's default. The client SDK workflow uses the same action with the default port, because its test service always listens on 8000.

Suppressed tests

The persistence suites found three pre-existing defects in the store integrations. They are suppressed with a comment in each suppression file. The harness reads every non-blank line as a test ID, so a line that starts with # matches no test.

  1. A deleted flag is stored without a "key" property: {"version":N,"deleted":true}, from the ItemDescriptor serializer in DataStoreTypes.cs (Redis and Consul store that string), or as the placeholder "null" in the item attribute (DynamoDBDataStoreImpl.MarshalItem). The harness expects a JSON tombstone with a key property, the item key or $deleted. This fails read-write/data source deletions respect versioning for each store (both harness versions) and, in v3, the five store recovery tests that check a deleted flag after the write back. With a one-line serializer change that writes "key": "$deleted", the Redis and Consul tests pass, and the recovery write back itself works; DynamoDB needs its own fix.
  2. ConsulDataStoreImpl.GetAllAsync iterates result.Response, which is null when no key has the collection's prefix, so reading an empty collection throws. This fails collection cardinality/collection holds no flags for Consul.

These need SDK and store changes, which this PR does not make. For the keyless tombstone, the v3 harness's own tombstone suite lists {"version":N,"deleted":true} as a shape that .NET, Java, and Node write and that readers must accept, so for Redis and Consul the fix may belong in the harness's read-write matcher instead. DynamoDB's "null" placeholder is not one of the accepted shapes. Until one of these lands, CI does not check the store write back after recovery or the store side of deletions for any store; the only recovery test that runs checks evaluations from memory during the outage.

Testing

With Redis, DynamoDB local, and Consul running in Docker on their default ports:

  • Harness v2 (2.41.1), full suite with persistence enabled and the suppression file: all tests passed (4989 ran).
  • Harness v3 (3.2.0-alpha.15, from v3 at 58f4122), full suite with persistence enabled and the FDv2 suppression file: all tests passed (5174 ran).

Without the suppressions, the persistence suites fail exactly the tests listed above (4 in v2, 19 in v3).

After the review changes (the store projects, the port default, the readiness wait), both persistence suites pass again with the suppressions (v3: 141 ran; v2: 111 ran), and LaunchDarkly.ServerSdk.sln builds with no errors.

This is the dotnet-server-sdk-v9 version of #391.

Opened from termag

SDK-3493


Note

Overview
Enables the SDK test harness’s persistent-store suites in CI by wiring the contract test service to real Redis, DynamoDB, and Consul integrations and extending the shared contract-test GitHub Action.

The contract test service now maps harness FDv1 persistentDataStore and FDv2 dataSystem.store into Components.PersistentDataStore (Redis, DynamoDB, Consul) with prefix/cache options, 500 ms fail-fast client settings for recovery tests, and daemon mode (ExternalUpdatesOnly) when a store is configured without streaming/polling. It advertises the new persistence capabilities. Wrapper projects under Stores/ compile each integration’s sources against the in-repo SDK so CI does not pull a second published SDK copy.

CI: enable_persistence_tests and test_service_port are plumbed through the composite action; when enabled it starts stores, waits on ports 6379/8000/8500 and Consul leadership, and launches the service on a non-8000 port (DynamoDB local uses 8000). Server SDK workflow turns persistence on Linux only and uses port 9000.

Known integration gaps are temporarily skipped via suppression entries (deletion tombstone shape without "key", DynamoDB "null" deletes, Consul empty collection reads)—CI still runs the rest of the persistence suites.

Reviewed by Cursor Bugbot for commit 8f77f3f. Bugbot is set up for automated code reviews on this repo. Configure here.

The contract test service had no persistent store support: the data system
store configuration was commented out, and the FDv1 configuration was not
handled at all, so CI ran the harness with persistence tests disabled.

- The service configures a Redis, DynamoDB, or Consul store from the FDv2
  dataSystem.store configuration (harness v3) and from the FDv1
  persistentDataStore configuration (harness v2). An FDv1 configuration with
  a store and no data source runs in daemon mode, as the harness expects.
  The store clients fail fast, as the store recovery tests require.
- The service references the three store projects in this repository. They
  reference the published SDK package unless UseLocalServerSdk is set, which
  the service sets, so the service always runs against the local SDK.
- The service advertises the persistent-data-store-redis, -dynamodb, -consul
  and -recovery capabilities.
- CI starts the stores on the Linux runner and enables the persistence tests
  there. The service listens
  on port 9000, its first argument, since DynamoDB uses port 8000.

Pre-existing store format defects that the tests found are suppressed: a
deleted flag is written without a "key" property (Redis, Consul) or as the
placeholder "null" (DynamoDB), and the Consul store fails to read an empty
collection.
@kinyoklion

Copy link
Copy Markdown
Member Author

bugbot review

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

…r projects

From the review of this change:

- The contract-tests action defaulted its new port input to 9000, which the
  client SDK CI also uses, while the client test service always listens on
  8000. The default is 8000 again, and the server CI and nightly pass 9000.
- Switching the store projects to the local SDK with a property broke
  solution builds: a solution restore resolves the published SDK package for
  them, and the build then compiled them against both SDKs. The store
  projects are unchanged now. A small project under contract-tests/Stores
  builds each store from its sources against the local SDK; one project per
  store, because the stores share internal type names.
- The action waits until the stores accept connections and Consul has a
  leader before the tests start.
- The service treats a configuration with service endpoint URIs as having a
  data source when it decides on daemon mode, treats an empty prefix as no
  prefix, and rejects a store configuration without a DSN.
@kinyoklion

Copy link
Copy Markdown
Member Author

bugbot review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 8f77f3f. Configure here.

@kinyoklion
kinyoklion marked this pull request as ready for review November 3, 2026 23:25
@kinyoklion
kinyoklion requested a review from a team as a code owner November 3, 2026 23:25
@kinyoklion
kinyoklion merged commit cf74c7e into dotnet-server-sdk-v9 Nov 6, 2026
20 of 21 checks passed
@kinyoklion
kinyoklion deleted the rlamb/contract-tests-persistence-v9 branch November 6, 2026 17:11
kinyoklion added a commit that referenced this pull request Nov 6, 2026
<!-- TERMAG_PR_BODY_BEGIN -->
## Summary

The server SDK's contract test service had no persistent store support:
the data system store configuration in `SdkClientEntity.cs` was
commented out with a TODO about the store packages, and the FDv1
`persistentDataStore` configuration was not handled. CI therefore ran
both harness versions with `enable_persistence_tests: 'false'`. This
change makes the service drive real Redis, DynamoDB, and Consul stores
and runs the persistence suites in CI.

It is separate from the persistence fix in #389 (and #390 on
`dotnet-server-sdk-v9`), which these tests do not cover yet; the harness
gains tests for that behavior in a separate change.

## Test service

- The FDv2 `dataSystem.store` and `storeMode` configuration (harness v3)
and the FDv1 top-level `persistentDataStore` configuration (harness v2)
build the store with `Components.PersistentDataStore`, with the
requested prefix and cache mode.
- An FDv1 configuration with a store and no streaming, polling, or
service endpoint configuration runs in daemon mode
(`Components.ExternalUpdatesOnly`), which the v2 suite expects. Without
this, the client tried to reach LaunchDarkly and failed to initialize.
- An empty prefix means the store's default prefix, and a store
configuration without a DSN is rejected.
- The Redis and DynamoDB clients fail fast (500 ms timeouts, no DynamoDB
retries), as the store recovery tests require. The Consul client does
not retry.
- The service advertises `persistent-data-store-redis`,
`persistent-data-store-dynamodb`, `persistent-data-store-consul`, and
`persistent-data-store-recovery`.
- The service takes its port as an optional first argument, as on
`dotnet-server-sdk-v9`. Its default stays 8000.

### Building against the local SDK

The store projects in `pkgs/` reference the published
`LaunchDarkly.ServerSdk` package, so a reference to them would bring a
second copy of the SDK into the service. Instead, a small project per
store under `contract-tests/Stores/` compiles that store's sources
against the SDK project, with the same client library and version range
as the store's own project. There is one project per store because the
Redis and DynamoDB stores share internal type names. The store projects
themselves are not changed, and neither are their packages or their
builds.

## CI

- `.github/actions/contract-tests` gains `enable_persistence_tests`
(default `false`) and `test_service_port` (default `8000`). With
persistence enabled, it starts the stores with
`launchdarkly/gh-actions/actions/persistent-stores`, waits until they
accept connections and Consul has a leader, and enables the tests in
both harness runs.
- The server SDK workflow enables the persistence tests on the Linux
runner only, where the stores start in Docker, and passes port 9000 on
every runner: the harness expects DynamoDB on port 8000, the service's
default. The client SDK workflow uses the same action with the default
port, because its test service always listens on 8000.
- The nightly enables them too, also with port 9000.

## Suppressed tests

The persistence suites found three pre-existing defects in the store
integrations. They are suppressed with a comment in each suppression
file. The harness reads every non-blank line as a test ID, so a line
that starts with `#` matches no test.

1. A deleted flag is stored without a `"key"` property:
`{"version":N,"deleted":true}`, from the `ItemDescriptor` serializer in
`DataStoreTypes.cs` (Redis and Consul store that string), or as the
placeholder `"null"` in the item attribute
(`DynamoDBDataStoreImpl.MarshalItem`). The harness expects a JSON
tombstone with a `key` property, the item key or `$deleted`. This fails
`read-write/data source deletions respect versioning` for each store
(both harness versions) and, in v3, the five store recovery tests that
check a deleted flag after the write back. With a one-line serializer
change that writes `"key": "$deleted"`, the Redis and Consul tests pass,
and the recovery write back itself works; DynamoDB needs its own fix.
2. `ConsulDataStoreImpl.GetAllAsync` iterates `result.Response`, which
is null when no key has the collection's prefix, so reading an empty
collection throws. This fails `collection cardinality/collection holds
no flags` for Consul.

These need SDK and store changes, which this PR does not make. For the
keyless tombstone, the v3 harness's own tombstone suite lists
`{"version":N,"deleted":true}` as a shape that .NET, Java, and Node
write and that readers must accept, so for Redis and Consul the fix may
belong in the harness's read-write matcher instead. DynamoDB's `"null"`
placeholder is not one of the accepted shapes. Until one of these lands,
CI does not check the store write back after recovery or the store side
of deletions for any store; the only recovery test that runs checks
evaluations from memory during the outage.

## Testing

With Redis, DynamoDB local, and Consul running in Docker on their
default ports:

- Harness v2 (2.41.1), full suite with persistence enabled and the
suppression file: all tests passed (4989 ran).
- Harness v3 (3.2.0-alpha.15, from `v3` at 58f4122), full suite with
persistence enabled and the FDv2 suppression file: all tests passed
(5095 ran).
- After moving the port to the first argument: both persistence suites
with the service on port 9000 passed.

Without the suppressions, the persistence suites fail exactly the tests
listed above (4 in v2, 19 in v3).

After the review changes (the store projects, the port default, the
readiness wait), both persistence suites pass again with the
suppressions (v3: 141 ran; v2: 111 ran), and
`LaunchDarkly.ServerSdk.sln` builds with no errors.

#392 makes the same change on `dotnet-server-sdk-v9`.
<!-- TERMAG_PR_BODY_END -->

<!-- termag: box=i-0575d1c54853058d7 project=Overrides -->
<sub>Opened from termag</sub>

[SDK-3493](https://launchdarkly.atlassian.net/browse/SDK-3493)
<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Overview**
> Enables **persistent store contract tests** in CI by wiring the server
SDK contract test service to real Redis, DynamoDB, and Consul, and
turning those suites on in GitHub Actions.
> 
> The contract test service now builds Redis, DynamoDB, and Consul
integrations from in-repo sources (via `Stores/*` wrapper projects)
against the local SDK, implements `CreatePersistentDataStore` for
harness FDv1 `persistentDataStore` and FDv2 `dataSystem.store`, supports
**daemon mode** when a store is configured without streaming/polling,
advertises the new store capabilities, and accepts an **optional listen
port** (9000 in CI so DynamoDB can use 8000). Store clients use short
timeouts for recovery tests.
> 
> The **contract-tests** composite action gains
`enable_persistence_tests` and `test_service_port`, starts stores and
waits for readiness when enabled, and forwards those flags to both
harness runs. **sdk-server-ci** runs persistence tests on Linux only;
**nightly** always enables them with port 9000.
> 
> Known failures from **pre-existing tombstone / empty-collection bugs**
in the store integrations are listed in `test-supressions*.txt` so CI
stays green until separate fixes land.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
fd9c0d6. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

[SDK-3493]:
https://launchdarkly.atlassian.net/browse/SDK-3493?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
kinyoklion added a commit to launchdarkly/sdk-test-harness that referenced this pull request Nov 6, 2026
<!-- TERMAG_PR_BODY_BEGIN -->
## Summary

The data system spec requires each data source to state whether its data
is persisted (requirements 1.1.7 and 1.2.8 of
`DATASYSTEM-data-system/v2`). Its example is the file initializer, which
does not persist: file data must not replace the LaunchDarkly data in a
store that other SDK instances read. The persistence suites had no test
of this. This change adds tests for the file data source and for the
SDK's test data source, which is not persisted by default either.

## Service spec

Three new capabilities, all optional:

- `file-data-source`: a `file` object in a `dataSystem.initializers` or
`dataSystem.synchronizers` entry, with `filePaths`, configures the SDK's
file data source. The harness writes the files on the shared filesystem,
as for override files, and changes a file by rewriting it in place,
which is not atomic: the data source must not serve a partly written
file, and must end up with the complete content. As a synchronizer, the
data source reloads the files within ten seconds of a change.
- `test-data-source`: a `testData` object in one
`dataSystem.synchronizers` entry, with `flags` mapping each flag key to
a value, configures the SDK's test data source; each flag serves its
value to every context. The new `updateTestData` command changes one
flag. An SDK whose test data source is persisted by default must not
declare this capability.
- `data-source-persist-opt-in`: `persist: true` on a `file` or
`testData` object requests persistence.

## Tests

New tests under `persistent data store/<store>/data source persistence`,
so they need a store capability and `-enable-persistence-tests`, like
the rest of the persistence suite:

- `file data` (`file-data-source`):
- An initializer, and a synchronizer including a reload, leave the store
unchanged: both an empty store and one seeded with LaunchDarkly data.
- File data from a synchronizer does not replace the LaunchDarkly data
that the SDK itself wrote from a polling initializer.
- A LaunchDarkly basis after a file initializer is written to the store.
The streaming synchronizer holds its response until the test has checked
that the store is still empty, because the basis would otherwise replace
file data that the SDK wrongly wrote and hide the write.
- Store recovery (`persistent-data-store-recovery`): with file data in
memory last, the SDK does not write it back to the store after an outage
(TXNS requirement 1.4.3.2). An SDK detects an outage only when a store
operation fails, and it never writes file data, so the test holds a
LaunchDarkly polling initializer's response until the store is
unreachable; the SDK's write of that basis fails, and the file
synchronizer's data follows.
- With persistence requested (`data-source-persist-opt-in`): an
initializer's data and a synchronizer's reload are written, and a
read-only store is still not written.
- `test data` (`test-data-source`):
- The initial data and an update leave the store unchanged, empty or
seeded.
- With persistence requested: the initial data and an update are
written.

`RedisPersistentStore.Get` now reports a missing key as no value without
an error, as the Consul and DynamoDB stores do. Every existing caller
already ignored the error.

## Verification

- Against the .NET server SDK on `dotnet-server-sdk-v9`, with the
persistence fix (launchdarkly/dotnet-core#390), persistent store support
in its test service (launchdarkly/dotnet-core#392), and the new
capabilities (launchdarkly/dotnet-core#393): all the new tests pass for
Redis, DynamoDB, and Consul.
- Each of these defects, put into the .NET SDK one at a time, fails the
tests that target it, for every store:
- The write-through store's persistence check removed: the tests of data
that must not be persisted.
- Upserts always persisted, as Java's FDv1 adapter does: the test data
update test.
- The recovery write back done whatever the persistence indicator: the
store recovery test.
- Persisting everything once anything was persisted: the
file-after-LaunchDarkly test.
- The store mode ignored for data that requests persistence: the
read-only store test.
- `make lint` reports no issues, and the data source tests pass three
times in a row against the .NET service with the in-place rewrites.

No other SDK declares the new capabilities yet. The Go, Python, and Ruby
SDKs write synchronizer data to the store whatever its persist flag, so
their file synchronizer and test data tests would fail. The Node SDK
does not mirror conditionally yet (`TransactionalFeatureStore.ts`,
`TODO: SDK-1047`), so its file initializer test would fail too. Java's
test data sources are persisted by default, so it cannot declare
`test-data-source`, though its file data sources could declare
`file-data-source`.
<!-- TERMAG_PR_BODY_END -->

<!-- termag: box=i-0575d1c54853058d7 project=sdk-test-harness -->
<sub>Opened from termag</sub>

[SDK-3493](https://launchdarkly.atlassian.net/browse/SDK-3493)
<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Overview**
> Adds **contract coverage and service-spec support** for which data
sources write to a shared persistent store in read-write mode, matching
the data-system persist opt-in rules.
> 
> **Service spec** documents three optional capabilities
(`file-data-source`, `test-data-source`, `data-source-persist-opt-in`),
`file` / `testData` entries on `dataSystem` initializers and
synchronizers (with optional `persist`), the `updateTestData` command,
and clarifies that **store recovery write-back** and **read-write
`storeMode`** apply only to sources that request persistence.
> 
> **Harness** wires a new **`data source persistence`** sub-suite under
server-side persistent-store tests: file initializer/synchronizer
(including reloads and outage recovery), test-data updates, coexistence
with LaunchDarkly-written store data, and opt-in persistence plus
read-only store behavior. Helpers add shared **`DataSourceFile`**
fixtures (non-atomic in-place rewrites) and SDK config/command plumbing.
**`RedisPersistentStore.Get`** now treats a missing key as absent
without error, consistent with other store backends.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
8775a41. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

[SDK-3493]:
https://launchdarkly.atlassian.net/browse/SDK-3493?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
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