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

TypeError with logging.StreamHandler.__repr__ and pytest's EncodedFile #2555

Closed
blueyed opened this issue Jul 5, 2017 · 3 comments
Closed

TypeError with logging.StreamHandler.__repr__ and pytest's EncodedFile #2555

blueyed opened this issue Jul 5, 2017 · 3 comments

Comments

@blueyed
Copy link
Contributor

@blueyed blueyed commented Jul 5, 2017

Python's logging.StreamHandler has the following __repr__ (https://github.com/python/cpython/blob/0c3116309307ad2c7f8e2d2096612f4ab33cbb62/Lib/logging/__init__.py#L1000-L1005):

def __repr__(self):
    level = getLevelName(self.level)
    name = getattr(self.stream, 'name', '')
    if name:
        name += ' '
    return '<%s %s(%s)>' % (self.__class__.__name__, name, level)

Whereas pytest's EncodedFile.name is a integer (the fd), which results in a TypeError when it is used during tests:

> import logging
> print(logging.root.handlers)
*** TypeError: unsupported operand type(s) for +=: 'int' and 'str'

I think this should get fixed in Python itself, by using str() explicitly, but pytest should provide a proper name for the stream handler?!

The following patch works, but name might be expected to be the fd really in some places?!

diff --git i/_pytest/capture.py w/_pytest/capture.py
index 3661f269..aa7152a5 100644
--- i/_pytest/capture.py
+++ w/_pytest/capture.py
@@ -241,6 +241,7 @@ class EncodedFile(object):
     def __init__(self, buffer, encoding):
         self.buffer = buffer
         self.encoding = encoding
+        self.name = '_pytest.capture.EncodedFile {!r}'.format(buffer)
 
     def write(self, obj):
         if isinstance(obj, unicode):
blueyed added a commit to blueyed/cpython that referenced this issue Jul 5, 2017
This fixes a TypeError in case the `name` is a integer.

Ref: pytest-dev/pytest#2555
@nicoddemus
Copy link
Member

@nicoddemus nicoddemus commented Jul 5, 2017

Hmm good catch! I would say it is a bug even if someone currently expects self.name to be an int given that the file interface expects it to be a str. What do you think @RonnyPfannschmidt?

@RonnyPfannschmidt
Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt commented Jul 5, 2017

api documents it as string, so it should be one

the "bugfix" to cpython is incorrect from my pov

@blueyed
Copy link
Contributor Author

@blueyed blueyed commented Jul 5, 2017

Thanks for your input.

Gave it at shot at #2557.

blueyed added a commit to blueyed/pytest that referenced this issue Jul 5, 2017
blueyed added a commit to blueyed/pytest that referenced this issue Jul 25, 2017
blueyed added a commit to blueyed/pytest that referenced this issue Jul 25, 2017
bluetech added a commit to bluetech/pytest that referenced this issue Mar 7, 2020
I tried to understand what the `safe_text_dupfile()` function and
`EncodedFile` class do. Outside tests, `EncodedFile` is only used by
`safe_text_dupfile`, and `safe_text_dupfile` is only used by
`FDCaptureBinary.__init__()`. I then started to eliminate always-true
conditions based on the single call site, and in the end nothing was
left.

The old code had some workarounds:

- Ensure `errors` property is set: pytest-dev#555.
  It is set now, although now it will return `replace`, which is the
  actual method used also before, instead of `strict`. AFAIU, the issue
  just wanted it to be set to something, not `strict` specifically.

- Infinite recursion trying to pickle `EncodedFile` (f7282b8).
  Not sure, I don't think this object should be pickle-able at all (and
  it isn't...).

- A bug in `logging.Handler.__repr__` with int `file.name`.
  pytest-dev#2555
  Was fixed in Python>=3.7.4: https://bugs.python.org/issue36015.
  TODO: This workaround is probably still needed?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants