Skip to content

Commit e1efe86

Browse files
MarketDataDev03MarketDataDev03
committed
adds integration test
1 parent ab8e115 commit e1efe86

10 files changed

Lines changed: 583 additions & 62 deletions

File tree

‎.github/workflows/main.yml‎

Lines changed: 55 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,16 +34,19 @@ jobs:
3434
- name: Checkout
3535
uses: actions/checkout@v4
3636

37-
# Install both JDK 17 (for compilation) and the matrix JDK (for
38-
# test execution). setup-java exports JAVA_HOME_<version>_<arch>;
39-
# Gradle's toolchain auto-detection picks them up.
40-
- name: Set up JDKs (compile=17, test=${{ matrix.java }})
37+
# Install both the matrix JDK (test execution) and JDK 17 (compile +
38+
# Gradle daemon runtime). Order matters: setup-java sets JAVA_HOME
39+
# to the LAST entry, and Gradle 8.12 only supports JDKs up to 23 as
40+
# its own runtime — so JDK 17 must be last for matrix.java = 25.
41+
# setup-java still exports JAVA_HOME_<version>_<arch> for both, and
42+
# setup-gradle registers them as toolchain candidates.
43+
- name: Set up JDKs (test=${{ matrix.java }}, compile/daemon=17)
4144
uses: actions/setup-java@v4
4245
with:
4346
distribution: temurin
4447
java-version: |
45-
17
4648
${{ matrix.java }}
49+
17
4750
4851
- name: Set up Gradle
4952
uses: gradle/actions/setup-gradle@v4
@@ -72,8 +75,50 @@ jobs:
7275
files: build/reports/jacoco/test/jacocoTestReport.xml
7376
fail_ci_if_error: true
7477

75-
# Integration-tests job is intentionally not wired up yet:
76-
# SDK requirements §13 says they run on PRs and release pipelines, but
77-
# they hit the live API and require a MARKETDATA_TOKEN secret. Add this
78-
# job (gated on `if: ${{ secrets.MARKETDATA_TOKEN != '' }}`) once the
79-
# token is configured in the repo's GitHub Actions secrets.
78+
# SDK requirements §13: integration tests run mandatorily on every push
79+
# to main (release-pipeline gate) on the full forward-compat matrix.
80+
# On PRs they're triggered on demand instead — see
81+
# pr-integration-on-demand.yml.
82+
integration-tests:
83+
name: Integration tests (live API, JDK ${{ matrix.java }})
84+
runs-on: ubuntu-latest
85+
needs: verify
86+
strategy:
87+
fail-fast: false
88+
matrix:
89+
java: ['17', '21', '25']
90+
steps:
91+
- name: Checkout
92+
uses: actions/checkout@v4
93+
94+
- name: Set up JDKs (test=${{ matrix.java }}, compile/daemon=17)
95+
uses: actions/setup-java@v4
96+
with:
97+
distribution: temurin
98+
java-version: |
99+
${{ matrix.java }}
100+
17
101+
102+
- name: Set up Gradle
103+
uses: gradle/actions/setup-gradle@v4
104+
105+
- name: Run integration tests against live API
106+
env:
107+
MARKETDATA_TOKEN: ${{ secrets.MARKETDATA_TOKEN }}
108+
MARKETDATA_RUN_INTEGRATION_TESTS: 'true'
109+
run: |
110+
if [ -z "$MARKETDATA_TOKEN" ]; then
111+
echo "::error::MARKETDATA_TOKEN secret is required on main; integration tests must run on every merge."
112+
exit 1
113+
fi
114+
./gradlew integrationTest -PtestJdk=${{ matrix.java }} --stacktrace
115+
116+
- name: Upload integration-test reports on failure
117+
if: failure()
118+
uses: actions/upload-artifact@v4
119+
with:
120+
name: integration-test-reports-jdk${{ matrix.java }}
121+
path: |
122+
build/reports/tests/integrationTest/
123+
build/test-results/integrationTest/
124+
retention-days: 14

‎.github/workflows/pr-integration-on-demand.yml‎

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
name: Integration tests on demand
2+
3+
# Manually triggered by commenting on an open PR:
4+
# `integrationtest` → JDK 17 only
5+
# `integrationtestfull` → full matrix {17, 21, 25}
6+
#
7+
# Integration tests hit the live Market Data API, so we don't run them
8+
# automatically on every PR open/sync (saves API quota + CI minutes).
9+
# They ARE required for merge — branch protection on `main` should list
10+
# "Integration tests pass" as a required status check, which is the
11+
# aggregator job below. PRs cannot merge until a reviewer comments one
12+
# of the two trigger phrases AND the resulting run is green.
13+
#
14+
# Important security note: workflows triggered by `issue_comment` always
15+
# run from the *default branch's* version of the workflow file, not from
16+
# the PR. Adding/changing this file on a feature branch has no effect
17+
# until it lands on main.
18+
on:
19+
issue_comment:
20+
types: [created]
21+
22+
permissions:
23+
contents: read
24+
pull-requests: write # to react and post the result comment
25+
26+
# Multiple trigger comments on the same PR cancel earlier runs.
27+
concurrency:
28+
group: pr-integration-on-demand-${{ github.event.issue.number }}
29+
cancel-in-progress: true
30+
31+
jobs:
32+
guard:
33+
name: Guard
34+
runs-on: ubuntu-latest
35+
# Only fire on PR comments (not generic issue comments) that contain
36+
# one of the two accepted slash-style commands. `contains` is
37+
# substring match — note that `integrationtest` is itself a substring
38+
# of `integrationtestfull`, so this OR matches both, and the matrix
39+
# decision below disambiguates.
40+
if: |
41+
github.event.issue.pull_request != null && (
42+
contains(github.event.comment.body, 'integrationtest') ||
43+
contains(github.event.comment.body, 'integrationtestfull')
44+
)
45+
outputs:
46+
head_sha: ${{ steps.pr.outputs.head_sha }}
47+
jdks: ${{ steps.matrix.outputs.jdks }}
48+
mode: ${{ steps.matrix.outputs.mode }}
49+
steps:
50+
- name: Verify commenter has write permission
51+
uses: actions/github-script@v7
52+
with:
53+
script: |
54+
const { data: perm } = await github.rest.repos.getCollaboratorPermissionLevel({
55+
owner: context.repo.owner,
56+
repo: context.repo.repo,
57+
username: context.payload.comment.user.login,
58+
});
59+
const allowed = ['write', 'maintain', 'admin'].includes(perm.permission);
60+
if (!allowed) {
61+
core.setFailed(
62+
`@${context.payload.comment.user.login} (${perm.permission}) ` +
63+
`cannot trigger integration tests; write access required.`
64+
);
65+
}
66+
67+
- name: React 👀 to the trigger comment
68+
uses: actions/github-script@v7
69+
with:
70+
script: |
71+
await github.rest.reactions.createForIssueComment({
72+
owner: context.repo.owner,
73+
repo: context.repo.repo,
74+
comment_id: context.payload.comment.id,
75+
content: 'eyes',
76+
});
77+
78+
- name: Resolve PR head SHA
79+
id: pr
80+
uses: actions/github-script@v7
81+
with:
82+
script: |
83+
const { data: pr } = await github.rest.pulls.get({
84+
owner: context.repo.owner,
85+
repo: context.repo.repo,
86+
pull_number: context.payload.issue.number,
87+
});
88+
if (pr.state !== 'open') {
89+
core.setFailed(`PR #${pr.number} is ${pr.state}; refusing to run.`);
90+
return;
91+
}
92+
core.setOutput('head_sha', pr.head.sha);
93+
94+
- name: Decide JDK matrix from comment body
95+
id: matrix
96+
env:
97+
BODY: ${{ github.event.comment.body }}
98+
run: |
99+
# Check for the long form first because it contains 'integrationtest' as a substring.
100+
if [[ "$BODY" == *"integrationtestfull"* ]]; then
101+
echo 'jdks=["17","21","25"]' >> "$GITHUB_OUTPUT"
102+
echo 'mode=full' >> "$GITHUB_OUTPUT"
103+
echo "Trigger: integrationtestfull → matrix {17, 21, 25}"
104+
else
105+
echo 'jdks=["17"]' >> "$GITHUB_OUTPUT"
106+
echo 'mode=single' >> "$GITHUB_OUTPUT"
107+
echo "Trigger: integrationtest → JDK 17"
108+
fi
109+
110+
integration-tests:
111+
name: Integration tests (JDK ${{ matrix.java }})
112+
needs: guard
113+
runs-on: ubuntu-latest
114+
strategy:
115+
# Don't cancel siblings: if JDK 21 fails, we still want 17 and 25
116+
# results to surface.
117+
fail-fast: false
118+
matrix:
119+
java: ${{ fromJSON(needs.guard.outputs.jdks) }}
120+
121+
steps:
122+
# Check out exactly the PR's HEAD commit so we test the proposed
123+
# change, not the merge ref.
124+
- name: Checkout PR head
125+
uses: actions/checkout@v4
126+
with:
127+
ref: ${{ needs.guard.outputs.head_sha }}
128+
129+
# Order matters: JDK 17 must be last so JAVA_HOME=17 (Gradle 8.12
130+
# only supports JDKs up to 23 as its daemon runtime). The matrix
131+
# JDK is still installed and registered as a toolchain target.
132+
- name: Set up JDKs (test=${{ matrix.java }}, compile/daemon=17)
133+
uses: actions/setup-java@v4
134+
with:
135+
distribution: temurin
136+
java-version: |
137+
${{ matrix.java }}
138+
17
139+
140+
- name: Set up Gradle
141+
uses: gradle/actions/setup-gradle@v4
142+
143+
- name: Run integration tests against live API
144+
env:
145+
MARKETDATA_TOKEN: ${{ secrets.MARKETDATA_TOKEN }}
146+
MARKETDATA_RUN_INTEGRATION_TESTS: 'true'
147+
run: |
148+
if [ -z "$MARKETDATA_TOKEN" ]; then
149+
echo "::error::MARKETDATA_TOKEN secret missing — cannot run integration tests."
150+
exit 1
151+
fi
152+
./gradlew integrationTest -PtestJdk=${{ matrix.java }} --stacktrace
153+
154+
- name: Upload integration-test reports on failure
155+
if: failure()
156+
uses: actions/upload-artifact@v4
157+
with:
158+
name: integration-test-reports-jdk${{ matrix.java }}
159+
path: |
160+
build/reports/tests/integrationTest/
161+
build/test-results/integrationTest/
162+
retention-days: 14
163+
164+
# Aggregator job. Branch protection on `main` should require this
165+
# check name ("Integration tests pass") so a single required check
166+
# covers both `integrationtest` (matrix=[17]) and `integrationtestfull`
167+
# (matrix=[17,21,25]) modes uniformly. Without this, branch protection
168+
# would have to list the per-matrix-entry check names which only exist
169+
# in the `full` mode.
170+
required-check:
171+
name: Integration tests pass
172+
needs: [guard, integration-tests]
173+
if: always() && needs.guard.result == 'success'
174+
runs-on: ubuntu-latest
175+
steps:
176+
- name: Aggregate matrix outcome
177+
env:
178+
MATRIX_RESULT: ${{ needs.integration-tests.result }}
179+
MODE: ${{ needs.guard.outputs.mode }}
180+
run: |
181+
echo "Mode: $MODE"
182+
echo "Matrix outcome: $MATRIX_RESULT"
183+
if [[ "$MATRIX_RESULT" != "success" ]]; then
184+
echo "::error::One or more integration-test JDK entries failed."
185+
exit 1
186+
fi
187+
echo "All integration tests passed."
188+
189+
- name: Comment outcome on the PR
190+
if: always()
191+
uses: actions/github-script@v7
192+
with:
193+
script: |
194+
const ok = '${{ needs.integration-tests.result }}' === 'success';
195+
const mode = '${{ needs.guard.outputs.mode }}';
196+
const emoji = ok ? '✅' : '❌';
197+
const status = ok ? 'passed' : 'failed';
198+
const matrix = mode === 'full' ? '`{17, 21, 25}`' : '`17`';
199+
const runUrl = `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`;
200+
await github.rest.issues.createComment({
201+
owner: context.repo.owner,
202+
repo: context.repo.repo,
203+
issue_number: context.payload.issue.number,
204+
body: `${emoji} On-demand integration tests on JDK ${matrix} ${status}. [View run](${runUrl}).`,
205+
});

‎.github/workflows/pr-matrix-on-demand.yml‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,14 +110,16 @@ jobs:
110110
with:
111111
ref: ${{ needs.guard.outputs.head_sha }}
112112

113-
# Compile=17, test=matrix JDK; same shape as main.yml.
114-
- name: Set up JDKs (compile=17, test=${{ matrix.java }})
113+
# Order matters: JDK 17 must be last so JAVA_HOME=17 (Gradle 8.12
114+
# doesn't support JDK 24+ as its daemon runtime). The matrix JDK
115+
# is still installed and registered as a toolchain target.
116+
- name: Set up JDKs (test=${{ matrix.java }}, compile/daemon=17)
115117
uses: actions/setup-java@v4
116118
with:
117119
distribution: temurin
118120
java-version: |
119-
17
120121
${{ matrix.java }}
122+
17
121123
122124
- name: Set up Gradle
123125
uses: gradle/actions/setup-gradle@v4

‎.github/workflows/pull-request.yml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,3 +63,10 @@ jobs:
6363
token: ${{ secrets.CODECOV_TOKEN }}
6464
files: build/reports/jacoco/test/jacocoTestReport.xml
6565
fail_ci_if_error: true
66+
67+
# NOTE: integration tests are NOT run automatically on PR open/sync.
68+
# They are required for merge but only fire when a reviewer comments
69+
# `integrationtest` (JDK 17) or `integrationtestfull` (matrix 17/21/25)
70+
# — see .github/workflows/pr-integration-on-demand.yml. Branch-protection
71+
# rules should require the "Integration tests pass" check name produced
72+
# by that workflow before allowing merge to main.

‎CLAUDE.md‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,13 @@ The Java SDK must also satisfy the canonical, cross-language [SDK Requirements](
6868
- §12 concurrency: `Semaphore(50)` field on `MarketDataClient` (wiring of acquire/release lands with the request layer).
6969
- §15 packaging: SemVer, MIT `LICENSE`, `CHANGELOG.md` in Keep a Changelog format, version auto-detected via JAR manifest (`Implementation-Version`).
7070
- §16 security: tokens never logged verbatim (use `Tokens.redact`); TLS validated by default (`HttpClient` does not expose a skip-verify option).
71-
- ADR-002 CI: split into three workflows.
72-
- `.github/workflows/pull-request.yml` — runs on PR `opened`/`synchronize`/`reopened` (no pre-PR push trigger by design). JDK 17 only. Runs `./gradlew build` and uploads `build/reports/jacoco/test/jacocoTestReport.xml` to Codecov.
73-
- `.github/workflows/main.yml` — runs only on `push` to `main`. Full forward-compat matrix `{17, 21, 25}` via `-PtestJdk=N` (wired into `tasks.test.javaLauncher` in `build.gradle.kts`). The JDK 17 matrix entry also uploads coverage to Codecov, establishing the base coverage that PRs compare against.
74-
- `.github/workflows/pr-matrix-on-demand.yml` — manually triggered by commenting one of `/run-all-jdks`, `/jdk-matrix`, or `/test-all` on an open PR. Runs JDK 21 and 25 (17 already ran via `pull-request.yml`). Gated to commenters with write/maintain/admin permission. Reacts 👀 to the trigger comment and posts a result summary comment when the matrix finishes. Note: `issue_comment` workflows always execute from the default branch's copy of the file — feature-branch edits to this workflow have no effect until merged to main.
71+
- ADR-002 CI: split into four workflows.
72+
- `.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.
73+
- `.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).
74+
- `.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.
75+
- `.github/workflows/pr-integration-on-demand.yml` — manually triggered on a PR by commenting `integrationtest` (JDK 17 only) or `integrationtestfull` (matrix `{17, 21, 25}`). Runs the **integration-test** suite against the live API. Same write+ permission gate as the matrix-on-demand workflow. 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.
76+
- 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.
77+
- `-PtestJdk=N` is wired to **all** `Test` tasks (`test` and `integrationTest`) via `tasks.withType<Test>().configureEach { javaLauncher.set(...) }` in `build.gradle.kts`, so the matrix flag works uniformly across unit and integration tests.
7578
- 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`.
7679

7780
**Deliberately deferred (require the request/endpoint layer to land first):**
@@ -83,7 +86,7 @@ The Java SDK must also satisfy the canonical, cross-language [SDK Requirements](
8386
- §9 retry/backoff policy and `/status/` cache workflow.
8487
- §12 acquire/release of the concurrency semaphore around dispatched requests.
8588
- §13 100% coverage threshold via JaCoCo `violationRules`; deferred until there is functional code worth the threshold.
86-
- §13 integration-test CI job: stubbed at the bottom of `ci.yml` with a comment. Gating on a `MARKETDATA_TOKEN` GitHub Actions secret; will be wired up once the secret exists.
89+
- §13 integration-test CI job: wired into both `pull-request.yml` and `main.yml` as a separate `integration-tests` job that depends on `verify`. Pulls `MARKETDATA_TOKEN` from repo secrets, exports `MARKETDATA_RUN_INTEGRATION_TESTS=true`, runs `./gradlew integrationTest`. A shell-level guard skips gracefully with `::warning::` + `exit 0` when the secret isn't available (e.g. fork PRs). The IT class `MarketsStatusIT` parameterizes scenarios over `CallMode.{SYNC,ASYNC}` — `CallMode` is extracted to its own file in `src/test/java/com/marketdata/sdk/markets/CallMode.java` so the integrationTest source set can reuse it via `compileClasspath += sourceSets.test.get().output`.
8790

8891
When picking up new work, check this list before reaching for the SDK requirements doc — most foundational rules are already encoded in code; missing pieces are deferred deliberately, not by accident.
8992

‎build.gradle.kts‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -86,18 +86,19 @@ dependencies {
8686
tasks.test {
8787
useJUnitPlatform()
8888
finalizedBy(tasks.jacocoTestReport)
89+
}
8990

90-
// ADR-002 CI matrix: optionally run tests on a specific JDK while
91-
// compilation stays pinned to --release 17. The CI workflow passes
92-
// -PtestJdk=17|21|25; locally you can do ./gradlew test -PtestJdk=21
93-
// (Gradle will provision the JDK via the foojay resolver if missing).
94-
val testJdk = providers.gradleProperty("testJdk").orNull
95-
if (testJdk != null) {
96-
javaLauncher.set(
97-
javaToolchains.launcherFor {
98-
languageVersion = JavaLanguageVersion.of(testJdk.toInt())
99-
}
100-
)
91+
// ADR-002 CI matrix: optionally run any Test task on a specific JDK
92+
// while compilation stays pinned to --release 17. The flag is wired to
93+
// every Test task (unit `test` + `integrationTest`) so on-demand and
94+
// merge-to-main matrix runs cover the live API on JDK 17/21/25 too.
95+
val testJdkProperty = providers.gradleProperty("testJdk").orNull
96+
if (testJdkProperty != null) {
97+
val launcher = javaToolchains.launcherFor {
98+
languageVersion = JavaLanguageVersion.of(testJdkProperty.toInt())
99+
}
100+
tasks.withType<Test>().configureEach {
101+
javaLauncher.set(launcher)
101102
}
102103
}
103104

0 commit comments

Comments
 (0)