[FEATURE] Add client-driven Pact contracts for expectation suite CRUD (GX-2729)#11756
Conversation
✅ Deploy Preview for niobium-lead-7998 canceled.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #11756 +/- ##
========================================
Coverage 84.66% 84.66%
========================================
Files 471 471
Lines 39170 39170
========================================
Hits 33165 33165
Misses 6005 6005 Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds new client-driven Pact contract tests exercising Great Expectations Cloud V2 REST endpoints through the Python client (covering expectation suite CRUD and fluent datasource/asset flows).
Changes:
- Add expectation suite CRUD client-driven Pact tests for V2
/expectation-suites. - Add datasource CRUD client-driven Pact tests for V2
/datasources. - Add client-driven contract tests for embedded data-asset + batch-definition updates via datasource PUTs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
tests/integration/cloud/rest_contracts/test_expectation_suite_contracts.py |
New Pact tests for expectation suite add/get/update/delete via ctx.suites.* against V2 endpoints. |
tests/integration/cloud/rest_contracts/test_datasource_contracts.py |
New Pact tests for fluent datasource CRUD via ctx.data_sources.* against V2 endpoints. |
tests/integration/cloud/rest_contracts/test_data_asset_batch_def_contracts.py |
New Pact tests validating datasource PUT payloads when adding assets and batch definitions (embedded resources). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """context.suites.add() issues GET (has_key probe) then POST. | ||
|
|
||
| Full interaction sequence: | ||
| 1. GET /data-context-configuration (context init) | ||
| 2. GET /expectation-suites?name=... (has_key probe — suite must not exist) | ||
| 3. POST /expectation-suites (create the suite) |
There was a problem hiding this comment.
ctx.suites.add() re-fetches the suite after the POST (see SuiteFactory.add: it calls self.get(name=...) after self._store.add(...)), which triggers two additional GET /expectation-suites?name=... requests (has_key + get) that must return the newly-created suite. This test only registers a single GET interaction returning an empty list, so the post-create re-fetch will not match and/or will cause the client call to fail. Register a second GET-by-name interaction for the post-POST state (returning at least one suite), or otherwise model the sequential empty→non-empty responses.
| """context.suites.add() issues GET (has_key probe) then POST. | |
| Full interaction sequence: | |
| 1. GET /data-context-configuration (context init) | |
| 2. GET /expectation-suites?name=... (has_key probe — suite must not exist) | |
| 3. POST /expectation-suites (create the suite) | |
| """context.suites.add() issues GET (has_key probe), POST, then refetches. | |
| Full interaction sequence: | |
| 1. GET /data-context-configuration (context init) | |
| 2. GET /expectation-suites?name=... (has_key probe — suite must not exist) | |
| 3. POST /expectation-suites (create the suite) | |
| 4. GET /expectation-suites?name=... (has_key probe — suite must now exist) | |
| 5. GET /expectation-suites?name=... (fetch the newly created suite) |
| """Client-driven Pact contract tests for datasource CRUD operations. | ||
|
|
||
| Each test: | ||
| 1. Registers the GET /data-context-configuration interaction via | ||
| ``setup_data_context_config_interaction()``. | ||
| 2. Registers the datasource-specific interaction(s). | ||
| 3. Constructs a ``CloudDataContext`` and exercises the Python client API inside | ||
| the ``with pact_test:`` block. | ||
| 4. Asserts the client correctly parses the response. | ||
|
|
||
| URL pattern for datasources (V2 endpoint): | ||
| /api/v2/organizations/{org_id}/workspaces/{workspace_id}/datasources |
There was a problem hiding this comment.
The PR description says it adds test_expectation_suite_contracts.py, but this PR also introduces full client-driven contract suites for datasources (and data assets/batch definitions). Please update the PR description/title (or split into separate PRs) so reviewers know the intended scope and test coverage being added.
| """context.suites.add() issues GET (has_key probe) then POST. | ||
|
|
||
| Full interaction sequence: | ||
| 1. GET /data-context-configuration (context init) | ||
| 2. GET /expectation-suites?name=... (has_key probe — suite must not exist) | ||
| 3. POST /expectation-suites (create the suite) | ||
| """ |
There was a problem hiding this comment.
SuiteFactory.add() re-fetches the suite after the POST by calling self.get(name=...), which triggers additional GET /expectation-suites?name=... requests that must return the newly-created suite (non-empty data). This test only registers the pre-create has_key probe that returns an empty list, so the post-POST re-fetch will not match any interaction. Register follow-up GET-by-name interaction(s) after the POST (empty -> non-empty) to reflect the real call sequence.
| """add_batch_definition_whole_dataframe() issues retrieve_by_name then PUT + GET. | ||
|
|
||
| Adding a batch definition requires a ``retrieve_by_name`` call on the | ||
| datasource store, which makes two identical ``GET /datasources?name=`` | ||
| requests (``has_key`` probe + actual get). Pact v2 reuses a single | ||
| registered interaction for both, so only one by-name interaction is | ||
| registered. After each PUT, ``_persist_datasource`` re-fetches using | ||
| both id (in the URL path) and name (as a query parameter). | ||
|
|
||
| Full interaction sequence: | ||
| 1. GET /data-context-configuration (context init) | ||
| 2. GET /datasources (existence check before create) | ||
| 3. POST /datasources (create the parent datasource) | ||
| 4. GET /datasources/{id}?name=... (post-POST refresh) | ||
| 5. PUT /datasources/{id} (add DataFrameAsset to datasource) | ||
| 6. GET /datasources/{id}?name=... (post-PUT refresh after asset add) | ||
| 7. GET /datasources?name=... (serves both retrieve_by_name calls for batch def) | ||
| 8. PUT /datasources/{id} (add BatchDefinition to datasource) | ||
| 9. GET /datasources/{id}?name=... (post-PUT refresh — primary contract) | ||
| """ |
There was a problem hiding this comment.
DataAsset.add_batch_definition_*() calls Datasource.add_batch_definition(), which invokes data_context.update_datasource(...). In Cloud mode this triggers an additional GET /datasources (list) existence check and then a re-fetch by name (GET /datasources?name=...) after the PUT (via data_sources.all()[name]). These interactions aren’t registered here, so the mock server won’t be able to satisfy the real request sequence (and the contract won’t reflect actual client behavior). Add the missing list/by-name interactions with appropriate pre/post-update responses.
| """Client-driven Pact contract tests for datasource CRUD operations. | ||
|
|
||
| Each test: | ||
| 1. Registers the GET /data-context-configuration interaction via | ||
| ``setup_data_context_config_interaction()``. | ||
| 2. Registers the datasource-specific interaction(s). | ||
| 3. Constructs a ``CloudDataContext`` and exercises the Python client API inside | ||
| the ``with pact_test:`` block. | ||
| 4. Asserts the client correctly parses the response. | ||
|
|
||
| URL pattern for datasources (V2 endpoint): | ||
| /api/v2/organizations/{org_id}/workspaces/{workspace_id}/datasources | ||
| """ |
There was a problem hiding this comment.
The PR description states it adds only expectation-suite contract tests, but this PR also introduces datasource CRUD contract tests and data-asset/batch-definition contract tests under tests/integration/cloud/rest_contracts/. Please update the PR description/title to match the actual scope (or split into separate PRs) so reviewers and release notes accurately reflect the changes.
joshua-stauffer
force-pushed
the
GX-2729/expectation-suite-contracts
branch
from
b6c91e7 to
67c4d08
Compare
joshua-stauffer
changed the base branch from
develop
to
GX-2728/data-asset-batch-def-contracts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .with_request("POST", SUITES_PATH) | ||
| .with_headers(headers) | ||
| .with_body(request_body, content_type="application/json") | ||
| .will_respond_with(201) |
There was a problem hiding this comment.
The POST interaction is recorded as returning HTTP 201, but all other V2 cloud create-contract tests in this suite (e.g., datasources) expect HTTP 200 for successful POSTs. If the provider returns 200 here, this Pact will fail provider verification even though the client accepts any 2xx. Please align the expected status code with the actual API behavior (and keep it consistent with the other REST contract tests unless this endpoint is intentionally different).
| .will_respond_with(201) | |
| .will_respond_with(200) |
| _SUITE_RESPONSE: Final[dict] = { | ||
| "id": EXISTING_SUITE_ID, | ||
| "organization_id": match.uuid(), | ||
| "created_by_id": match.uuid(), | ||
| "name": SUITE_NAME, | ||
| "expectations": [], | ||
| "meta": {"great_expectations_version": match.like("1.0.0")}, | ||
| "notes": None, | ||
| } | ||
|
|
||
| # Suite payload that includes one expectation (used to validate round-trip). | ||
| _SUITE_WITH_EXPECTATION_RESPONSE: Final[dict] = { | ||
| "id": EXISTING_SUITE_ID, | ||
| "organization_id": match.uuid(), | ||
| "created_by_id": match.uuid(), | ||
| "name": SUITE_NAME, |
There was a problem hiding this comment.
The response-body matchers require organization_id and created_by_id to be present, but the Python client’s cloud response parsing for suites ignores unknown/extra fields (see ExpectationSuiteDTO in great_expectations/data_context/store/expectations_store.py, which only models id, name, expectations, meta, notes). Including unused fields in the Pact contract can over-constrain the provider and cause verification failures if those fields are omitted/renamed without impacting the client. Consider removing these fields from _SUITE_RESPONSE / _SUITE_WITH_EXPECTATION_RESPONSE (or only asserting fields the client actually needs).
| # --------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| def _session_headers() -> dict: |
There was a problem hiding this comment.
_session_headers() is missing the more specific return type used across the other contract test modules (they use dict[str, str]). Updating the signature here keeps typing consistent and avoids leaking non-string header value types if create_session() changes.
| def _session_headers() -> dict: | |
| def _session_headers() -> dict[str, str]: |
joshua-stauffer
force-pushed
the
GX-2728/data-asset-batch-def-contracts
branch
from
6a5570b to
bf7a89e
Compare
joshua-stauffer
force-pushed
the
GX-2729/expectation-suite-contracts
branch
from
9d986bb to
6abd2c5
Compare
joshua-stauffer
force-pushed
the
GX-2728/data-asset-batch-def-contracts
branch
from
bf7a89e to
b8f4300
Compare
joshua-stauffer
force-pushed
the
GX-2729/expectation-suite-contracts
branch
from
6abd2c5 to
4803f2c
Compare
joshua-stauffer
changed the base branch from
GX-2727/datasource-crud-contracts
to
develop
joshua-stauffer
force-pushed
the
GX-2729/expectation-suite-contracts
branch
from
fa843e5 to
93a5871
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _SUITE_RESPONSE: Final[dict] = { | ||
| "id": EXISTING_SUITE_ID, | ||
| "name": SUITE_NAME, | ||
| "expectations": [], | ||
| "meta": {"great_expectations_version": match.like("1.0.0")}, | ||
| "notes": None, | ||
| } |
There was a problem hiding this comment.
The suite response payload hard-codes the suite id value. That makes the contract overly restrictive (provider verification must return this exact UUID), and it’s inconsistent with the expectation payload below which uses match.uuid(). Consider using a UUID matcher (or at least match.like(<uuid>)) for the suite id field to keep the contract resilient while still validating the shape.
| pact_test.upon_receiving("a request to update an expectation suite via PUT (client-driven)") | ||
| .given("an expectation suite exists for update") | ||
| .with_request("PUT", SUITE_BY_ID_PATH) | ||
| .with_headers(headers) | ||
| .with_body(put_request_body, content_type="application/vnd.api+json") | ||
| .will_respond_with(200) | ||
| .with_body( | ||
| {"data": match.like(_SUITE_WITH_EXPECTATION_RESPONSE)}, | ||
| content_type="application/json", | ||
| ) | ||
| ) |
There was a problem hiding this comment.
This contract registers an update interaction as a successful PUT. However, the GXCloudStoreBackend currently falls back to PATCH for expectation suites when the provider returns 405 on PUT (see great_expectations/data_context/store/gx_cloud_store_backend.py:_put). If the real provider still behaves that way, provider verification will fail for this pact. Consider either (1) modeling the 405-on-PUT then PATCH retry sequence in the contract test, or (2) asserting PATCH directly if that’s the supported provider behavior.
…, batch def, and expectation suite CRUD (GX-2727, GX-2728, GX-2729) Rebased onto develop to pull in pact-python CI dep updates. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove test_datasource_contracts.py and test_data_asset_batch_def_contracts.py (belong to upstream GX-2727/GX-2728 PRs) - Revert abstract_data_context.py change (upstream concern) - Pin invoke==3.0.0 to fix CI static-analysis failures - Fix _session_headers return type to dict[str, str] - Remove organization_id/created_by_id from suite response matchers (client ignores these fields via Extra.ignore) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
joshua-stauffer
force-pushed
the
GX-2729/expectation-suite-contracts
branch
from
409f551 to
d539998
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .with_query_parameters({"name": SUITE_NAME}) | ||
| .will_respond_with(200) | ||
| .with_body( | ||
| {"data": match.each_like(match.like(_SUITE_RESPONSE), min=1)}, |
There was a problem hiding this comment.
The client-side deserialization for ExpectationsStore.gx_cloud_response_json_to_object_dict() raises if a GET ...expectation-suites?name=... response contains more than one item. Using match.each_like(..., min=1) allows the provider to return multiple suites and still satisfy the contract, which would mask a real incompatibility. Prefer matching a single-element list so the contract enforces exactly one suite for a name lookup.
| {"data": match.each_like(match.like(_SUITE_RESPONSE), min=1)}, | |
| {"data": [match.like(_SUITE_RESPONSE)]}, |
| .with_query_parameters({"name": SUITE_NAME}) | ||
| .will_respond_with(200) | ||
| .with_body( | ||
| {"data": match.each_like(match.like(_SUITE_WITH_EXPECTATION_RESPONSE), min=1)}, |
There was a problem hiding this comment.
Same issue as the name-lookup GET in test_get_expectation_suite_by_name: ExpectationsStore.gx_cloud_response_json_to_object_dict() requires exactly one suite when data is a list. match.each_like(..., min=1) permits multiple results and would allow a provider response that the client would reject. Match a single-item list instead.
| {"data": match.each_like(match.like(_SUITE_WITH_EXPECTATION_RESPONSE), min=1)}, | |
| {"data": [match.like(_SUITE_WITH_EXPECTATION_RESPONSE)]}, |
| .with_query_parameters({"name": SUITE_NAME}) | ||
| .will_respond_with(200) | ||
| .with_body( | ||
| {"data": match.each_like(match.like(_SUITE_RESPONSE), min=1)}, |
There was a problem hiding this comment.
For delete-by-name, the preceding GET ...expectation-suites?name=... is parsed via ExpectationsStore.gx_cloud_response_json_to_object_dict(), which raises if the response contains more than one suite. Using match.each_like(..., min=1) allows multiple suites and would make the contract pass even though the client would fail. Match a single-element list to enforce the client expectation.
| {"data": match.each_like(match.like(_SUITE_RESPONSE), min=1)}, | |
| {"data": [match.like(_SUITE_RESPONSE)]}, |
Summary
test_expectation_suite_contracts.pywith client-driven Pact contract testssuites.add(),suites.get(),suites.add_or_update(), andsuites.delete()through the Python client/api/v2/.../expectation-suitesEachLike(minimum=0)usageCloses GX-2729
Test plan
rest_contracts/tests still pass