Skip to content

Preserve chronological order of stdout and stderr with capsys #6690

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/5449.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Allow ``capsys`` to retrieve combined stdout + stderr streams with original order of messages.
18 changes: 18 additions & 0 deletions doc/en/capture.rst
Original file line number Diff line number Diff line change
Expand Up @@ -170,3 +170,21 @@ as a context manager, disabling capture inside the ``with`` block:
with capsys.disabled():
print("output not captured, going directly to sys.stdout")
print("this output is also captured")


Preserving streams order
------------------------

The ``capsys`` fixture has an additional ``read_combined()`` method. This method returns single value
with both ``stdout`` and ``stderr`` streams combined with preserved chronological order.

.. code-block:: python

def test_combine(capsys):
print("I'm in stdout")
print("I'm in stderr", file=sys.stderr)
print("Hey, stdout again!")

output = capsys.read_combined()
Copy link
Member

Choose a reason for hiding this comment

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

Hmm this just occurred to me, to follow with the naming of the other methods (readouterr), we should stick to readcombined.


assert output == "I'm in stdout\nI'm in stderr\nHey, stdout again!\n"
72 changes: 69 additions & 3 deletions src/_pytest/capture.py
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ def close(self):
self._capture.stop_capturing()
self._capture = None

def readouterr(self):
def readouterr(self) -> "CaptureResult":
"""Read and return the captured output so far, resetting the internal buffer.

:return: captured content as a namedtuple with ``out`` and ``err`` string attributes
Expand All @@ -380,6 +380,17 @@ def readouterr(self):
self._captured_err = self.captureclass.EMPTY_BUFFER
return CaptureResult(captured_out, captured_err)

def read_combined(self) -> str:
"""
Read captured output with both stdout and stderr streams combined, with preserving
the correct order of messages.
"""
if self.captureclass is not OrderedCapture:
raise AttributeError("Only capsys is able to combine streams.")
Copy link
Member

Choose a reason for hiding this comment

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

it is usually a design smell for a superclass to know about a particular subclass, perhaps this method should defer to the captured data's class and the particular subclasses can decide whether they implement this api or not

result = "".join(line[0] for line in OrderedCapture.streams)
OrderedCapture.flush()
return result

def _suspend(self):
"""Suspends this fixture's own capturing temporarily."""
if self._capture is not None:
Expand Down Expand Up @@ -622,13 +633,17 @@ def __init__(self, fd, tmpfile=None):
name = patchsysdict[fd]
self._old = getattr(sys, name)
self.name = name
self.fd = fd
if tmpfile is None:
if name == "stdin":
tmpfile = DontReadFromInput()
else:
tmpfile = CaptureIO()
tmpfile = self._get_writer()
self.tmpfile = tmpfile

def _get_writer(self):
return CaptureIO()

def __repr__(self):
return "<{} {} _old={} _state={!r} tmpfile={!r}>".format(
self.__class__.__name__,
Expand Down Expand Up @@ -695,14 +710,65 @@ def __init__(self, fd, tmpfile=None):
self.tmpfile = tmpfile


class OrderedCapture(SysCapture):
"""Capture class that keeps streams in order."""

streams = collections.deque() # type: collections.deque
Copy link
Member

Choose a reason for hiding this comment

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

seems a bit odd for this to be a class member and not an instance member (as written this makes it effectively a global variable). I'd rather see it as an instance variable

Copy link
Member

Choose a reason for hiding this comment

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

Definitely, but see my comment in in #6690 (comment):

As @bluetech commented, it would probably better if we could get away with the global variables and tight coupling between OrderedCapture and OrderedWriter, but looking at the overall design, I see why it was coded this way: currently MultiCapture always demands 3 captures (stdout, stderr, stdin), so we need a way to communicate/order the writes.

IOW I don't know how to easily fix this, as @aklajnert also commented.

Copy link
Member

Choose a reason for hiding this comment

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

I think all that's needed is to grow another api on the capture classes which represents combined? if that's possible I think that would also fix the problem below of the backward-class-coupling

Copy link
Member

Choose a reason for hiding this comment

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

Hmm but currently who deals with each capture class is MultiCapture, which always creates 3 capture instances (Capture(0), Capture(1), and Capture(2)).

Or do you mean MultiCapture to grow this new API, and forward to the underlying Capture classes?

Copy link
Member

Choose a reason for hiding this comment

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

oh I see :/ maybe nevermind then

Copy link
Member

Choose a reason for hiding this comment

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

Yeah it is a bit frustrating, that's why I think we have to rethink the whole design (in a future PR that is).


def _get_writer(self):
return OrderedWriter(self.fd)

def snap(self):
res = self.tmpfile.getvalue()
if self.name == "stderr":
# both streams are being read one after another, while stderr is last - it will clear the queue
self.flush()
return res

@classmethod
def flush(cls) -> None:
"""Clear streams."""
cls.streams.clear()

@classmethod
def close(cls) -> None:
cls.flush()


map_fixname_class = {
"capfd": FDCapture,
"capfdbinary": FDCaptureBinary,
"capsys": SysCapture,
"capsys": OrderedCapture,
"capsysbinary": SysCaptureBinary,
}


class OrderedWriter:
encoding = sys.getdefaultencoding()

def __init__(self, fd: int) -> None:
super().__init__()
self._fd = fd # type: int
Copy link
Member

Choose a reason for hiding this comment

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

I think mypy can infer this value


def write(self, text: str, **kwargs) -> int:
OrderedCapture.streams.append((text, self._fd))
return len(text)

def getvalue(self) -> str:
return "".join(
line[0] for line in OrderedCapture.streams if line[1] == self._fd
)

def flush(self) -> None:
pass

def isatty(self) -> bool:
return False

def close(self) -> None:
OrderedCapture.close()


class DontReadFromInput:
encoding = None

Expand Down
29 changes: 29 additions & 0 deletions testing/test_capture.py
Original file line number Diff line number Diff line change
Expand Up @@ -1522,3 +1522,32 @@ def test_logging():
)
result.stdout.no_fnmatch_line("*Captured stderr call*")
result.stdout.no_fnmatch_line("*during collection*")


def test_combined_streams(capsys):
"""Show that capsys is capable of preserving chronological order of streams."""
print("stdout1")
print("stdout2")
print("stderr1", file=sys.stderr)
print("stdout3")
print("stderr2", file=sys.stderr)
print("stderr3", file=sys.stderr)
print("stdout4")
print("stdout5")

output = capsys.read_combined()
assert (
output == "stdout1\n"
"stdout2\n"
"stderr1\n"
"stdout3\n"
"stderr2\n"
"stderr3\n"
"stdout4\n"
"stdout5\n"
)


def test_no_capsys_exceptions(capfd):
with pytest.raises(AttributeError, match="Only capsys is able to combine streams."):
capfd.read_combined()