test: unit + integration suite for downstream usage, with CI - #18
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an initial automated test suite and GitHub Actions CI for dspace_rest_client, focusing on downstream consumer contracts by mocking only HTTP transport while exercising real URL construction and response parsing.
Changes:
- Added unit-style contract tests for models and all downstream-used
DSpaceClientread/write/auth methods. - Added higher-level “usage chain” contract tests that replay multi-call flows used by downstream tooling.
- Added CI workflow + test dependency set to run the suite on Python 3.10 and 3.12.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tests/test_repo_usage_contract.py |
End-to-end contract tests mirroring downstream multi-call chains. |
tests/test_models.py |
Pins model construction/accessor behavior used by downstream code. |
tests/test_client_write.py |
Covers write-path calls (create policy/bundle/item/bitstream; delete). |
tests/test_client_read.py |
Covers read-path calls and key error/edge contracts (e.g., 404 bundles). |
tests/test_client_auth.py |
Covers constructor-derived endpoints + authenticate True/False semantics. |
tests/conftest.py |
Ensures in-tree package importability under pytest. |
tests/_helpers.py |
Shared request/response builders and request inspection helpers. |
requirements-test.txt |
Declares test-only dependencies (pytest + requests-mock + requests). |
.gitignore |
Ignores common Python build/test artifacts. |
.github/workflows/tests.yml |
Adds GitHub Actions workflow running the test suite across a small Python matrix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+57
to
+69
| def multipart_properties(request) -> dict: | ||
| """Parse the JSON ``properties`` part of a create_bitstream multipart body. | ||
|
|
||
| ``create_bitstream`` sends ``properties = json.dumps({name, metadata, | ||
| bundleName}) + ';application/json'`` as a form field. This is what actually | ||
| carries the bitstream's metadata to DSpace, so tests assert on it. | ||
| """ | ||
| body = request.body | ||
| if isinstance(body, bytes): | ||
| body = body.decode("utf-8", "replace") | ||
| m = re.search(r'name="properties"\r?\n\r?\n(.*?);application/json', | ||
| body, re.DOTALL) | ||
| return json.loads(m.group(1)) if m else None |
DSpace-ISstag-integration drives this client from src/{ingest,export,reposync}
and mcp/, but nothing here was under test - a library change could silently
break that consumer. Add a pytest suite that mocks only the HTTP transport
(requests_mock) and exercises the real URL-building and response-parsing for
every DSpaceClient method and model the consumer relies on:
- test_models: Item/Community/Collection/Bundle/Bitstream/ResourcePolicy
construction + accessors (incl. the live _embedded.group -> groupUUID lift)
- test_client_auth: constructor + authenticate() True/False semantics
- test_client_read: search_objects, get_items, get_item, get_bundles (incl.
the 404 -> [] contract from #16), get_bitstreams, get_collections,
get_communities, get_resourcepolicy, fetch_resource (last_err on 404)
- test_client_write: create_resourcepolicy, api_delete, create_bundle,
create_item, create_bitstream (multipart upload)
- test_repo_usage_contract: the exact multi-call chains the consumer runs
(bitstream export, resource-policy replacement, MCP bundle walk, group-uuid
resolution)
Adds .github/workflows/tests.yml (pytest on Python 3.10 + 3.12) and
requirements-test.txt. 56 tests, no network.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Acting on a two-reviewer audit (workflow + Opus-5 advisor): Faithfulness fixes (tests that could pass against a broken library): - get_bitstreams embedded-link test used an href byte-identical to the fallback URL, so it could not distinguish the branches; give it a distinct href that the fallback could not produce. - MCP chain fed a link-bearing Bundle to get_bitstreams; the real consumer round-trips through as_dict() (drops _links) and hits the fallback URL - rebuild the Bundle from as_dict() and assert the fallback is used. - export chain fetched policies with action='READ' (raw-client default) but the real exporter goes through a wrapper defaulting to action=None (no filter); call with action=None and assert no action param is sent. - get_resourcepolicy empty test used a no-_embedded body (defensive branch); the live API returns an _embedded envelope even when empty - use that. Coverage / stronger assertions: - get_resourcepolicy action=None omits the filter and returns all actions. - create_item now asserts the POST body (name/metadata/type/flags), not just the uuid; create_bitstream asserts the multipart 'properties' payload (name/bundleName/metadata). - search_objects result now asserts .as_dict() and links['self']['href'], the two accessors every consumer reads. - model tests assert parsed .metadata and checkSum.checkSumAlgorithm (dropped the tautological hard-set .type assertions' reliance). - get_bitstreams non-200, and create_item/create_bundle server-error: characterization tests pinning the current (non-fail-safe) behavior the consumers depend on, flagged in-comment for a future library hardening. 60 tests, still no network. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dtq-dev no longer exists, and we only care about dtq for now. PRs still trigger via the pull_request event. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ndle Review follow-ups, each confirmed by the tests: - get_bitstreams: a 404 now returns [] (a gone bundle has no bitstreams), mirroring the get_bundles #16 fix; other errors still surface so a transient 5xx is retried, not swallowed; a 200 with no bitstreams returns [] not None. The consumers (export/_dspace, reposync/_files) iterate the result unguarded. - create_item / create_bundle: return None on a non-2xx response instead of a truthy uuid-less object, so the importer's `if dso is None` / `if not bundle` guards actually fire. Also addresses the Copilot review: the multipart_properties test helper now asserts the 'properties' part exists (fails loudly) instead of returning None. Tests updated to assert the new behavior. 62 tests, no network. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
vidiecan
force-pushed
the
test/dtq-usage-coverage
branch
from
August 17, 2026 16:59
2a1d13d to
a2b7140
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (1)
dspace_rest_client/client.py:814
get_bitstreams()can crash with an unhelpfulTypeErrorwhenfetch_resource()returnsNonefor a non-404 error (e.g., 500). After the 404 special-case, the code still executes'_embedded' in r_jsoneven whenr_jsonisNone, so callers won’t see the HTTP status/URL that failed. Consider raising an explicit exception whenr_json is Noneand the status isn’t 404.
bitstreams = list()
if '_embedded' in r_json and 'bitstreams' in r_json['_embedded']:
for bitstream_resource in r_json['_embedded']['bitstreams']:
bitstreams.append(Bitstream(bitstream_resource))
return bitstreams
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds the first automated test suite (and CI) to this client. Until now the
library had no tests, so a change to it could silently break its downstream
consumer DSpace-ISstag-integration, which drives it from
src/{ingest,export,reposync}andmcp/.The tests mock only the HTTP transport (
requests_mock) and let the realDSpaceClientbuild URLs, send params and parse responses into model objects —so a regression in URL construction or response parsing (the two things the
consumer depends on) fails a test instead of shipping.
Coverage (every DSpaceClient method + model the consumer uses)
tests/test_models.pyItem / Community / Collection / Bundle / Bitstream / ResourcePolicyconstruction + accessors, incl. the live_embedded.group → groupUUIDlifttests/test_client_auth.pyauthenticate()True/False semantics, CSRF-403 retrytests/test_client_read.pysearch_objects,get_items,get_item,get_bundles(incl. the404 → []contract from #16),get_bitstreams,get_collections,get_communities,get_resourcepolicy,fetch_resource(last_erron 404)tests/test_client_write.pycreate_resourcepolicy,api_delete,create_bundle,create_item,create_bitstream(multipart upload)tests/test_repo_usage_contract.pyCI
.github/workflows/tests.ymlrunspyteston Python 3.10 and 3.12 (matchesthe consumer's CI matrix; the library declares
>=3.8). No network in any test.56 tests, ~0.2s locally.