Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions .github/actions/contract-tests/action.yml
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
name: Contract Tests
description: Runs Contract Tests
description: Runs Contract Tests (builds, starts service, runs SDK contract test harness)
inputs:
workspace_path:
description: 'Path to the package.'
description: 'Path to the package (e.g. lib/sdk/server).'
required: true
token:
description: 'Github token, used for contract tests'
required: false
default: ''
test_harness_params:
description: 'Optional extra parameters for the SDK test harness (e.g. -enable-long-running-tests). Passed as TEST_HARNESS_PARAMS to make.'
required: false
default: ''

runs:
using: composite
steps:
- name: Run contract tests
shell: bash
run: make contract-tests -C ${{ inputs.workspace_path }}
run: make contract-tests -C ${{ inputs.workspace_path }} TEST_HARNESS_PARAMS="${{ inputs.test_harness_params }}"
99 changes: 99 additions & 0 deletions .github/workflows/nightly-contract-tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
name: Nightly Contract Tests

on:
schedule:
- cron: "0 4 * * *" # every day at 4am UTC
workflow_dispatch:
inputs:
branch:
description: 'Branch to run contract tests on'
required: false
default: ''
test_slack_notification:
description: 'Also send a test Slack notification (to verify Slack integration)'
required: false
type: boolean
default: false

jobs:
nightly-contract-tests:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
with:
ref: ${{ inputs.branch || github.ref }}

- name: Shared CI Steps
uses: ./.github/actions/ci
with:
workspace_path: 'lib/sdk/server'
java_version: 8

- name: Contract Tests (with long-running tests)
uses: ./.github/actions/contract-tests
with:
workspace_path: 'lib/sdk/server'
token: ${{ secrets.GITHUB_TOKEN }}
test_harness_params: '-enable-long-running-tests'

notify-slack-on-failure:
runs-on: ubuntu-latest
if: ${{ always() && needs.nightly-contract-tests.result == 'failure' }}
needs:
- nightly-contract-tests
steps:
- name: Send Slack notification
uses: slackapi/slack-github-action@91efab103c0de0a537f72a35f6b8cda0ee76bf0a # v2.1.1
with:
webhook: ${{ secrets.SLACK_WEBHOOK_URL }}
webhook-type: incoming-webhook
payload: |
{
"blocks": [
{
"type": "section",
"text": {
"type": "mrkdwn",
"text": ":warning: *Nightly Contract Tests Failed* :warning:\nThe nightly contract tests (with long-running tests enabled) failed on `${{ github.ref_name }}`."
},
"accessory": {
"type": "button",
"text": {
"type": "plain_text",
"text": "View failed run"
},
"url": "https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}"
}
}
]
}

test-slack-notification:
runs-on: ubuntu-latest
if: ${{ inputs.test_slack_notification == true || inputs.test_slack_notification == 'true' }}
steps:
- name: Send test Slack notification
uses: slackapi/slack-github-action@91efab103c0de0a537f72a35f6b8cda0ee76bf0a # v2.1.1
with:
webhook: ${{ secrets.SLACK_WEBHOOK_URL }}
webhook-type: incoming-webhook
payload: |
{
"blocks": [
{
"type": "section",
"text": {
"type": "mrkdwn",
"text": ":white_check_mark: *Nightly Contract Tests – Slack test*\nThis is a test notification to verify Slack integration is working. Triggered manually from the Nightly Contract Tests workflow."
},
"accessory": {
"type": "button",
"text": {
"type": "plain_text",
"text": "View workflow run"
},
"url": "https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}"
}
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ public static class SdkConfigParams {
Long startWaitTimeMs;
boolean initCanFail;
SdkConfigStreamParams streaming;
SdkConfigPollingParams polling;
SdkConfigEventParams events;
SdkConfigBigSegmentsParams bigSegments;
SdkConfigTagParams tags;
Expand Down Expand Up @@ -170,6 +171,7 @@ public static class SdkConfigSynchronizerParams {
public static class SdkConfigPollingParams {
URI baseUri;
Long pollIntervalMs;
String filter;
}

public static class SdkConfigStreamingParams {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,16 @@ private LDConfig buildSdkConfig(SdkConfigParams params, String tag) {
}
dataSource.payloadFilter(params.streaming.filter);
builder.dataSource(dataSource);
} else if (params.polling != null && params.dataSystem == null) {
// v2 harness: top-level polling only (no dataSystem); use FDv1 polling data source
PollingDataSourceBuilder pollingDataSource = Components.pollingDataSource();
if (params.polling.pollIntervalMs != null) {
pollingDataSource.pollInterval(Duration.ofMillis(params.polling.pollIntervalMs));
}
if (params.polling.filter != null && !params.polling.filter.isEmpty()) {
pollingDataSource.payloadFilter(params.polling.filter);
}
builder.dataSource(pollingDataSource);
}

if (params.events == null) {
Expand Down Expand Up @@ -463,6 +473,11 @@ private LDConfig buildSdkConfig(SdkConfigParams params, String tag) {
endpoints.events(params.serviceEndpoints.events);
}
}

if (params.polling != null && params.polling.baseUri != null && !params.polling.baseUri.toString().trim().isEmpty()) {
endpoints.polling(params.polling.baseUri);
}
Copy link

Choose a reason for hiding this comment

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

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.

Additional Locations (1)

Fix in Cursor Fix in Web


builder.serviceEndpoints(endpoints);

if (params.hooks != null && params.hooks.hooks != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public class TestService {
"service-endpoints",
"strongly-typed",
"tags",
"server-side-polling"
};

static final Gson gson = new GsonBuilder().serializeNulls().create();
Expand Down
3 changes: 3 additions & 0 deletions lib/sdk/server/contract-tests/test-suppressions-fdv2.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
streaming/fdv2/recoverable fallback to secondary synchronizer
streaming/fdv2/recoverable fallback with recovery
streaming/fdv2/permanent fallback with recovery