Usage tests: bulk data download#218
Open
NandaScott wants to merge 1 commit into
Open
Conversation
Extend stub_response to intercept bulk_data_mixins.urlopen so download() can be exercised without network access. A new conftest fixture arms both the API metadata stub and a synthetic bulk payload; the test asserts that download() returns the dataset via the public return value only.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new usage test to validate that scrython.bulk_data.ById(...).download() returns a full dataset when the bulk payload is supplied via the usage-test harness seam (issue #207).
Changes:
- Add a new usage test covering
bulk_data.ById.download()returning parsed JSON data. - Extend the
tests/usageHTTP stub seam to support a separate “bulk-data/download” payload path (CDN download interception). - Add a minimal synthetic bulk dataset fixture for the download test.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| tests/usage/test_bulk_data_download.py | New usage test asserting download() returns a dataset via public surface. |
| tests/usage/conftest.py | Extend stub_response to also intercept bulk download calls and provide a synthetic bulk payload fixture. |
Comments suppressed due to low confidence (3)
tests/usage/conftest.py:86
BulkDataObjectMixin.download(progress=True)usesresponse.headers.get(...)andresponse.info().get(...). The current_MockResponsedoesn’t defineheadersand doesn’t provide a realinfo().getimplementation, which will raise (or yield non-strings) if any usage test enablesprogress=Trueor if download starts relying on the Content-Encoding value being a string.
def __init__(self, data: dict | list) -> None:
self._data = json.dumps(data).encode("utf-8")
self._info = Mock()
self._info.get_param = Mock(return_value="utf-8")
tests/usage/conftest.py:86
BulkDataObjectMixin.download(progress=True)usesresponse.headers.get(...)andresponse.info().get(...). The current_MockResponsedoesn’t defineheadersand doesn’t provide a realinfo().getimplementation, which will raise (or yield non-strings) if any usage test enablesprogress=Trueor if download starts relying on the Content-Encoding value being a string.
def __init__(self, data: dict | list) -> None:
self._data = json.dumps(data).encode("utf-8")
self._info = Mock()
self._info.get_param = Mock(return_value="utf-8")
tests/usage/conftest.py:86
BulkDataObjectMixin.download(progress=True)usesresponse.headers.get(...)andresponse.info().get(...). The current_MockResponsedoesn’t defineheadersand doesn’t provide a realinfo().getimplementation, which will raise (or yield non-strings) if any usage test enablesprogress=Trueor if download starts relying on the Content-Encoding value being a string.
def __init__(self, data: dict | list) -> None:
self._data = json.dumps(data).encode("utf-8")
self._info = Mock()
self._info.get_param = Mock(return_value="utf-8")
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+9
to
+11
| assert isinstance(result, list) | ||
| assert len(result) > 0 | ||
| assert result[0]["name"] == "Black Lotus" |
Comment on lines
+199
to
+201
| def bulk_data_by_id__oracle_cards_download(stub_response, load_fixture): | ||
| stub_response("bulk-data/id", load_fixture("bulk_data_by_id__oracle_cards")) | ||
| stub_response("bulk-data/download", _SAMPLE_BULK_CARDS) |
Comment on lines
+9
to
+11
| assert isinstance(result, list) | ||
| assert len(result) > 0 | ||
| assert result[0]["name"] == "Black Lotus" |
Comment on lines
+199
to
+201
| def bulk_data_by_id__oracle_cards_download(stub_response, load_fixture): | ||
| stub_response("bulk-data/id", load_fixture("bulk_data_by_id__oracle_cards")) | ||
| stub_response("bulk-data/download", _SAMPLE_BULK_CARDS) |
Comment on lines
+9
to
+11
| assert isinstance(result, list) | ||
| assert len(result) > 0 | ||
| assert result[0]["name"] == "Black Lotus" |
Comment on lines
+199
to
+201
| def bulk_data_by_id__oracle_cards_download(stub_response, load_fixture): | ||
| stub_response("bulk-data/id", load_fixture("bulk_data_by_id__oracle_cards")) | ||
| stub_response("bulk-data/download", _SAMPLE_BULK_CARDS) |
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.
Closes #207
Opened by Sandcastle. 1 commit(s) on
sandcastle/issue-207.