fix: Handle dotted parameters in classname #200
Conversation
|
|
||
| @pytest.mark.parametrize("parameter_1", ("foo",)) | ||
| @pytest.mark.parametrize("parameter_2", ("bar",)) | ||
| def test_doubly_parametrized(parameter_1, parameter_2, snapshot): |
There was a problem hiding this comment.
I was about to implement something that just split on [, on closer inspection, it seemed better to traverse the node's parents.
| return ( | ||
| ".".join( | ||
| node.name for node in self._node.listchain() if isinstance(node, Class) | ||
| ) | ||
| or None | ||
| ) |
There was a problem hiding this comment.
Would prefer we fixed the __valid_ids method so both __parse and classname are using the same logic
There was a problem hiding this comment.
I suspect classname doesn't need that logic – unlike with the others, in this context, we know that we have some set of valid Python identifiers anyway. It seems like it might, then, be slightly better to actually look for classes in the node hierarchy, as per this implementation?
There was a problem hiding this comment.
Hmm missed this yesterday, it's an easy enough refactor to revert. @noahnu what do you think?
There was a problem hiding this comment.
Right, I mean, that's what this implementation does – listchain shows all the current "nodes" in the test chain (modules, classes, &c.), so I'm going off that directly.
The alternative is just to split on [ and ignoring the part after that in __valid_ids before splitting on ., which gets you to the same place.
## [0.4.2](v0.4.1...v0.4.2) (2020-04-22) ### Bug Fixes * Handle dotted parameters in classname ([#200](#200)) ([d961f7c](d961f7c))
|
🎉 This PR is included in version 0.4.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Description
This fixes a regression from #197 for tests with parameters with dots in them.
Related Issues
#197, sorta
Checklist
Additional Comments
Without this change, the dotted parameter name would confuse the earlier iteration of the
classnamelogic.