Skip to content

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented Feb 2, 2020

Everything was using _pytest.compat.getfslineno basically, which
wrapped _pytest._code.source.getfslineno.

This moves the extra code from there into it directly, and uses the
latter everywhere.

This helps to eventually remove the one in compat eventually, and also
causes less cyclic imports.



def getfslineno(obj) -> Tuple[Optional[Union["Literal['']", py.path.local]], int]:
def getfslineno(obj: Any) -> Tuple[Union[str, py.path.local], int]:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bluetech had to take Any here instead of obj, for the place_as attribute check.

return "", -1

fspath = fn and py.path.local(fn) or None
fspath = fn and py.path.local(fn) or ""
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cannot really happen (becoming "", only if co_filename is empty on a code object, but that gets handled by Code then, which I've fixed to keep it (instead of turning it into cwd).
But I guess the or "" does not hurt - at least I'm confident that it never returned None actually.

Everything was using `_pytest.compat.getfslineno` basically, which
wrapped `_pytest._code.source.getfslineno`.

This moves the extra code from there into it directly, and uses the
latter everywhere.

This helps to eventually remove the one in compat eventually, and also
causes less cyclic imports.
Previously this would be turned via `py.path.local("")` into the current
working directory.

This appears to be what `fspath = fn and py.path.local(fn) or None`
tries to avoid in `getfslineno`'s `TypeError` handling already, if
`Code` would raise it.
@blueyed blueyed force-pushed the rm-compat-getfslineno branch from fcbb6f0 to dab90ef Compare February 3, 2020 18:09
Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cleanup of getfslineno looks good to me, please consider just removing it from compat altogether now if possible.

The typing changes I did not review because I trust your judgement/knowledge in that area more than mine. 😁

@blueyed blueyed requested a review from bluetech February 4, 2020 01:58
@blueyed blueyed merged commit 632800a into pytest-dev:master Feb 4, 2020
@blueyed blueyed deleted the rm-compat-getfslineno branch February 4, 2020 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants