ci: add nightly contract tests with long-running tests and Slack notification#136
Conversation
…fication Co-Authored-By: tanderson@launchdarkly.com <tanderson@launchdarkly.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Co-authored-by: semgrep-code-launchdarkly[bot] <167133144+semgrep-code-launchdarkly[bot]@users.noreply.github.com>
There was a problem hiding this comment.
For reviewers: These tests are not passing because they are written against the Go impl. I need to update the FDv2 Data System spec to specify the edge case behavior and timings to either match Go or update Go and then update the Java, Dotnet, and Node impls to match that.
Co-Authored-By: tanderson@launchdarkly.com <tanderson@launchdarkly.com>
|
|
||
| if (params.polling != null && params.polling.baseUri != null && !params.polling.baseUri.toString().trim().isEmpty()) { | ||
| endpoints.polling(params.polling.baseUri); | ||
| } |
There was a problem hiding this comment.
Polling endpoint override placed after serviceEndpoints, reversing precedence
Medium Severity
The endpoints.polling(params.polling.baseUri) call at lines 477–479 is placed after the serviceEndpoints block, so it silently overrides any params.serviceEndpoints.polling value. This is the opposite of the streaming pattern, where endpoints.streaming(params.streaming.baseUri) is set inside the if (params.streaming != null) block (line 397), allowing serviceEndpoints.streaming to take precedence later. Additionally, this endpoint override lacks the params.dataSystem == null guard that protects the data source creation at line 404, meaning it can set the global polling endpoint even when a dataSystem is configured. Moving this call inside the else if block would fix both issues and match the streaming pattern.


Requirements
Related issues
N/A
Describe the solution you've provided
Adds a new GitHub Actions workflow that runs the Java server SDK contract tests nightly (4am UTC) with the
-enable-long-running-testsflag enabled. If the tests fail, a Slack notification is sent to alert the team.Key details:
workflow_dispatchfor manual triggering (supports custom branch and Slack test notification inputs)ciandcontract-testscomposite actions; thecontract-testsaction now accepts atest_harness_paramsinput to pass-enable-long-running-testsslackapi/slack-github-action(pinned to v2.1.1 SHA) with an incoming webhook (SLACK_WEBHOOK_URLsecret) to post to Slack on failure, including a link to the failed runtest-slack-notificationjob can be triggered manually viaworkflow_dispatchto verify the Slack integration is workingserver-side-pollingcapability and polling parameter support to the test service entitystreaming/fdv2fallback/recovery tests are suppressed pending spec alignment between Go and Java implementationsItems for reviewer attention:
SLACK_WEBHOOK_URLsecret — must be configured in the repo settings with a webhook routed to#sdks-java. The channel is determined by the webhook configuration, not the workflow.timeout-minutes— long-running tests could potentially hang indefinitely. Consider adding a timeout.failureonly, notcancelledortimed_out. Decide if those states should also notify.SdkClientEntity.javais correct, particularly the conditional logic forparams.polling != null && params.dataSystem == nulland the base URI endpoint wiring.Describe alternatives you've considered
rtCamp/action-slack-notify(used byldcli) instead ofslackapi/slack-github-action, but the official Slack action is more widely used across LD repos (terraform-provider-launchdarkly,streamer,ld-docs-private).Updates since initial revision
contract-testscomposite action withtest_harness_paramsinput instead of splitting out individual make targets (addresses original item ci: adding release please support #4)workflow_dispatchinputs for branch selection and Slack notification testingtest-slack-notificationjob for verifying Slack integrationserver-side-pollingcapability and polling params to the contract test serviceslackapi/slack-github-actionto v2.1.1 SHA (91efab103c0de0a537f72a35f6b8cda0ee76bf0a)Additional context
Link to Devin run
Requested by: tanderson@launchdarkly.com
Note
Medium Risk
Primarily CI/workflow changes, but it also adjusts contract-test service configuration logic for polling which could alter how tests initialize and exercise the SDK.
Overview
Adds a new
Nightly Contract TestsGitHub Actions workflow that runs server SDK contract tests nightly (and on manual dispatch), with an optional branch override and an optional Slack test-notification path.Extends the
contract-testscomposite action to accepttest_harness_paramsand pass them through tomake, enabling the nightly workflow to run the harness with-enable-long-running-tests, and posts a Slack alert via an incoming webhook when the nightly job fails.Updates the contract test service to advertise
server-side-polling, accept polling config (including afilter), and wire polling endpoints/data source when streaming isn’t used; also adds FDv2 suppressions for several streaming fallback/recovery tests.Written by Cursor Bugbot for commit 69b9f64. This will update automatically on new commits. Configure here.