Skip to content

Issue 7076 - Fix relative paths for nodes - #7095

Closed
CarycaKatarzyna wants to merge 1 commit into
pytest-dev:masterfrom
CarycaKatarzyna:issue7076
Closed

Issue 7076 - Fix relative paths for nodes#7095
CarycaKatarzyna wants to merge 1 commit into
pytest-dev:masterfrom
CarycaKatarzyna:issue7076

Conversation

@CarycaKatarzyna

Copy link
Copy Markdown
Contributor

It solves #7076. I decided to change rootdir to invocation dir in _bestrelpathcache, because we want to have path for nodes relative to invocation directory. As I've seen, that function is used only for nodes (_node_location_to_relpath), so this change shouldn't mess other functionalities.

@CarycaKatarzyna CarycaKatarzyna changed the title Change rootdir to invocation dir in realtive paths for nodes Issue 7076 - Fix relavite paths for nodes Apr 15, 2020
@CarycaKatarzyna CarycaKatarzyna changed the title Issue 7076 - Fix relavite paths for nodes Issue 7076 - Fix relative paths for nodes Apr 15, 2020
@nicoddemus

nicoddemus commented May 2, 2020

Copy link
Copy Markdown
Member

Thanks @CarycaKatarzyna!

because we want to have path for nodes relative to invocation directory

I'm not sure that is correct: the rootdir exists specifically so tests have always reliable nodeids, regardless where pytest was invoked from. The nodeid is after all used by plugins to uniquely identify tests, for example --lf, which persists the node ids to disk; having them change depending on where pytest was invoked from would defeat that.

@RonnyPfannschmidt can you please correct me if I'm wrong?

@RonnyPfannschmidt

Copy link
Copy Markdown
Member

this absolutely has to use rootdir, we cant use invocationdir as things are now

@nicoddemus

Copy link
Copy Markdown
Member

Right thanks.

@CarycaKatarzyna thanks again for the PR, but we will need to decline this as is. Feel free to open another PR if you come up with a different approach! 👍

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.

3 participants