Repository navigation
ci: run the integration tests on every pull request - #90
Conversation
The live suite only ran on pushes to main and when a maintainer commented /integrationtest. Add an `integration` job to the Tests workflow so it runs on JDK 17 for every pull request that changes code, as the other SDKs do (SDK requirements 13). It skips, as a job, for documentation-only pull requests and for pull requests from dependabot or a fork, which cannot read MARKETDATA_TOKEN. When it runs and the secret is empty it fails (SDK requirements 17.3).
Tests passed is the check branch protection requires. It now needs the `integration` job too: success passes, and a skip passes only when the documentation gate asked for it or the pull request is from dependabot or a fork. Any other result fails.
Pull requests now run the live suite themselves, so the comments that said they do not were wrong. The on-demand workflow stays for the full JDK matrix, for fork and dependabot pull requests, and for re-runs without a push. Its "Integration tests pass" check only exists after a command is commented, so it must not be a required check. Comments only; no behaviour changes.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #90 +/- ##
=========================================
Coverage 98.27% 98.27%
Complexity 1043 1043
=========================================
Files 123 123
Lines 2606 2606
Branches 301 301
=========================================
Hits 2561 2561
Partials 45 45 Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
✅ Ready — Code reviewThis adds an automatic live integration-test job for eligible pull requests and makes the existing aggregate check enforce its result while retaining documented skip cases. Version impact: none; only CI configuration and documentation change, so no release or changelog entry is needed. Rating patch 5/6 (solid) · proof 5/6 (reproducible) · overall 5/6 Since last review
Reviewed |
`github.actor` is whoever triggered the run. A maintainer who reopens a dependabot pull request becomes the actor, so the `integration` job ran as on an ordinary pull request: with the real token, against a freshly bumped dependency, without anybody having decided to. Test `github.event.pull_request.user.login` instead, in the job's `if:` and in the negated expression `Tests passed` uses, so a dependabot pull request never runs the live suite on its own. A maintainer runs it with /integrationtest.
|
@marketdata-code-review waive a9b712d5 Why this finding is waived The finding is accurate about the job: it runs the pull request's code with
What does change, and we accept: code written by the SDK sync pipeline now runs with the What we did not choose: a protected environment with required approval would close the |
Run the integration tests on every pull request
The integration tests (
src/integrationTest/**, against the live API) ran on every push tomainand when a maintainer commented/integrationtest. They did not run on pull requests.The other SDKs run their live suite on every pull request (sdk-py, sdk-csharp, sdk-go, sdk-php),
as the SDK requirements ask (§13). Here, a pull request that added integration tests stayed
blocked by the code review until somebody ran them by hand: #89 was left with
"Proof: Run
/integrationtestwith an API token and provide the output".What changes
A new
integrationjob inpull-request.yml(Integration tests (live API, JDK 17)).main.yml, on JDK 17 only. The full{17, 21, 25} matrix stays in
main.ymland behind/integrationtestfull.Testsworkflow on purpose: the code review starts whenTestscompletes, so it finds the live run already finished.
API quota.
API token. There it shows as "skipped", not as a green run that tested nothing.
MARKETDATA_TOKENis empty, it fails (§17.3).verify(needs: changesonly), so the wait is the longer ofthe two jobs and not their sum.
timeout-minutes: 15.Tests passedcounts the new job.successpasses.skippedpasses only for the three reasons above: documentation only, dependabot, or afork.
The on-demand workflow stays (
/integrationtest,/integrationtestfull), now as acomplement:
their code against the live API;
Its check,
Integration tests pass, only exists after somebody comments a command. Branchprotection must not require it; the comments in the workflow now say so.
Documentation:
README.md,CLAUDE.mdand the comments in the three workflows describethe new behaviour.
main.ymlstill runs the full matrix on every push tomain. The build, the tests and the SDKcode are untouched, and so is
CHANGELOG.md: this is a CI change.Security
The repository is public. The API token is never within reach of code from a fork: the trigger is
pull_request(notpull_request_target), which does not pass secrets to forks, and the job isskipped for them as well.
Proof
This pull request changes a workflow, so it counts as code and the new job runs on it: see
Integration tests (live API, JDK 17)in this pull request's checks, andTests passed, whichnow reads its result.
Cost
Each pull request that changes code makes the suite's live API calls once per push, on one JDK.
Before, that happened once per merge to
main, on three.