Add contract testing (Schemathesis internal + Pact public)#125
Add contract testing (Schemathesis internal + Pact public)#125jschoedl wants to merge 3 commits into
Conversation
Verify py-recipe-service and py-help-service responses conform to the internal OpenAPI contract, run in-process over ASGI with the LLM stubbed. Both service CI workflows now also trigger on api/openapi-internal.yaml. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds Schemathesis-based OpenAPI contract conformance tests for py-help-service and py-recipe-service, and Pact consumer/provider contract tests between web-client and spring-api, with corresponding dev dependency pins, a generated pact file, gitattributes marking, and CI workflow trigger path additions. ChangesSchemathesis OpenAPI contract tests
Estimated code review effort: 3 (Moderate) | ~25 minutes Pact consumer/provider contract testing
Estimated code review effort: 4 (Complex) | ~55 minutes Sequence Diagram(s)sequenceDiagram
participant Test as spring-api.pact.test.ts
participant MockServer as Pact Mock Server
participant ConsumerClient as web-client (auth/useApi)
participant PactFile as web-client-spring-api.json
participant ProviderTest as SpringApiProviderPactTest
participant SpringApi as spring-api
Test->>MockServer: define interactions (auth, profile, recipes, AI)
Test->>ConsumerClient: register/login via real client hooks
ConsumerClient->>MockServer: HTTP requests matching interactions
MockServer-->>ConsumerClient: mocked responses
Test->>PactFile: write generated pact contract
ProviderTest->>PactFile: read consumer interactions
ProviderTest->>SpringApi: set up `@State` (seed users/recipes/tokens)
ProviderTest->>SpringApi: replay each interaction via HttpTestTarget
SpringApi-->>ProviderTest: live HTTP response
ProviderTest->>ProviderTest: verifyInteraction against contract
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Consumer-driven Pact over the public REST boundary: the web-client consumer test generates web-client/pacts/, which the spring-api provider test verifies against the real app (JWT auth real, Python GenAI clients mocked). Covers every route the client uses, happy and error paths. The committed pact is the filesystem handoff; spring-api CI triggers on it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jschoedl
force-pushed
the
contract-testing
branch
from
2e310a6 to
1fd4805
Compare
|
🚀 Frontend preview ↗ · commit |
jschoedl
requested review from
imol-ai and
paulwiese
and removed request for
paulwiese
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
services/py-help-service/tests/test_contract.py (1)
30-44: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
app.dependency_overrides.clear()removes all overrides, not just the two set here.If the app has pre-existing dependency overrides,
clear()will wipe them. Consider removing only the keys you set to avoid unintended side effects on other tests sharing the app singleton.♻️ Proposed fix: remove only set overrides
yield - app.dependency_overrides.clear() + app.dependency_overrides.pop(verify_internal_hmac, None) + app.dependency_overrides.pop(get_llm, None)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/py-help-service/tests/test_contract.py` around lines 30 - 44, The test fixture _stub_provider_dependencies currently wipes every dependency override on app with app.dependency_overrides.clear(), which can remove overrides set by other tests or shared setup. Update the teardown to remove only the two overrides this fixture adds, namely verify_internal_hmac and get_llm, while leaving any pre-existing app.dependency_overrides entries intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/py-help-service/tests/test_contract.py`:
- Around line 15-27: The module-level app.openapi_schema assignment is leaking a
shared schema change across tests; save the original app.openapi_schema before
setting the internal contract schema and restore it after the test run in the
autouse fixture. Use the existing app object and the schema setup around
schemathesis.openapi.from_path / schemathesis.openapi.from_asgi so the internal
contract only applies within this contract test module.
In `@services/py-recipe-service/tests/test_contract.py`:
- Around line 15-27: The module-level mutation of app.openapi_schema in this
test file should be moved behind the existing autouse fixture pattern instead of
happening at import time. Update the contract test setup around app, schema, and
the schemathesis.openapi.from_path call so the original openapi_schema value is
saved before assignment and restored after the test(s) complete, matching the
fix used in the help-service test.
In
`@services/spring-api/src/test/kotlin/org/openapitools/pact/SpringApiProviderPactTest.kt`:
- Around line 94-96: The global bearerToken on SpringApiProviderPactTest is
being injected into every interaction once set, even when the pact request does
not include Authorization. Update verifyPact() so it only substitutes the live
token when the interaction’s request already contains the placeholder
Authorization header, and leave other requests unchanged. Use the existing
bearerToken field, the request-building logic in verifyPact(), and the
provider-state setup methods like aUserExists() to scope injection per
interaction.
In `@web-client/tests/pact/spring-api.pact.test.ts`:
- Around line 97-105: The authenticated `/api/v1/users/profile` interaction in
`spring-api.pact.test.ts` overlaps with the unauthenticated 401 case, so Pact
can match the wrong response. Update the profile contract setup around the
`pact.given(...).uponReceiving(...).withRequest(...).willRespondWith(...)`
interaction so the authenticated case is disjoint from the header-less case,
either by using a separate path/matcher or by moving the unauthenticated
scenario into a separate test. Ensure only one interaction can match each
request.
---
Nitpick comments:
In `@services/py-help-service/tests/test_contract.py`:
- Around line 30-44: The test fixture _stub_provider_dependencies currently
wipes every dependency override on app with app.dependency_overrides.clear(),
which can remove overrides set by other tests or shared setup. Update the
teardown to remove only the two overrides this fixture adds, namely
verify_internal_hmac and get_llm, while leaving any pre-existing
app.dependency_overrides entries intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: aaa78a2a-97d4-42cf-8c8a-14f531f97dc9
⛔ Files ignored due to path filters (1)
web-client/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
.gitattributes.github/workflows/build-help-service.yml.github/workflows/build-recipe-service.yml.github/workflows/build-spring-api.ymlservices/py-help-service/dev-requirements.txtservices/py-help-service/tests/test_contract.pyservices/py-recipe-service/dev-requirements.txtservices/py-recipe-service/tests/test_contract.pyservices/spring-api/build.gradle.ktsservices/spring-api/src/test/kotlin/org/openapitools/pact/SpringApiProviderPactTest.ktweb-client/package.jsonweb-client/pacts/web-client-spring-api.jsonweb-client/tests/pact/spring-api.pact.test.ts
| _CONTRACT = ( | ||
| pathlib.Path(__file__).resolve().parents[3] / "api" / "openapi-internal.yaml" | ||
| ) | ||
| app.openapi_schema = schemathesis.openapi.from_path(_CONTRACT).raw_schema | ||
|
|
||
| # /ai/recipes belongs to py-recipe-service | ||
| schema = schemathesis.openapi.from_asgi("/openapi.json", app).exclude( | ||
| path="/ai/recipes" | ||
| ) | ||
|
|
||
| # Happy path only. Auth rejection (missing/invalid HMAC headers) is covered in test_help_service.py. | ||
| schema.config.generation.update(modes=[schemathesis.GenerationMode.POSITIVE]) | ||
| schema.config.phases.update(phases=["examples", "fuzzing"]) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Module-level app.openapi_schema mutation persists without restoration.
app.openapi_schema is set at import time (line 18) and never restored. Since app is a shared singleton from main, any other test module in the same session that relies on the auto-generated OpenAPI schema will see the internal contract schema instead. Consider saving and restoring the original value in the autouse fixture.
🛡️ Proposed fix: save/restore openapi_schema in fixture
app.openapi_schema = schemathesis.openapi.from_path(_CONTRACT).raw_schema
# /ai/recipes belongs to py-recipe-service
schema = schemathesis.openapi.from_asgi("/openapi.json", app).exclude(
path="/ai/recipes"
)
# Happy path only. Auth rejection (missing/invalid HMAC headers) is covered in test_help_service.py.
schema.config.generation.update(modes=[schemathesis.GenerationMode.POSITIVE])
schema.config.phases.update(phases=["examples", "fuzzing"])
+_original_openapi_schema = app.openapi_schema
+
`@pytest.fixture`(autouse=True)
def _stub_provider_dependencies():
app.dependency_overrides[verify_internal_hmac] = lambda: None
@@ -42,7 +44,10 @@
yield
app.dependency_overrides.clear()
+ app.openapi_schema = _original_openapi_schema🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/py-help-service/tests/test_contract.py` around lines 15 - 27, The
module-level app.openapi_schema assignment is leaking a shared schema change
across tests; save the original app.openapi_schema before setting the internal
contract schema and restore it after the test run in the autouse fixture. Use
the existing app object and the schema setup around
schemathesis.openapi.from_path / schemathesis.openapi.from_asgi so the internal
contract only applies within this contract test module.
| _CONTRACT = ( | ||
| pathlib.Path(__file__).resolve().parents[3] / "api" / "openapi-internal.yaml" | ||
| ) | ||
| app.openapi_schema = schemathesis.openapi.from_path(_CONTRACT).raw_schema | ||
|
|
||
| # /ai/help belongs to py-help-service | ||
| schema = schemathesis.openapi.from_asgi("/openapi.json", app).exclude(path="/ai/help") | ||
|
|
||
| # Happy path only. Auth rejection (missing/invalid HMAC headers) is covered in test_recipe_service.py. | ||
| schema.config.generation.update(modes=[schemathesis.GenerationMode.POSITIVE]) | ||
| schema.config.phases.update(phases=["examples", "fuzzing"]) | ||
|
|
||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Same module-level app.openapi_schema concern as help-service test.
This file has the identical pattern: app.openapi_schema is mutated at module level (line 18) without restoration. The same fix applies — save and restore in the autouse fixture.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/py-recipe-service/tests/test_contract.py` around lines 15 - 27, The
module-level mutation of app.openapi_schema in this test file should be moved
behind the existing autouse fixture pattern instead of happening at import time.
Update the contract test setup around app, schema, and the
schemathesis.openapi.from_path call so the original openapi_schema value is
saved before assignment and restored after the test(s) complete, matching the
fix used in the help-service test.
| /** Bearer token for the current interaction, set when a state creates a user. */ | ||
| private var bearerToken: String? = null | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Header injection isn't scoped to interactions that actually require Authorization.
verifyPact() unconditionally injects the live bearer token into every request once bearerToken is non-null — but bearerToken gets set by shared provider-state methods like aUserExists() (Line 112-115), which is also the state for "a valid login request". That pact interaction has no Authorization header in its recorded request, yet this code adds one anyway, breaking exact contract replay for that interaction. This works "by accident" only because Spring Security presumably ignores a stray header on the public /users/login endpoint — but it silently deviates from the pact and could mask real security-filter regressions.
Only substitute the token when the request built from the pact interaction already carries the placeholder Authorization header:
🔧 Proposed fix
fun verifyPact(
context: PactVerificationContext,
request: HttpRequest,
) {
- bearerToken?.let { request.setHeader("Authorization", "Bearer $it") }
+ if (request.containsHeader("Authorization")) {
+ bearerToken?.let { request.setHeader("Authorization", "Bearer $it") }
+ }
context.verifyInteraction()
}Also applies to: 107-115, 151-159
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@services/spring-api/src/test/kotlin/org/openapitools/pact/SpringApiProviderPactTest.kt`
around lines 94 - 96, The global bearerToken on SpringApiProviderPactTest is
being injected into every interaction once set, even when the pact request does
not include Authorization. Update verifyPact() so it only substitutes the live
token when the interaction’s request already contains the placeholder
Authorization header, and leave other requests unchanged. Use the existing
bearerToken field, the request-building logic in verifyPact(), and the
provider-state setup methods like aUserExists() to scope injection per
interaction.
| pact | ||
| .given('a user testuser exists') | ||
| .uponReceiving('an authenticated request for the current user profile') | ||
| .withRequest({ method: 'GET', path: '/api/v1/users/profile', headers: bearer }) | ||
| .willRespondWith({ | ||
| status: 200, | ||
| headers: jsonResponse, | ||
| body: like({ username: 'testuser', preferences: {} }), | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,320p' web-client/tests/pact/spring-api.pact.test.tsRepository: AET-DevOps26/team-devsecops
Length of output: 10383
🏁 Script executed:
rg -n '"`@pact-foundation/pact`"|pact' web-client/package.json package.json pnpm-lock.yaml yarn.lock package-lock.jsonRepository: AET-DevOps26/team-devsecops
Length of output: 448
🌐 Web query:
Pact V3 mock server matching two interactions same method and path one with required Authorization header and one without headers, does a request with Authorization match both or choose the most specific interaction
💡 Result:
In Pact, the mock server does not have an explicit built-in mechanism to prioritize the "most specific" interaction when multiple interactions match the same incoming request [1]. When multiple interactions qualify as a match, the mock server typically returns a warning and defaults to selecting one—often based on the order in which they appear in the contract file [1]. While Pact provides a sophisticated "matcher selection algorithm" for determining how to match nested JSON elements or body contents using weighted paths [2][3][4], this logic does not automatically apply to the selection between two different top-level interactions that both satisfy the request method, path, and headers. Because Pact's matching philosophy for requests is generally strict—requiring that all defined expectations must be met [5][6]—a request containing an Authorization header will technically "match" an interaction that requires an Authorization header, and it will also "match" an interaction that defines no headers (as Pact allows extra, unexpected headers in requests) [7][6]. When the mock server encounters this ambiguity, it does not guarantee the selection of the more specific match [1]. To manage this behavior: 1. Ordering: Many users work around this by ensuring the most specific interaction appears later in the contract (or within the test suite definition) so that it is registered or evaluated in a way that overrides the less specific one [1]. 2. Strictness: If you need to distinguish between these two scenarios, you must define the expectations in a way that the "less specific" interaction would fail to match the "more specific" request. For example, if you define the "no headers" interaction to explicitly exclude the Authorization header (if the implementation supports it) or rely on creating separate tests where only one interaction is active at a time to prevent overlapping matches [6]. In summary, the mock server will not reliably choose the most specific interaction by default [1]. You should structure your tests to avoid having multiple interactions that are simultaneously valid for the same request [6].
Citations:
- 1: Return most specific match when multiple matches are found pact-foundation/pact-mock_service#138
- 2: https://github.com/pact-foundation/pact-specification/blob/version-3/README.md
- 3: https://docs.pact.io/implementation_guides/jvm/matching
- 4: https://docs.rs/pact_matching/latest/pact_matching/
- 5: https://github.com/pact-foundation/pact-specification
- 6: https://github.com/pact-foundation/pact-ruby/wiki/Understanding-Request-Matching
- 7: https://docs.pact.io/getting_started/matching
Avoid overlapping /users/profile interactions
The authenticated 200 response and the header-less 401 response can both match the same request. Pact doesn’t reliably pick the more specific interaction here, so the authenticated profile call can resolve to the wrong response intermittently. Split the unauthenticated case into a disjoint matcher/path or a separate test so only one interaction can match.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-client/tests/pact/spring-api.pact.test.ts` around lines 97 - 105, The
authenticated `/api/v1/users/profile` interaction in `spring-api.pact.test.ts`
overlaps with the unauthenticated 401 case, so Pact can match the wrong
response. Update the profile contract setup around the
`pact.given(...).uponReceiving(...).withRequest(...).willRespondWith(...)`
interaction so the authenticated case is disjoint from the header-less case,
either by using a separate path/matcher or by moving the unauthenticated
scenario into a separate test. Ensure only one interaction can match each
request.
imol-ai
left a comment
imol-ai
left a comment
There was a problem hiding this comment.
good work! feel free to merge after fixing the conflict
2 participants
Summary
Adds contract testing across both inter-service boundaries, as required by the course best-practices. Each boundary uses the tool that fits its shape:
api/openapi-internal.yaml. Schemathesis derives cases from the spec and checks every response conforms (the LLM is stubbed). It re-syncs itself when the spec changes.web-client/pacts/; the provider test replays it against the real Spring app (real JWT/security, Python GenAI clients mocked). Covers every route the client uses, happy and error paths (401/404/409/400).Type of change
API changes
api/openapi.yamlupdated andapi/scripts/gen-all.shre-runDefinition of Done
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests