Skip to content
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

prioritize outputting relative paths #12974

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dongfangtianyu
Copy link
Contributor

  • [ ] Include documentation when adding new features.
  • Include new tests or update existing tests when applicable.
  • Allow maintainers to push and squash when merging my commits.
  • Add text like closes #XYZW to the PR description and/or commits.
  • Create a new changelog file in the changelog folder

Implementation of #12973

@psf-chronographer psf-chronographer bot added the bot:chronographer:provided (automation) changelog entry is part of PR label Nov 18, 2024
@nicoddemus
Copy link
Member

Thanks @dongfangtianyu for the PR.

I'm a bit worried about this change, as I expect this will break systems which expects those absolute paths to be there, such as IDEs which will often highlight those paths as links.

@pytest-dev/core what are your opinions?

@RonnyPfannschmidt
Copy link
Member

i think we should consider this a breaking change as path construction changes

perhaps a option to configure it might help and we switch it from absolute to cwd_relative with the next major release

i really like the potentially better ux

i do fear missed problems caused by output consuming software

@RonnyPfannschmidt
Copy link
Member

perhaps we want a concept of a "display path that collapss details

it would render cwd relative paths to - but also perhaps ${SITE}/somepacke/broken.py instead of the exact folder to the current python site

this could also be expanded with $TMP_PATH and some more

@dongfangtianyu
Copy link
Contributor Author

IDEs which will often highlight those paths as links.

Hi @nicoddemus ,Your concern is valid.

A brief test was conducted on PyCharm (2024.1.1) and VS Code (1.93.1):
If the path is correct and contains line numbers, both absolute and relative paths can be recognized as links. (Not sure if all IDEs will do this)

image
image

@dongfangtianyu
Copy link
Contributor Author

This is a classic pytest output, with most of its paths being relative paths:

================================================ test session starts =================================================
platform win32 -- Python 3.12.2, pytest-8.3.3, pluggy-1.5.0
rootdir: D:\git\github\dongfangtianyu\tmp_py312
configfile: pytest.ini
collected 1 item                                                                                                      

a\b\c\test_demo.py F                                                                                            [100%]

====================================================== FAILURES ====================================================== 
_____________________________________________________ test_demo ______________________________________________________ 

    def test_demo():
>       funcs.my_func()

a\b\c\test_demo.py:4:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _  

    def my_func():

>       1/0
E       ZeroDivisionError: division by zero

a\b\c\funcs.py:5: ZeroDivisionError
============================================== short test summary info =============================================== 
FAILED a/b/c/test_demo.py::test_demo - ZeroDivisionError: division by zero
================================================= 1 failed in 0.17s ================================================== 

This is the output for some scenarios (FixtureLookupError, skip, warnings): the absolute path (warnings summary) is output

================================================ test session starts =================================================
platform win32 -- Python 3.12.2, pytest-8.3.3, pluggy-1.5.0
rootdir: D:\git\github\dongfangtianyu\tmp_py312
configfile: pytest.ini
collected 1 item                                                                                                      

a\b\c\test_demo.py F                                                                                            [100%]


====================================================== FAILURES ====================================================== 
_____________________________________________________ test_demo ______________________________________________________ 

    def test_demo():
        import warnings
        warnings.warn('yyy', UserWarning)
>       funcs.my_func()

a\b\c\test_demo.py:8:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _  
a\b\c\funcs.py:4: in my_func
    json.loads("xxxxx")
C:\Users\administrator\AppData\Local\Programs\Python\Python312\Lib\json\__init__.py:348: in loads
    return _default_decoder.decode(s)
C:\Users\administrator\AppData\Local\Programs\Python\Python312\Lib\json\decoder.py:337: in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _  

self = <json.decoder.JSONDecoder object at 0x000002661F54C920>, s = 'xxxxx', idx = 0

    def raw_decode(self, s, idx=0):
        """Decode a JSON document from ``s`` (a ``str`` beginning with
        a JSON document) and return a 2-tuple of the Python
        representation and the index in ``s`` where the document ended.

        This can be used to decode a JSON document from a string that may
        have extraneous data at the end.

        """
        try:
            obj, end = self.scan_once(s, idx)
        except StopIteration as err:
>           raise JSONDecodeError("Expecting value", s, err.value) from None
E           json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

C:\Users\administrator\AppData\Local\Programs\Python\Python312\Lib\json\decoder.py:355: JSONDecodeError
================================================== warnings summary ================================================== 
a/b/c/test_demo.py::test_demo
  D:\git\github\dongfangtianyu\tmp_py312\a\b\c\test_demo.py:7: UserWarning: yyy
    warnings.warn('yyy', UserWarning)

a/b/c/test_demo.py::test_demo
C:\Users\administrator\AppData\Local\Programs\Python\Python312\Lib\json\__init__.py:334
  C:\Users\administrator\AppData\Local\Programs\Python\Python312\Lib\json\__init__.py:334: UserWarning: xxx
    warnings.warn('xxx', UserWarning)

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
============================================== short test summary info =============================================== 
FAILED a/b/c/test_demo.py::test_demo - json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)
=========================================== 1 failed, 3 warnings in 0.13s ============================================

This is the output of this PR: the relative paths of the files within the project are outputted, while the libraries and traceback maintain their original outputs

================================================ test session starts =================================================
platform win32 -- Python 3.12.2, pytest-8.3.3, pluggy-1.5.0
rootdir: D:\git\github\dongfangtianyu\tmp_py312
configfile: pytest.ini
collected 1 item                                                                                                      

a\b\c\test_demo.py F                                                                                            [100%]


====================================================== FAILURES ====================================================== 
_____________________________________________________ test_demo ______________________________________________________ 

    def test_demo():
        import warnings
        warnings.warn('yyy', UserWarning)
>       funcs.my_func()

a\b\c\test_demo.py:8:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _  
a\b\c\funcs.py:4: in my_func
    json.loads("xxxxx")
C:\Users\administrator\AppData\Local\Programs\Python\Python312\Lib\json\__init__.py:348: in loads
    return _default_decoder.decode(s)
C:\Users\administrator\AppData\Local\Programs\Python\Python312\Lib\json\decoder.py:337: in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _  

self = <json.decoder.JSONDecoder object at 0x000001CF1412C920>, s = 'xxxxx', idx = 0

    def raw_decode(self, s, idx=0):
        """Decode a JSON document from ``s`` (a ``str`` beginning with
        a JSON document) and return a 2-tuple of the Python
        representation and the index in ``s`` where the document ended.

        This can be used to decode a JSON document from a string that may
        have extraneous data at the end.

        """
        try:
            obj, end = self.scan_once(s, idx)
        except StopIteration as err:
>           raise JSONDecodeError("Expecting value", s, err.value) from None
E           json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

C:\Users\administrator\AppData\Local\Programs\Python\Python312\Lib\json\decoder.py:355: JSONDecodeError
================================================== warnings summary ================================================== 
a/b/c/test_demo.py::test_demo
  a\b\c\test_demo.py:7: UserWarning: yyy
    warnings.warn('yyy', UserWarning)

a/b/c/test_demo.py::test_demo
C:\Users\administrator\AppData\Local\Programs\Python\Python312\Lib\json\__init__.py:334
  C:\Users\administrator\AppData\Local\Programs\Python\Python312\Lib\json\__init__.py:334: UserWarning: xxx
    warnings.warn('xxx', UserWarning)

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
============================================== short test summary info =============================================== 
FAILED a/b/c/test_demo.py::test_demo - json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)
=========================================== 1 failed, 3 warnings in 0.12s ============================================ 


@nicoddemus
Copy link
Member

@dongfangtianyu did you post the correct outputs? I only see a single difference in the paths:

28c28
< self = <json.decoder.JSONDecoder object at 0x000002661F54C920>, s = 'xxxxx', idx = 0
---
> self = <json.decoder.JSONDecoder object at 0x000001CF1412C920>, s = 'xxxxx', idx = 0
48c48
<   D:\git\github\dongfangtianyu\tmp_py312\a\b\c\test_demo.py:7: UserWarning: yyy
---
>   a\b\c\test_demo.py:7: UserWarning: yyy
59c59,60
< =========================================== 1 failed, 3 warnings in 0.13s ============================================
---
> =========================================== 1 failed, 3 warnings in 0.12s ============================================

>

@dongfangtianyu
Copy link
Contributor Author

@nicoddemus Yes. These contents are copied from the actual output.

In this example, the PR converts D:\git\github\dongfangtianyu\tmp_py312\a\b\c\test_demo.py to a relative path because the file is part of the project and output by pytest:

The traceback content is not processed for several reasons:

  1. Most standard libraries and third-party libraries in venv have paths that differ from cwd, and converting them to relative paths doesn't work well.
  2. In complex call stacks, traceback content can be large, and processing it would be resource-intensive.
  3. Many file paths in exception messages are strings, making them harder to process.

@nicoddemus
Copy link
Member

The traceback content is not processed for several reasons

Oh no I agree we should not touch the traceback content, I was just making sure the post was correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:chronographer:provided (automation) changelog entry is part of PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants