-
Notifications
You must be signed in to change notification settings - Fork 11
ci: add nightly contract tests with long-running tests and Slack notification #136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
3365a34
ci: add nightly contract tests with long-running tests and Slack noti…
devin-ai-integration[bot] 7e2ded7
tuning workflow and adding some test exceptions for the moment
tanderson-ld af6eac9
tweaking test service to support polling
tanderson-ld 4b0ac23
Update .github/workflows/nightly-contract-tests.yml
tanderson-ld efe6245
ci: pin slackapi/slack-github-action to v2.1.1 SHA
devin-ai-integration[bot] 48839e2
Merge branch 'main' into devin/1770832864-nightly-contract-tests
tanderson-ld 69b9f64
Merge branch 'main' into devin/1770832864-nightly-contract-tests
tanderson-ld File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| 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 }}" |
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
| 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 }}" | ||
| } | ||
| } | ||
| ] | ||
| } |
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
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
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
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
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
| 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 |
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.
There was a problem hiding this comment.
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 theserviceEndpointsblock, so it silently overrides anyparams.serviceEndpoints.pollingvalue. This is the opposite of the streaming pattern, whereendpoints.streaming(params.streaming.baseUri)is set inside theif (params.streaming != null)block (line 397), allowingserviceEndpoints.streamingto take precedence later. Additionally, this endpoint override lacks theparams.dataSystem == nullguard that protects the data source creation at line 404, meaning it can set the global polling endpoint even when adataSystemis configured. Moving this call inside theelse ifblock would fix both issues and match the streaming pattern.Additional Locations (1)
lib/sdk/server/contract-tests/service/src/main/java/sdktest/SdkClientEntity.java#L403-L414