chore: port long-running test framework support from v3 - #428
Merged
Conversation
Cherry-picks the framework scaffolding portion of #314 (Jason Bailey) from v3 to v2. Adds: - `-enable-long-running-tests` CLI flag (default false), mirroring the `-enable-persistence-tests` shape. - `EnableLongRunningTests bool` on `ldtest.TestConfiguration`. - `(t *T) LongRunning()` method — long-running tests call this at the top of their body; if the flag isn't set, the test skips with reason "use -enable-long-running-tests to run". - Threaded through `RunSDKTestSuite` and `main.go`. Deliberately excluded from the port: the v3-specific FDv2 stream tests that came in the same original commit (`sdktests/common_tests_stream_fdv2.go`). Those exercise v3-only shapes. No behavior change to existing v2 tests -- none of them currently call `t.LongRunning()`. This is scaffolding for follow-up work: the RETRY- conformance tests in #404 will migrate their timing-compression-via-servicedef approach to real timing gated by this flag, per the review feedback that argued against exposing SDK-side test-harness bypasses.
keelerm84
approved these changes
Aug 18, 2026
4 tasks
tanderson-ld
added a commit
that referenced
this pull request
Aug 19, 2026
…ling (#404) ## Summary Adds contract tests exercising the RETRY specification's non-permanent-stop guarantee for the FDv1 streaming and polling data sources. Introduces two capability gates so SDKs can adopt independently: - `retry-conformance-fdv1-streaming` - `retry-conformance-fdv1-polling` The new tests exercise the SDK's real production 5-minute extended-regime timing — no test-harness bypass, no compression knobs on the servicedef, no test-only surface in the SDK's public API. They are marked as long-running (`t.LongRunning()`) and only run when the harness is invoked with `-enable-long-running-tests`. Legacy "do not retry after unrecoverable HTTP error" scenarios remain, gated behind the ABSENCE of the streaming capability so SDKs can migrate on their own schedule. Once every SDK reports the new capabilities, both the legacy tests and the gates can be removed. ## What's added **Streaming (guarded by `retry-conformance-fdv1-streaming` + `-enable-long-running-tests`):** - retry after unexpected HTTP error on initial connect (401/403/405) - retry after unexpected HTTP error on reconnect (401/403/405) - enters extended-regime backoff after unexpected HTTP error (paired-bound timing assertion) - does not permanently stop under sustained unexpected HTTP errors - returns to normal-regime backoff after healthy operation (streaming spec's activeSince reset: 75 s silent stream after an extended-regime reconnect, then next reconnect must be at normal-regime timing) **Polling (guarded by `retry-conformance-fdv1-polling` + `-enable-long-running-tests`):** - retry after unexpected HTTP error on initial poll (401/403/405) - returns to normal-regime cadence after two consecutive successful polls (RETRY §1.8.1 count-based reset) ## Design: long-running instead of compressed timings An earlier revision of this PR exposed extended-regime timing knobs on the servicedef (`ExtendedInitialDelayMS`, `ResetThresholdMS`) so tests could compress the 5-minute extended-initial delay into a ~30-second observable window. Reviewers pushed back because that shape forced SDKs to expose corresponding test-only knobs on their public API surface just to be testable by the harness. This revision drops all compression knobs and exercises real production timing instead. That makes each retry-conformance subtest take several minutes of wall clock, which is why the tests are marked `t.LongRunning()` and gated behind `-enable-long-running-tests`. Per-SDK CI schedules for the long-running suite are out of scope for this PR (planned follow-up work). Framework support for `t.LongRunning()` and the `-enable-long-running-tests` flag was ported from v3 in #428, a prerequisite that landed before this PR's refactor. ## Relationship to PR #401 Supersedes #401, which targeted `v3` and exercised FDv2 code paths — out of scope for the initial RETRY-conformance work, which is FDv1-only. This PR targets `v2` and drives the FDv1 streaming/polling paths in the SDK testservices. #401 will be closed with a pointer here. SDK-2808 ## Test plan - [x] Contract tests pass against go-server-sdk PR #429 (the reference server SDK implementation) at real production 5-minute extended-regime timing (~3-hour run with `-enable-long-running-tests`) - [x] Legacy "do not retry" tests still pass for SDKs without the new capability (verified by SDK capability check) - [x] Retry-conformance tests are correctly SKIPPED for SDKs without the capability - [x] Retry-conformance tests are correctly SKIPPED when `-enable-long-running-tests` is not set
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
2 participants
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
Ports the framework scaffolding portion of launchdarkly/sdk-test-harness#314 (Jason Bailey, "fix: Add support for optional long running tests") from v3 to v2.
What ships
-enable-long-running-testsCLI flag (default false). Mirrors the-enable-persistence-testsshape.EnableLongRunningTests boolonldtest.TestConfiguration.(t *T) LongRunning()method. Long-running tests call it at the top of their body; if the flag isn't set, the test skips with reason"use -enable-long-running-tests to run".RunSDKTestSuiteandmain.go.What's deliberately not ported
The original commit (
01a245c) also added 237 lines tosdktests/common_tests_stream_fdv2.go. That file exercises v3-only FDv2 shapes and has no place on v2. Skipped.Why now
Scaffolding for follow-up work on #404. Review feedback there (discussion, discussion) argued against exposing an SDK-side servicedef knob to compress the extended-regime timing in the RETRY-conformance tests. The consensus was that those tests should run at real timing gated by the long-running-tests mechanism — which requires that mechanism to exist on v2 first.
Once this lands, #404 rebases onto v2 tip and its RETRY-conformance tests get
t.LongRunning()calls at the top of their bodies, plus timing rewritten to production defaults.Verification
go build ./...— clean.go run . -h— new flag visible:-enable-long-running-tests enable tests that take a long time to run (10+ seconds).go test ./...— full suite green. No behavior change to existing v2 tests since none of them callt.LongRunning().Test plan
Note
Overview
Adds optional long-running test gating on v2, mirroring the existing
-enable-persistence-testspattern.The harness accepts
-enable-long-running-tests(default false). That value flows throughmain→RunSDKTestSuite→ldtest.TestConfiguration.EnableLongRunningTests. Tests can callt.LongRunning()at the start; when the flag is off they skip with reasonuse -enable-long-running-tests to run.No existing v2 tests call
LongRunning()yet, so default runs behave the same until follow-up tests adopt the helper.Reviewed by Cursor Bugbot for commit 536af50. Bugbot is set up for automated code reviews on this repo. Configure here.