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

caplog.clear() removes records but leaves collected .text #3297

Closed
ankostis opened this issue Mar 12, 2018 · 8 comments
Closed

caplog.clear() removes records but leaves collected .text #3297

ankostis opened this issue Mar 12, 2018 · 8 comments
Labels
plugin: logging related to the logging builtin plugin type: bug problem that needs to be addressed

Comments

@ankostis
Copy link
Contributor

Problem

Currently (see pytest versions below), method LogCaptureHandler.clear() removes all records from the handler, but it does not clear the collected text in it's .stream, leading to contradictory results:

pytest/_pytest/logging.py

Lines 240 to 244 in 07e768a

def clear(self):
"""Reset the list of log records."""
self.handler.records = []

pytest/_pytest/logging.py

Lines 220 to 224 in 07e768a

@property
def text(self):
"""Returns the log text."""
return self.handler.stream.getvalue()

Example

import logging

def test_caplog_clear(caplog):
    rlog = logging.getLogger()

    rlog.error("Hi there!")
    assert "Hi there!" in caplog.text

    caplog.clear()
    assert all("Hi there!" not in r for r in caplog.records)
    assert "Hi there!" not in caplog.text  # Fails!

Suggestion

One can either add this line into .clear():

self.handler.stream = py.io.TextIO()

OR add this one, which will work also for clients holding handler's stream property:

self.handler.stream.truncate(0)

Versions

pip list |grep pytest
pytest (3.4.2)
pytest-cov (2.5.1)
pytest-cover (3.0.0)
pytest-coverage (0.0)
pytest-mock (1.7.1)
pytest-runner (4.0)
@pytestbot pytestbot added the plugin: logging related to the logging builtin plugin label Mar 12, 2018
@pytestbot
Copy link
Contributor

GitMate.io thinks possibly related issues are #3037 (caplog fixture: capture log records from another process), #891 (remove named marker attributes and collect markers in items), #3080 (Remove pytest_namespace), #3083 (Remove metafunc.addcall), and #3079 (Remove yield tests).

@pytestbot pytestbot added the type: bug problem that needs to be addressed label Mar 12, 2018
@twmr twmr self-assigned this Mar 12, 2018
@nicoddemus
Copy link
Member

nicoddemus commented Mar 12, 2018

OR add this one, which will work also for clients holding handler's stream property:

This seems like a nice solution (@Thisch what do you think?). @ankostis you have done some researching here, would you like to work on a PR? Sorry nevermind, only now I noticed that @Thisch has already self-assigned this issue. 😉

@twmr
Copy link
Contributor

twmr commented Mar 12, 2018

I'm for reinitializing the stream, hence not using truncate. See https://stackoverflow.com/questions/4330812/how-do-i-clear-a-stringio-object#4330829

@ankostis If you want to work on this I can unassign myself. (I'll probably not have time to work on it soon)

twmr added a commit to twmr/pytest that referenced this issue Mar 12, 2018
@ankostis
Copy link
Contributor Author

I'll give it a try.

twmr added a commit to twmr/pytest that referenced this issue Mar 12, 2018
@twmr
Copy link
Contributor

twmr commented Mar 12, 2018

You can take a look at my commit (62b224b)

@twmr twmr removed their assignment Mar 12, 2018
ankostis added a commit to ankostis/pytest that referenced this issue Mar 12, 2018
ankostis pushed a commit to ankostis/pytest that referenced this issue Mar 12, 2018
@ankostis
Copy link
Contributor Author

Done

@ankostis
Copy link
Contributor Author

Oups, i see now my changelog change is forbiden. Rewritting history without it.

Use this if useful:

- Fix ``clear()`` method on ``caplog`` fixture which cleared only the ``records``,
  and not the ``text```property. (`#3297
  <https://github.com/pytest-dev/pytest/issues/3297>`_)

@nicoddemus
Copy link
Member

nicoddemus commented Mar 12, 2018

Oups, i see now my changelog change is forbiden

Hmm it seems your markup around text has an extra backtick and failed to parse. Also don't need to add the link to #3297 because this is added automatically.

I suggested a changelog entry on my review because I did not see your comment here, so feel free to use your initial take instead of mine:

Fixed ``clear()`` method on ``caplog`` fixture which cleared only the ``records`` attribute but not the ``text`` property.

RonnyPfannschmidt added a commit that referenced this issue Mar 13, 2018
Fix #3297 where caplog.clear() did not clear text, just records
KKoukiou pushed a commit to KKoukiou/pytest that referenced this issue Mar 14, 2018
KKoukiou pushed a commit to KKoukiou/pytest that referenced this issue Mar 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin: logging related to the logging builtin plugin type: bug problem that needs to be addressed
Projects
None yet
Development

No branches or pull requests

4 participants