Repository navigation
ci: add nightly contract tests with long-running tests enabled - #454
Merged
Merged
Conversation
Runs the full contract suite nightly with -enable-long-running-tests, which per-PR CI omits because these tests wait on real RETRY backoff timing and take roughly an hour. This is the Go counterpart to the java-core nightly. Both harness steps float their major version, matching common_ci.yml. Persistence tests are off, so the persistent-stores step is omitted. The Go version comes from the same shared sdk-go-version-matrix workflow ci.yml uses, so the nightly cannot drift from per-PR CI. Adds two intentionally-empty suppression files so that adding a suppression is a one-line change rather than requiring the -skip-from plumbing to be worked out first. They must exist even when empty: the harness hard-errors on a missing -skip-from path. The job is expected to fail on exactly one test until a follow-up PR fixes the underlying bug: streaming/fdv2/permanent fallback with recovery
tanderson-ld
commented
Sep 18, 2026
Contributor
Author
There was a problem hiding this comment.
Intentionally empty
tanderson-ld
commented
Sep 18, 2026
Contributor
Author
There was a problem hiding this comment.
Intentionally empty
Drops the header comment block; the background it carried lives in the pull request description instead, where it stays accurate. Replaces the TEST_SERVICE_PORT job env var with the literal at both use sites. In this repo the variable could not relocate the service anyway: testservice/service.go hardcodes port 10000 and the Makefile passes nothing, so it only told the harness where to connect. The literal also matches the java-core nightly.
tanderson-ld
marked this pull request as ready for review
September 18, 2026 13:58
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort 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.
Reviewed by Cursor Bugbot for commit 2aab427. Configure here.
joker23
approved these changes
Sep 21, 2026
2 of 3 tasks
tanderson-ld
added a commit
that referenced
this pull request
Oct 12, 2026
**Requirements** - [x] I have followed the repository's [pull request submission guidelines](../blob/v7/CONTRIBUTING.md#submitting-pull-requests) - [x] I have added test coverage for new or changed functionality - [ ] I have validated my changes against all supported platform versions — n/a, no platform-specific behavior **Related issues** SDK-3235. Also fixes the one known failure in the nightly contract tests added in #454: `streaming/fdv2/permanent fallback with recovery`. **Describe the solution you've provided** The data system has two fallback timers, selected by reading the status: a **10-second** one for startup (`cannotInitialize`) and a **1-minute** one for steady state (`interruptedAtRuntime`). A synchronizer that fails permanently during startup reports `Off`, which the loop publishes as `Interrupted`. Nothing restores `Initializing`, so the startup branch — which tested the status value — was unreachable for the rest of startup, leaving only the 1-minute timer. That only becomes visible when the *next* synchronizer connects and then stays silent: it produces no result for the loop to act on, so the loop depends entirely on the timer, and the client's start-wait expires first. The healthy source is never contacted. ```diff -cannotInitialize := status.State == interfaces.DataSourceStateInitializing && +cannotInitialize := !fdv2.dataApplied.Get() && time.Since(status.StateSince) > 10*time.Second ``` Keying on whether data has ever been applied expresses the actual predicate and cannot go stale as states are added. Runtime behavior is unchanged: once data is applied, only the 1-minute timer applies. Pre-existing, not a regression — it reproduces on 7.15.6, before the RETRY work. **Describe alternatives you've considered** - **Resetting the status to `Initializing` on removal.** Rejected on cross-SDK evidence: Java and .NET both report `INITIALIZING → INTERRUPTED` and neither re-publishes `Initializing` after a source fails. .NET makes that choice explicitly ("spoof interrupted"). This keeps Go's externally visible status sequence identical to today's and consistent with both siblings. - **Adding `Interrupted` to the startup branch's state check.** Rejected: `Interrupted && >10s` subsumes `Interrupted && >1min`, which would make the runtime timer dead code and abandon a source after 10 seconds instead of a minute — and recovery back to it takes 5 minutes of healthy operation. - **Only refreshing `StateSince` on the transition.** Insufficient: the 1-minute timer would restart and fire at ~70s, still past a 60s start-wait. **Additional context** The fallback and recovery conditions had **zero execution coverage** before this (the closure bodies measured 0 executions, while the file overall sat at 59.6%) — they are only evaluated on a 10-second tick with 2+ synchronizers configured, which nothing but the long-running contract test reached. The new in-package tests cover both the predicate and the multi-synchronizer loop path, and both fail when the fix is reverted. Verified: all 4 `streaming/fdv2` fallback contract tests pass against the patched SDK, including the recovery assertion that had never executed for Go; all 28 root-package FDv2 end-to-end tests pass; `golangci-lint` reports 0 issues. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > Fixes FDv2 multi-synchronizer **startup** so a synchronizer that connects but never delivers data can still be abandoned within the **10s** window, even after an earlier source fails permanently and the data source status stays **Interrupted** instead of **Initializing**. > > The startup branch of `fallbackCond` now keys off **`!fdv2.dataApplied`** rather than `DataSourceStateInitializing`, so the short timer stays active until the first payload is applied; steady-state behavior after data has been applied is unchanged (still **1 minute** on **Interrupted**). > > Adds in-package tests that exercise the predicate (including **Interrupted** before any data) and a multi-synchronizer loop that waits out the real 10s fallback twice past a silent synchronizer. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit fc2eca8. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
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.
Requirements
Related issues
Tracked in Jira. The issue key is omitted on purpose so that merging this does not auto-close the story before the follow-up fix PR.
Describe the solution you've provided
The test harness gates a set of RETRY-conformance and FDv2 tests behind
-enable-long-running-tests, because they wait on real backoff timing and take about an hour. Per-PR CI does not pass that flag, so none of them have ever run for this SDK. This adds a nightly job that does — 11 test groups newly covered. It is the Go counterpart to java-core's nightly.Warning
This job is expected to fail on exactly one test until a follow-up PR in this repo fixes it:
streaming/fdv2/permanent fallback with recovery. On permanent synchronizer removal the SDK never resets its data source status, so it does not fall back to a healthy synchronizer before the init timeout. Pre-existing — it reproduces on 7.15.6, so not a RETRY regression. Deliberately not suppressed: a nightly that goes red on a real defect is the point. More than one failure means something else broke — check the uploaded JUnit results.Choices worth noting:
v2,v3), matchingcommon_ci.yml, so nightly and per-PR CI resolve the same harness.latest, from the shared version-matrix workflow). RETRY timing is not Go-version-sensitive, and per-PR CI already runs the fast suite on all three.persistent-storesstep.timeout-minutes: 120. java-core's equivalent job measures 59–65 min.-skip-frompath.Describe alternatives you've considered
concurrencygroup — unnecessary, since each run gets its own ephemeral VM, and it would queue the dispatch runs used to validate this.Additional context
This cannot be validated before merge, because a new workflow file is not dispatchable until it is on the default branch. After merge:
Expect zero
use -enable-long-running-tests to runskips, and exactly one failure.Note
Overview
Adds a nightly GitHub Actions workflow (4:00 UTC cron plus manual dispatch) that runs the SDK contract harness the same way as per-PR CI—build/start the test service, then v2 and v3 harness runs—but turns on
-enable-long-running-testsso RETRY/backoff and related scenarios that take ~an hour actually execute.Compared to
common_ci.ymlcontract tests, the job uses latest Go only (via the shared version matrix), disables persistence tests (no persistent-stores setup), passes-skip-fromto empty suppression files undertestservice/, writes JUnit underjunit/, enables debug logging, and sets a 120-minute job timeout. It always uploads JUnit artifacts and test-service logs on completion.The workflow is expected to stay red on one known FDv2 failure until a follow-up fix lands; more than one failure would indicate a broader regression.
Reviewed by Cursor Bugbot for commit 2aab427. Bugbot is set up for automated code reviews on this repo. Configure here.