pytest-isort icon indicating copy to clipboard operation
pytest-isort copied to clipboard

"ERROR: <filename> Imports are incorrectly sorted" has disappeared

Open fenekku opened this issue 1 year ago • 1 comments

Hi! I noticed that the

ERROR: <filename> Imports are incorrectly sorted.

"header" before an isort diff is shown has disappeared in pytest-isort output i.e. the filename associated with an isort-check error is not shown which makes it hard to debug/associate which diff goes with which file. For example, the output I am getting is:

> pytest --isort

<skipping irrelevant output>

=============== FAILURES ===============
_______________ isort-check ________________
<Missing Error: ... header would have been here but it is absent>
+from werkzeug import __version__ as werkzeug_version
 from werkzeug.datastructures import Headers
-from werkzeug import __version__ as werkzeug_version
=============== short test summary info ================
FAILED tests/mytest.py::ISORT

With multiple failing tests, the "short test summary info" section isn't enough to reconnect the diffs with the files they apply to.

This seems to be because _pytest.capture.MultiCapture splits the out/err streams and pytest-isort only uses the stdout one (https://github.com/stephrdev/pytest-isort/blob/master/pytest_isort/init.py#L229), but the "Error: <filename> Imports are incorrectly sorted." is output on the err stream by isort.

However, it also seems like capturing both streams chronologically is not a solved problem on the pytest.capture side : https://github.com/pytest-dev/pytest/issues/5449 ... but that's an old discussion and maybe you know of a better alternative :smiley: ? (Popen can capture them correctly, so it should be possible somehow).

Thanks so much for taking a look!

fenekku avatar Jan 05 '24 21:01 fenekku

Thank you for your report. As you stated correctly, output capturing is complicated and sadly, isort uses print to stderr directly with no option to configure it. As we call the isort methods directly, Popen won't help here.

From what I see, hijacking stderr before calling the check_file method (https://github.com/stephrdev/pytest-isort/blob/master/pytest_isort/init.py#L224) is the only likely working method (similar to https://github.com/pytest-dev/pytest/issues/5449#issuecomment-955103558) but I don't have to for that right now.

If you come up with a PR to fix this, I'd be more than happy to review and merge swiftly.

stephrdev avatar Jan 09 '24 08:01 stephrdev