fix #12426: adds warning to empty usefixtures. #12439
Conversation
RonnyPfannschmidt
left a comment
RonnyPfannschmidt
left a comment
There was a problem hiding this comment.
Thanks for getting this started
For better reporting of the mark application, the node.warn function should be used to ensure the function/class/module where the mark is used is reported
Additionally there's need for a test and a change log entry as per the contribution documentation
For easier iteration on this change I recommend using a branch different from main
src/_pytest/fixtures.py
Outdated
| """Return the names of usefixtures fixtures applicable to node.""" | ||
| for mark in node.iter_markers(name="usefixtures"): | ||
| if not mark.args: | ||
| warnings.warn(f"Warning: empty usefixtures in {node.name}.") |
There was a problem hiding this comment.
Instead of using a direct warning, inter_marks_with_nodes ought to be used and node.warn should be used to ensure the warning reports the definition site for warning about the marks
There was a problem hiding this comment.
I'm sorry, I don't understand what you mean by inter_marks_with_nodes. I was able to switch to node.warn, but I don't see inter_marks_with_nodes in the documentation. Could you please clarify?
There was a problem hiding this comment.
My bad,the method name was incorrect
https://docs.pytest.org/en/8.1.x/reference/reference.html#pytest.nodes.Node.iter_markers_with_node
There was a problem hiding this comment.
Oh okay, understood. Thanks!
- Updated _getusefixturesnames to iterate with node and marker using node.iter_markers_with_node. - Modified warning message to include the location of the marker node. - Replaced warnings.warn with node.warn to issue warnings.
psf-chronographer
bot
added
the
bot:chronographer:provided
for more information, see https://pre-commit.ci
fixed typo
RonnyPfannschmidt
left a comment
RonnyPfannschmidt
left a comment
There was a problem hiding this comment.
thanks for starting this
| def _getusefixturesnames(self, node: nodes.Item) -> Iterator[str]: | ||
| """Return the names of usefixtures fixtures applicable to node.""" | ||
| for mark in node.iter_markers(name="usefixtures"): | ||
| for marker_node, mark in node.iter_markers_with_node(name="usefixtures"): |
There was a problem hiding this comment.
we need a automated test for this behavior and
src/_pytest/fixtures.py
Outdated
| for marker_node, mark in node.iter_markers_with_node(name="usefixtures"): | ||
| if not mark.args: | ||
| warnings.warn(f"Warning: empty usefixtures in {node.name}.") | ||
| location = getattr(marker_node, "location", "unknown location") |
There was a problem hiding this comment.
as per https://github.com/pytest-dev/pytest/blob/main/src/_pytest/nodes.py#L271 we tell the location of the node when warning, so why not just use marker_node.warn
There was a problem hiding this comment.
I used marker_node.location, but sometimes I got an attribute error: node has no attribute 'location.' I can't seem to replicate the error. Would you prefer marker_node.warn?
There was a problem hiding this comment.
Yes, that Will Auto fill the warning file and line
nicoddemus
left a comment
nicoddemus
left a comment
There was a problem hiding this comment.
Besides @RonnyPfannschmidt comments, left small suggestions.
Co-authored-by: Bruno Oliveira <[email protected]>
Co-authored-by: Bruno Oliveira <[email protected]>
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Closes #12426 Closes #12439 --------- Co-authored-by: Jiajun Xu <[email protected]> Co-authored-by: Bruno Oliveira <[email protected]>
3 participants
closes #12426
added conditional statement to check if usefixtures is empty, raises warning if there is no fixture names.