pytest-dev / pytest

The pytest framework makes it easy to write small tests, yet scales to support complex functional testing
https://pytest.org
MIT License
11.91k stars 2.65k forks source link

Captured IO accepts bytes even tough it should not #4861

Closed mbra closed 5 years ago

mbra commented 5 years ago

On Python3, when output capturing is enabled, bytes are accepted where normaly only text would we accepted.

I am on Ubuntu 18.04 and using it's Python 3.6 packages with pytest 4.3.0.

Code:

# coding: utf-8

import sys as _sys

def test_output():
    _sys.stdout.write(b"foo")

With capturing:

=============================== test session starts ===============================
platform linux -- Python 3.6.6, pytest-4.3.0, py-1.6.0, pluggy-0.7.1
rootdir: /tmp/tmp.t1x157Dqyo, inifile:
plugins: localserver-0.5.0, httpbin-0.3.0, cov-2.6.0
collected 1 item                                                                  

test.py .                                                                   [100%]

============================ 1 passed in 0.00 seconds =============================

Without capturing:

=============================== test session starts ===============================
platform linux -- Python 3.6.6, pytest-4.3.0, py-1.6.0, pluggy-0.7.1
rootdir: /tmp/tmp.t1x157Dqyo, inifile:
plugins: localserver-0.5.0, httpbin-0.3.0, cov-2.6.0
collected 1 item                                                                  

test.py F

==================================== FAILURES =====================================
___________________________________ test_output ___________________________________

    def test_output():
>       _sys.stdout.write(b"foo")
E       TypeError: write() argument must be str, not bytes

test.py:7: TypeError
============================ 1 failed in 0.02 seconds =============================
blueyed commented 5 years ago

Confirmed. To be clear: also the first run should fail, right? I have also tested with --assert=plain, which did not change it.

blueyed commented 5 years ago

I've extended the test a bit:

def test_output():
    assert type(b"foo") == bytes
    print(_sys.stdout)
    print(_sys.stdout.write)
    print(id(_sys.stdout.write))
    _sys.stdout.write(b"foo")

With output capturing:

>       assert 0
E       assert 0

test_t.py:14: AssertionError
------------------------------------------------------------------------------------ Captured stdout call ------------------------------------------------------------------------------------
<_pytest.capture.EncodedFile object at 0x7f39cf8530b8>
<bound method EncodedFile.write of <_pytest.capture.EncodedFile object at 0x7f39cf8530b8>>
139886271186184
foo

Without:

test_t.py <_io.TextIOWrapper name='<stdout>' mode='w' encoding='UTF-8'>
<built-in method write of _io.TextIOWrapper object at 0x7fac03919630>
140376731642472
F

Looks like pytest's write should maybe just validate the type also? https://github.com/pytest-dev/pytest/blob/df8869cf1a3a0e2a2c5a1c7ce7a6b5e1793c7997/src/_pytest/capture.py#L412-L415

blueyed commented 5 years ago

cpython itself appears to check it here: https://github.com/python/cpython/blob/ef17fdbc1c274dc84c2f611c40449ab84824607e/Modules/_io/clinic/textio.c.h#L253-L256

mbra commented 5 years ago

Yes, sorry, the tests should both fail on Python 3, should have mentioned that.

As for handling the type, yes that should probably be changed to mimc Python 3 behaviour, but as usual it also still needs to mimic Python 2, which is to accept both...which you have already done in your commit...that just popped up while writing this. :heart_eyes: