diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 2fa86e2..4d7e934 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -77,7 +77,8 @@ jobs: # SDK requirements §13: integration tests run mandatorily on every push # to main (release-pipeline gate) on the full forward-compat matrix. - # On PRs they're triggered on demand instead — see + # Pull requests run them on JDK 17 only — see the `integration` job in + # pull-request.yml — and on the full matrix on demand, see # pr-integration-on-demand.yml. integration-tests: name: Integration tests (live API, JDK ${{ matrix.java }}) diff --git a/.github/workflows/pr-integration-on-demand.yml b/.github/workflows/pr-integration-on-demand.yml index 35bee1a..942f27f 100644 --- a/.github/workflows/pr-integration-on-demand.yml +++ b/.github/workflows/pr-integration-on-demand.yml @@ -13,12 +13,19 @@ name: Integration tests on demand # then a strict bash `case` validates the exact command — anything that # slips through is rejected before any live-API request is made. # -# Integration tests hit the live Market Data API, so we don't run them -# automatically on every PR open/sync (saves API quota + CI minutes). -# They ARE required for merge — branch protection on `main` should list -# "Integration tests pass" as a required status check, which is the -# aggregator job below. PRs cannot merge until a reviewer comments one -# of the two slash-commands AND the resulting run is green. +# Every pull request already runs the suite on JDK 17: the `integration` +# job of pull-request.yml, whose result is part of the required +# "Tests passed" check. This workflow is for what that job does not cover: +# - the full {17, 21, 25} matrix before merge (`/integrationtestfull`); +# - pull requests from forks and from dependabot, which cannot read the +# API token, so the automatic job skips and a maintainer decides +# whether to run their code against the live API; +# - running the suite again without pushing a new commit. +# +# The aggregator job below reports as "Integration tests pass". It only +# exists once somebody has commented one of the two commands, so branch +# protection on `main` must NOT require it: a pull request nobody +# commented on would wait for it forever. # # Important security note: workflows triggered by `issue_comment` always # run from the *default branch's* version of the workflow file, not from @@ -182,12 +189,10 @@ jobs: build/test-results/integrationTest/ retention-days: 14 - # Aggregator job. Branch protection on `main` should require this - # check name ("Integration tests pass") so a single required check - # covers both `integrationtest` (matrix=[17]) and `integrationtestfull` - # (matrix=[17,21,25]) modes uniformly. Without this, branch protection - # would have to list the per-matrix-entry check names which only exist - # in the `full` mode. + # Aggregator job. One check name ("Integration tests pass") covers both + # `/integrationtest` (matrix=[17]) and `/integrationtestfull` + # (matrix=[17,21,25]) uniformly, where the per-matrix-entry check names + # differ between the two modes. Not a required check: see the header. required-check: name: Integration tests pass needs: [guard, integration-tests] diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index ee036f4..66046be 100644 --- a/.github/workflows/pull-request.yml +++ b/.github/workflows/pull-request.yml @@ -131,18 +131,85 @@ jobs: files: build/reports/jacoco/test/jacocoTestReport.xml fail_ci_if_error: true - # NOTE: integration tests are NOT run automatically on PR open/sync. - # They are required for merge but only fire when a reviewer comments - # `integrationtest` (JDK 17) or `integrationtestfull` (matrix 17/21/25) - # — see .github/workflows/pr-integration-on-demand.yml. Branch-protection - # rules should require the "Integration tests pass" check name produced - # by that workflow before allowing merge to main. + integration: + name: Integration tests (live API, JDK 17) + # SDK requirements §13: the live suite runs on every pull request. JDK 17 + # only, like `/integrationtest`; the full {17, 21, 25} matrix stays in + # main.yml and behind `/integrationtestfull`. + # + # `needs: changes` and NOT `needs: verify`. The code review starts when + # this workflow completes and reads the result of this job, so running the + # two side by side is what keeps the wait at the longer of them rather + # than their sum. The live suite takes about two minutes and a build that + # does not compile fails here before a single request is sent, so the + # quota a red `verify` would have saved is small. sdk-py, sdk-csharp, + # sdk-go and sdk-php gate their live job on `changes` alone as well. + needs: changes + # Three conditions, and `Tests passed` below accepts a skip for exactly + # these three and no other: + # 1. the gate did not say "documentation only", so a markdown change + # spends no live API quota. `!= 'false'` so a failed gate still runs; + # 2. the pull request was not opened by dependabot, and + # 3. it does not come from a fork: neither receives repository secrets + # on `pull_request`. They skip this JOB, which shows as "skipped" in + # the checks, not as a green run that tested nothing. + # Clause 2 reads the AUTHOR of the pull request, not `github.actor`, which + # is whoever triggered this run. A maintainer who reopens a dependabot + # pull request becomes the actor, and the run then does receive the + # token, so a test on the actor would send a freshly bumped dependency's + # code to the live API without anybody having decided to. A dependabot + # pull request never runs the suite on its own, whoever triggers the run; + # a maintainer runs it for one, or for a fork, with `/integrationtest`. + # A head repository that has since been deleted reads as empty, which is + # not this repository, so it skips too. + if: >- + needs.changes.outputs.code != 'false' && + github.event.pull_request.user.login != 'dependabot[bot]' && + github.event.pull_request.head.repo.full_name == github.repository + runs-on: ubuntu-latest + # Every test talks to the live API; without a cap one hung request would + # hold the runner until the 6-hour default. + timeout-minutes: 15 + steps: + - name: Checkout + uses: actions/checkout@v7 + + - name: Set up JDK 17 + uses: actions/setup-java@v5 + with: + distribution: temurin + java-version: '17' + + - name: Set up Gradle + uses: gradle/actions/setup-gradle@v4 + + # SDK requirements §17.3: a missing token fails the job, never skips it. + - name: Run integration tests against live API + env: + MARKETDATA_TOKEN: ${{ secrets.MARKETDATA_TOKEN }} + MARKETDATA_RUN_INTEGRATION_TESTS: 'true' + run: | + if [ -z "$MARKETDATA_TOKEN" ]; then + echo "::error::MARKETDATA_TOKEN secret is missing; the live integration suite fails rather than skips (SDK requirements 17.3)." + exit 1 + fi + ./gradlew integrationTest -PtestJdk=17 --stacktrace + + - name: Upload integration-test reports on failure + if: failure() + uses: actions/upload-artifact@v4 + with: + name: integration-test-reports-jdk17 + path: | + build/reports/tests/integrationTest/ + build/test-results/integrationTest/ + retention-days: 14 tests-passed: # The single context branch protection requires. It always reports, so it is # always a decision rather than a silence. name: Tests passed - needs: [changes, verify] + needs: [changes, verify, integration] if: always() runs-on: ubuntu-latest timeout-minutes: 2 @@ -152,9 +219,16 @@ jobs: env: CODE: ${{ needs.changes.outputs.code }} R0: ${{ needs.verify.result }} + R1: ${{ needs.integration.result }} + # The negation of the last two clauses of the `integration` job's + # `if:`, evaluated by the same expression engine so the two cannot + # disagree: `true` for a pull request opened by dependabot, whoever + # triggered this run, and for a fork. + NO_SECRETS: ${{ github.event.pull_request.user.login == 'dependabot[bot]' || github.event.pull_request.head.repo.full_name != github.repository }} run: | echo "gate said code=${CODE:-}" echo "verify: ${R0}" + echo "integration: ${R1} (pull request without secrets: ${NO_SECRETS})" # Two classes of job, because they skip for different reasons. # @@ -175,4 +249,22 @@ jobs: esac done + # LIVE jobs need the API token, so they have one more acceptable skip: + # a pull request opened by dependabot or coming from a fork, which + # never runs the suite on its own. Nothing else is accepted, and a + # FAILURE always fails: that includes the job refusing to run without + # its token on a pull request that should have had one. + for result in "${R1}"; do + case "$result" in + success) ;; + skipped) + if [ "$CODE" != "false" ] && [ "$NO_SECRETS" != "true" ]; then + echo "The live suite was skipped, and neither the gate nor a pull request without secrets explains it." + exit 1 + fi + ;; + *) echo "The live suite reported: $result"; exit 1 ;; + esac + done + echo "Every job passed, or skipped for a reason this gate accepts." diff --git a/CLAUDE.md b/CLAUDE.md index fe9f10d..4de4147 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -74,10 +74,10 @@ The Java SDK must also satisfy the canonical, cross-language [SDK Requirements]( - §15 packaging: SemVer, MIT `LICENSE`, `CHANGELOG.md` in Keep a Changelog format, version auto-detected via JAR manifest (`Implementation-Version`). - §16 security: tokens never logged verbatim (use `Tokens.redact`); TLS validated by default (`HttpClient` does not expose a skip-verify option); query strings stripped from log output (logged as `/path?…`) so request params never persist to logs — exception context retains the full URI for diagnostic use; `EnvVars.systemLookup()` restricts reads to the declared `MARKETDATA_*` keys. - ADR-002 CI: split into four workflows. - - `.github/workflows/pull-request.yml` — runs on PR `opened`/`synchronize`/`reopened` (no pre-PR push trigger by design). JDK 17 only. Runs `./gradlew build` (unit tests + Spotless + JaCoCo) and uploads coverage to Codecov. **Does not** run integration tests — those are handled by the on-demand workflow below. + - `.github/workflows/pull-request.yml` — runs on PR `opened`/`synchronize`/`reopened` (no pre-PR push trigger by design). JDK 17 only. The `verify` job runs `./gradlew build` (unit tests + Spotless + JaCoCo) and uploads coverage to Codecov. The `integration` job runs the **integration-test** suite against the live API in parallel with it (`needs: changes` only, so the code review that starts when this workflow completes does not wait for the two in sequence). `integration` skips for a documentation-only PR and for PRs from dependabot or a fork, which cannot read `MARKETDATA_TOKEN`; when it does run and the secret is empty it fails rather than skips. `Tests passed` is the single check branch protection requires: it accepts `success` from both jobs, and a skip only for one of those reasons. - `.github/workflows/main.yml` — runs only on `push` to `main`. Two jobs: `verify` does the full forward-compat matrix `{17, 21, 25}` for unit tests via `-PtestJdk=N`; `integration-tests` does a parallel matrix `{17, 21, 25}` against the live API. Both are mandatory for the merge to be considered successful. The JDK 17 matrix entry of `verify` also uploads coverage to Codecov as the new baseline that PRs compare against. `integration-tests` fails the build if `MARKETDATA_TOKEN` secret is absent (it is required on main). - `.github/workflows/pr-matrix-on-demand.yml` — manually triggered on a PR by commenting `/run-all-jdks`, `/jdk-matrix`, or `/test-all`. Runs the **unit-test** matrix on JDK 21 and 25 (17 already ran via `pull-request.yml`). Gated to write/maintain/admin commenters. Reacts 👀 to the trigger comment and posts a result summary. - - `.github/workflows/pr-integration-on-demand.yml` — manually triggered on a PR by commenting `/integrationtest` (JDK 17 only) or `/integrationtestfull` (matrix `{17, 21, 25}`) on the **first line** of the comment body. Runs the **integration-test** suite against the live API. Same write+ permission gate as the matrix-on-demand workflow. The first-line + exact-match constraint prevents accidental triggers from quoted replies (`> /integrationtest`) or prose that mentions the command. Aggregates the matrix outcome into a single required check named **"Integration tests pass"** so branch protection can require it uniformly regardless of which command was used. Branch-protection rules on `main` should list this check as required for merge. + - `.github/workflows/pr-integration-on-demand.yml` — manually triggered on a PR by commenting `/integrationtest` (JDK 17 only) or `/integrationtestfull` (matrix `{17, 21, 25}`) on the **first line** of the comment body. Runs the **integration-test** suite against the live API. Every PR already runs it on JDK 17 via `pull-request.yml`; this workflow is for the full matrix, for fork and dependabot PRs (where the automatic job skips), and for re-running without a new push. Same write+ permission gate as the matrix-on-demand workflow. The first-line + exact-match constraint prevents accidental triggers from quoted replies (`> /integrationtest`) or prose that mentions the command. Aggregates the matrix outcome into a single check named **"Integration tests pass"**, uniform regardless of which command was used. It only exists once someone has commented a command, so branch protection must **not** require it — the required check is `Tests passed`. - All four `issue_comment`-driven workflows execute from the default branch's copy of their YAML, not the PR's. Feature-branch edits to these workflows take effect only after merge to main. - `-PtestJdk=N` is wired to **all** `Test` tasks (`test` and `integrationTest`) via `tasks.withType().configureEach { javaLauncher.set(...) }` in `build.gradle.kts`, so the matrix flag works uniformly across unit and integration tests. - Coverage ratchet lives in `codecov.yml`: project status with `target: auto, threshold: 5%` (cannot drop >5 pp vs base branch) plus a patch-coverage requirement of 70 % on new code. Requires a `CODECOV_TOKEN` repo secret — without it the upload step fails because workflows pass `fail_ci_if_error: true`. diff --git a/README.md b/README.md index e4a029f..19c2249 100644 --- a/README.md +++ b/README.md @@ -345,9 +345,15 @@ install — the wrapper downloads the right Gradle version on first run. MARKETDATA_RUN_INTEGRATION_TESTS=true ./gradlew integrationTest ``` -On PRs, integration tests are not run automatically (live-API quota + -CI minutes). A reviewer with `write` access triggers them by posting a -slash-command on the **first line** of a PR comment: +Every pull request runs the integration tests against the live API on +JDK 17, and their result is part of the required `Tests passed` check. +Two kinds of pull request skip them: one that only changes documentation, +and one from a fork or from dependabot, which cannot read the API token. + +A reviewer with `write` access can also run them by posting a +slash-command on the **first line** of a PR comment — for the full JDK +matrix, for a fork or dependabot pull request, or to run them again +without a new push: - `/integrationtest` — JDK 17 only. - `/integrationtestfull` — full matrix `{17, 21, 25}`.