Repository navigation
feat: Support the file and test data source capabilities in the contract test service - #393
Open
kinyoklion wants to merge 2 commits into
Open
kinyoklion wants to merge 2 commits into
kinyoklion wants to merge 2 commits into
Conversation
Member
Author
|
bugbot review |
Member
Author
|
bugbot review |
There was a problem hiding this comment.
✅ 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 e8c9f4a. Configure here.
kinyoklion
marked this pull request as ready for review
November 3, 2026 23:26
tanderson-ld
approved these changes
Nov 4, 2026
…ract test service From the review of this change: - testData is a synchronizer only, and a configuration may have one: the updateTestData command addresses the client's test data source, and a test data initializer is disposed when the initializer phase ends. - A file data source needs at least one file path. With none it would load an empty data set, which hides LaunchDarkly data in memory or, when persisted, empties the store. - updateTestData without a flag key is rejected.
kinyoklion
force-pushed
the
rlamb/contract-tests-data-source-persistence-v9
branch
from
November 4, 2026 22:42
e8c9f4a to
165061a
Compare
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
This branch has not been deployed
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
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
The test harness gains tests that a file data source and a test data source do not write their data to a persistent store, and that they do when they request it (launchdarkly/sdk-test-harness#537). This change lets the server SDK's contract test service run them, on
dotnet-server-sdk-v9, whereTestDatais not persisted by default (#390).fileentry indataSystem.initializersordataSystem.synchronizersconfiguresFileData.DataSource()with the given file paths. As a synchronizer it reloads the files when they change (AutoUpdate(true)).testDatasynchronizer configures aTestDatasource whose flags each serve one value to every context (ValueForAll), and theupdateTestDatacommand changes one of its flags. A configuration may have onetestDatasynchronizer, and notestDatainitializer, as the service spec says.fileentry needs at least one file path, andupdateTestDataneeds a flag key; the service rejects the configuration or the command otherwise.persist: trueon either entry callsShouldPersist(true).file-data-source,test-data-source, anddata-source-persist-opt-in.BuildSdkConfigandCreateSynchronizerbecome instance methods, so that the client entity keeps its test data source for the command.Dependencies
ShouldPersisttoFileDataSourceBuilderandTestData.mainversion. Onmain,TestDatais persisted by default, so the test data tests, which expect it not to be, would fail there.Testing
Against a local merge of this branch and #392, with Redis, DynamoDB local, and Consul in Docker, the harness branch's
data source persistencetests pass for all three stores.To check that the tests catch the defects they target, defects were put into the SDK one at a time, and each failed the tests that target it for every store: the write-through store's persistence check removed; upserts always persisted, as Java's FDv1 adapter does; the recovery write back done whatever the persistence indicator; persisting everything once anything was persisted; and the store mode ignored for data that requests persistence.
Opened from termag
SDK-3493
Note
Overview
The .NET server contract test service now implements harness scenarios for file and test data sources, including optional persistence to the data store.
Harness JSON can configure a
fileinitializer or synchronizer (paths plus optionalpersist); synchronizers use auto-reload on file changes. AtestDatasynchronizer seeds flags withValueForAll, and the newupdateTestDatacommand updates a flag on the single registeredTestDatainstance. Invalid configs (empty file paths, duplicate testData, missing test data on update) fail fast.BuildSdkConfig/CreateSynchronizerare instance methods so each client keeps itsTestDatareference for commands. The status endpoint advertisesfile-data-source,test-data-source, anddata-source-persist-opt-in.Reviewed by Cursor Bugbot for commit 165061a. Bugbot is set up for automated code reviews on this repo. Configure here.