FSCollector: use session._node_location_to_relpath #6701
Conversation
blueyed
changed the title
blueyed
changed the title
blueyed
force-pushed
the
package-nodeid-abs-upstream
branch
from
4d1836a
to
ec05d8d
Compare
blueyed
added
topic: collection
blueyed
force-pushed
the
package-nodeid-abs-upstream
branch
from
ec05d8d
to
bcc267a
Compare
This is relevant for `pytest t/foo.py --rootdir=/tmp`. Before: ``` ../../../../tmp F.sxx … t/foo.py:5: ValueError … FAILED ../../../../tmp/::test_fail - ValueError ``` This removes `_check_initialpaths_for_relpath` (added via pytest-dev#2776 to address part of the issue), but it is apparently bad trying to make them relative to any given arg, when they are meant to be relative to `rootdir` really.
blueyed
force-pushed
the
package-nodeid-abs-upstream
branch
from
bcc267a
to
27766e7
Compare
|
Using |
blueyed
force-pushed
the
package-nodeid-abs-upstream
branch
from
afcbbec
to
bcb2db2
Compare
There was a problem hiding this comment.
as far as i can gather,this bugfix implies a breaking change
as the nodeids for everyone using --pyargs are liekly to change,
this has to be part of a breaking release as it affects reporting, reporting metadata, and thus may break subsequent processes at users
| "test_hello.py::test_hello*PASSED*", | ||
| "test_hello.py::test_other*PASSED*", | ||
| "rootdir: {}/world".format(testdir.tmpdir), | ||
| "../hello/ns_pkg/hello/test_hello.py::test_hello PASSED *", |
There was a problem hiding this comment.
this indicates a breaking change
There was a problem hiding this comment.
Node ids are documented to be relative to the rootdir, so this looks like a bugfix to me.
There was a problem hiding this comment.
some bugfixes are breaking changes, based on the required output change here, this is a breaking change
i wll have to crosscheck as im realatively sure that this change would break reporting continuitiy in at least one work project
There was a problem hiding this comment.
ping @RonnyPfannschmidt
|
i verified that this changes thousands of test ids that previously didn't contain certain path elements |
|
I still think this is a bug though. |
|
Have it your way, you made clear what you value |
Maybe related/fixes #3714.
_check_initialpaths_for_relpathwas added in #2776 (14b6380).The adjusted test for it still works, and was hardened inbetween (and fixed here).