-
-
Notifications
You must be signed in to change notification settings - Fork 101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor pytests to make pickle testing in-memory #924
base: master
Are you sure you want to change the base?
Changes from all commits
0fbcb9a
e35a2ba
28d6acb
7b9f815
5300c00
ec3739b
3eba40e
5711740
187f1b1
37b0c71
b5c5d06
95b8064
a10412c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -86,3 +86,6 @@ TAGS | |
|
||
# pyenv | ||
.python-version | ||
|
||
# .pkl files in tests folder | ||
tests/*.pickle.* | ||
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -158,6 +158,17 @@ def multidict_getversion_callable(multidict_module: ModuleType) -> Callable: | |
return multidict_module.getversion | ||
|
||
|
||
@pytest.fixture(params=list(range(pickle.HIGHEST_PROTOCOL + 1))) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this work without turning the generator into a list? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If not, please use tuple instead. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's already a |
||
def in_memory_pickle_object( | ||
request: pytest.FixtureRequest, | ||
any_multidict_class: Type[MutableMultiMapping[str]], | ||
) -> bytes: | ||
"""Generate an in-memory pickle of the multi-dict object.""" | ||
proto = request.param | ||
d = any_multidict_class([("a", 1), ("a", 2)]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please, use a reasonable variable name for the data you're storing there. |
||
return pickle.dumps(d, proto) | ||
|
||
|
||
def pytest_addoption( | ||
parser: pytest.Parser, | ||
pluginmanager: pytest.PytestPluginManager, | ||
|
This file was deleted.
webknjaz marked this conversation as resolved.
Show resolved
Hide resolved
|
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,18 +21,12 @@ def test_pickle_proxy(any_multidict_class, any_multidict_proxy_class): | |
pickle.dumps(proxy) | ||
|
||
|
||
def test_load_from_file(any_multidict_class, multidict_implementation, pickle_protocol): | ||
multidict_class_name = any_multidict_class.__name__ | ||
pickle_file_basename = "-".join( | ||
( | ||
multidict_class_name.lower(), | ||
multidict_implementation.tag, | ||
) | ||
) | ||
def test_load_from_file( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test might need renaming since it no longer uses a file on disk. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OTOH, upon further reflection, I think I might know why the actual files are committed in Git — this is to ensure that newer multidict versions can load pickles of the older ones. We might need to have a CLI interface for making the files in pytest but keep them. A fixture w/o an in-memory strucutre would be useful, though. |
||
any_multidict_class, | ||
multidict_implementation, | ||
in_memory_pickle_object, | ||
): | ||
d = any_multidict_class([("a", 1), ("a", 2)]) | ||
fname = f"{pickle_file_basename}.pickle.{pickle_protocol}" | ||
p = here / fname | ||
with p.open("rb") as f: | ||
obj = pickle.load(f) | ||
obj = pickle.loads(in_memory_pickle_object) | ||
assert d == obj | ||
assert isinstance(obj, any_multidict_class) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you move this into
tests/.gitignore
?Also, having text files without a trailing LF is a bad tone.