Skip to content

feat(testservice): support file-based flag overrides - #416

Open
kinyoklion wants to merge 4 commits into
rlamb/sdk-2656/ldoverrides-file-sourcefrom
rlamb/sdk-2657/testservice-flag-overrides
Open

kinyoklion wants to merge 4 commits into
rlamb/sdk-2656/ldoverrides-file-sourcefrom
rlamb/sdk-2657/testservice-flag-overrides

Conversation

@kinyoklion

@kinyoklion kinyoklion commented Jul 8, 2026

Copy link
Copy Markdown
Member

Reopens #413 (auto-marked merged for the same base-branch reason as #412; see #415). Rebuilt as a cherry-pick stacked on #415 so the diff shows only the test service changes. Review discussion lives on #413.

Wires the contract-test service up to the flag-overrides feature so the harness suite (launchdarkly/sdk-test-harness#374) can run against this SDK: advertises the flag-overrides capability, mirrors the harness's overrides config block in the test service's servicedef, and maps it onto ldoverrides.FileSource() on the data system builder; requesting overrides without a data system configuration is an error, since overrides are an FDv2 feature. The test service module pins the same override-aware pseudo-versions of go-sdk-common, go-sdk-events, and go-server-sdk-evaluation as the SDK.

Verified locally against the harness branch with the updated override tests (which expect the overrideAffected reason indicator and summary counter marker): the flag-overrides suite passes (32 tests ran, 0 failures, across static configuration, uninitialized client, summary events, transitive marking, multiple files, YAML, and watching and polling hot reload), and the full harness run is green (4961 tests total, 4949 ran, 12 skipped, 0 failures).

SDK-2657


Note

Overview
Refactors file-based flag overrides to use a single ChangeDetection mode (Polling vs Watching) instead of independent Watch / Poll toggles, and defaults to polling (1s) rather than filesystem watches. Build now rejects unknown detection modes; PollInterval applies only in polling mode.

The contract test service advertises flag-overrides, accepts an overrides config block (paths, duplicate-key handling, change detection, poll interval), wires it to ldoverrides.FileSource() on the FDv2 data system, and errors if overrides are requested without a data system. The testservice module bumps Go and pins override-related dependencies.

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

@kinyoklion
kinyoklion force-pushed the rlamb/sdk-2657/testservice-flag-overrides branch from a8e7b3f to d908177 Compare September 15, 2026 21:46
@kinyoklion
kinyoklion force-pushed the rlamb/sdk-2656/ldoverrides-file-source branch from f5ddaf5 to 669cd50 Compare September 15, 2026 21:46
@kinyoklion
kinyoklion changed the base branch from rlamb/sdk-2656/ldoverrides-file-source to feat/overrides September 17, 2026 21:15
@kinyoklion
kinyoklion force-pushed the rlamb/sdk-2657/testservice-flag-overrides branch 3 times, most recently from abeb245 to 7657c3e Compare September 17, 2026 22:00
@kinyoklion
kinyoklion changed the base branch from feat/overrides to rlamb/sdk-2656/ldoverrides-file-source September 17, 2026 22:17
@kinyoklion
kinyoklion force-pushed the rlamb/sdk-2657/testservice-flag-overrides branch from 7657c3e to 3b324a7 Compare September 18, 2026 15:56
@kinyoklion
kinyoklion force-pushed the rlamb/sdk-2656/ldoverrides-file-source branch from e53b4d2 to 2f0e71b Compare September 18, 2026 15:56
@kinyoklion
kinyoklion force-pushed the rlamb/sdk-2657/testservice-flag-overrides branch from 3b324a7 to 079665a Compare September 18, 2026 16:38
@kinyoklion
kinyoklion force-pushed the rlamb/sdk-2656/ldoverrides-file-source branch from 2f0e71b to 5e46a28 Compare September 18, 2026 16:38
@kinyoklion
kinyoklion force-pushed the rlamb/sdk-2657/testservice-flag-overrides branch from 079665a to 04cb0c1 Compare September 21, 2026 16:26
@kinyoklion
kinyoklion force-pushed the rlamb/sdk-2656/ldoverrides-file-source branch from 5e46a28 to 393b4bc Compare September 21, 2026 16:26
@kinyoklion
kinyoklion force-pushed the rlamb/sdk-2657/testservice-flag-overrides branch from 04cb0c1 to f0853ba Compare September 21, 2026 16:51
@kinyoklion
kinyoklion force-pushed the rlamb/sdk-2656/ldoverrides-file-source branch from 393b4bc to 595f835 Compare September 21, 2026 16:51
@kinyoklion
kinyoklion force-pushed the rlamb/sdk-2657/testservice-flag-overrides branch from f0853ba to 5931e29 Compare September 21, 2026 17:01
@kinyoklion
kinyoklion force-pushed the rlamb/sdk-2656/ldoverrides-file-source branch from 595f835 to d99d55c Compare September 21, 2026 17:01
@kinyoklion
kinyoklion force-pushed the rlamb/sdk-2657/testservice-flag-overrides branch from 5931e29 to 42cfae2 Compare September 21, 2026 17:03
@kinyoklion
kinyoklion force-pushed the rlamb/sdk-2656/ldoverrides-file-source branch from d99d55c to 3af8e81 Compare September 21, 2026 17:03
Watching and polling were two independent switches, which allowed both on and both off. They are alternatives, so the builder now takes one ChangeDetection mode. Polling is the default because it does not depend on notification delivery. Build rejects an unrecognized mode.
@kinyoklion
kinyoklion force-pushed the rlamb/sdk-2657/testservice-flag-overrides branch from 42cfae2 to 393312c Compare September 21, 2026 21:31
@kinyoklion
kinyoklion marked this pull request as ready for review September 22, 2026 20:37
@kinyoklion
kinyoklion requested a review from a team as a code owner September 22, 2026 20:37

@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.

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

Fix All in Cursor

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

Reviewed by Cursor Bugbot for commit 393312c. Configure here.

FilePaths []string `json:"filePaths"`
DuplicateKeysHandling *string `json:"duplicateKeysHandling,omitempty"`
ChangeDetection *string `json:"changeDetection,omitempty"`
PollIntervalMS *int `json:"pollIntervalMs,omitempty"`

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Watch/poll harness fields ignored

Medium Severity

The test service overrides block reads changeDetection, but the harness contract cited here (sdk-test-harness#374) sends watch and poll booleans. Those fields are dropped, so every client stays on the Polling default. Watching-mode harness tests still pass because polling also reloads on file changes, so the suite never actually runs Watching.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 393312c. Configure here.

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