-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Function: use originalname
in _getobj
and make it default to name
#7035
Changes from 2 commits
d4b5990
c145437
3e44a10
faa50ca
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 |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
import pytest | ||
from _pytest.config import ExitCode | ||
from _pytest.nodes import Collector | ||
from _pytest.pytester import Testdir | ||
|
||
|
||
class TestModule: | ||
|
@@ -659,16 +660,39 @@ def test_passed(x): | |
result = testdir.runpytest() | ||
result.stdout.fnmatch_lines(["* 3 passed in *"]) | ||
|
||
def test_function_original_name(self, testdir): | ||
def test_function_originalname(self, testdir: Testdir) -> None: | ||
items = testdir.getitems( | ||
""" | ||
import pytest | ||
|
||
@pytest.mark.parametrize('arg', [1,2]) | ||
def test_func(arg): | ||
pass | ||
|
||
def test_no_param(): | ||
pass | ||
""" | ||
) | ||
assert [x.originalname for x in items] == ["test_func", "test_func"] | ||
assert [x.originalname for x in items] == [ | ||
"test_func", | ||
"test_func", | ||
"test_no_param", | ||
] | ||
|
||
def test_function_with_square_brackets(self, testdir: Testdir) -> None: | ||
"""Check that Function._getobj uses originalname.""" | ||
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. Nitpick: I think this description is too low-level, I would go for something more like "Check that functions with square brackets don't cause trouble". 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. That's in the test name already. 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. I would leave it out then -- it might just become outdated if the mechanism changes, but the test itself will stay relevant always because it tests high-level behavior. Feel free to disregard though if you prefer to keep it :) |
||
p1 = testdir.makepyfile( | ||
""" | ||
locals()["test_foo[name]"] = lambda: None | ||
""" | ||
) | ||
result = testdir.runpytest("-v", str(p1)) | ||
result.stdout.fnmatch_lines( | ||
[ | ||
"test_function_with_square_brackets.py::test_foo[[]name[]] PASSED *", | ||
bluetech marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"*= 1 passed in *", | ||
] | ||
) | ||
|
||
|
||
class TestSorting: | ||
|
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.
Given a field called "originalname", I'd expect it to only contain the original name, but this adds a fallback to
name
, which is presumably not the original name? If so, then this seems unadvisable. Maybe do thisif
in_getobj
instead?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.
Ideally this would be a alias for functiondefinition.name
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.
@bluetech the point is that it should be usable as-is, i.e. also for non-parametrized functions (where
name
is the original name AFAICT).Check 1a79137: it only passes it in for parametrized functions.
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.
(Replying only to the diff, not Ronny's comment which I didn't look into)
Would it be possible to fix that? I.e. make
originalname
be passed always, even when it's the same asname
? That would address my concern and makes some sense anyway I think...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.
Done it. 👍
I tried @RonnyPfannschmidt's suggestion, which I think is more elegant, but the problem is that
_getobj
usesoriginalname
to obtain the function, andoriginalname
requires the function, so we get a recursion.