-
Notifications
You must be signed in to change notification settings - Fork 457
fix[pytest]: Default parameter JSON encoding to use repr #2684
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
Conversation
…ntation of parameters
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.
Nice - much more robust now imo! In hindsight this should have been the first approach we went with... 🙄 😆
|
||
class A: | ||
def __repr__(self): | ||
raise Exception("Cannot __repr__") |
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.
👍
params = {"arguments": item.callspec.params, "metadata": {}} | ||
span.set_tag(test.PARAMETERS, json.dumps(params, default=repr)) | ||
parameters = {"arguments": {}, "metadata": {}} # type: Dict[str, Dict[str, str]] | ||
for param_name, param_val in item.callspec.params.items(): |
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.
item.callspec.params
is always a Dict[str, Any]
?
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.
if so, then all good, if not, then we are probably missing some test cases
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.
Yes! item.callspec.params
is a dictionary that contains all parameter objects. You can see here in the pytest docs:
https://docs.pytest.org/en/6.2.x/reference.html?highlight=callspec#function
If given, this is function has been parametrized and the callspec contains meta information about the parametrization.
@Mergifyio backport 0.50 |
* Remove JSON encoding for pytest parameters, default to string representation of parameters * Added type hinting (cherry picked from commit f5bc986)
Command
|
) (#2691) * fix[pytest]: Default parameter JSON encoding to use repr (#2684) * Remove JSON encoding for pytest parameters, default to string representation of parameters * Added type hinting (cherry picked from commit f5bc986) * Added logger import Co-authored-by: Yun Kim <[email protected]> Co-authored-by: Yun Kim <[email protected]>
Description
A bug was caught with the pytest plugin where parameterized tests with dictionary arguments containing tuple keys raised a TypeError and failed to encode. This is due to the JSON library not being able to encode non-basic (str, int, float, bool, None) key types. To allow the tracer to handle arbitrary key types, the pytest plugin now defaults to encoding the string
__repr__
representation of each parameter argument.