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.

Sign up for GitHub

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

Jump to bottom

Live logs (-s / --capture=no) #25

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Live logs (-s / --capture=no) #25

wants to merge 5 commits into from

Conversation

abusalimov
Copy link
Collaborator

Partially resolves #14 ("Integrate with --capture and -v options of pytest"), in a slightly more natural and efficient way than proposed in #20 ("Move pytest-logging functionality to pytest-catchlog.")

Note: this includes changes proposed in PR #24, DO NOT MERGE until #24 is accepted. Also this lacks a proper description in the README.

TODO

  • README
  • record.message not initialized, add tests

@abusalimov
Separate capturing/reporting and 'caplog' recording/inspection features
05fa345 
This makes a distinction between two features / use cases:

  1. Inspection: record and check logs emitted by the code being tested
     using the `caplog` fixture

  2. Reporting: show all logs in case of a failure

Implemented through adding a dedicated log handler for the recording.
As a bonus, simplifies the capturing logic (no more awkward
item.catch_log_handler).
@abusalimov
Replace CatchLogHandler with RecordingHandler that doesn't use streams
a52a26b 
We only need formatted text (hence the stream) for log reporting and
for the caplog.text() method. On the other hand, we don't need a list
of records for the reporting facility.

So, we only leave storing of records in the RecordingHandler class, and
use ad hoc solutions using logging.StreamHandler where needed.
@abusalimov
(py26) Force RecordingHandler to be a new-style class
29db8e6 
Turns out, logging.Handler is an old-style class in Python 2.6, so that
in order to use super() we need to make it's subclass a new-style class
(through extending 'object' explicitly).
@abusalimov
Setup one master handler and reuse it across all test calls
4121487 
This saves us from unnecessary interaction with the logging subsystem
(creating a new handler, configuring it, attaching and removing it
afterwards) and opens the way for lots of future optimizations and
improvements.
@abusalimov
Add live logs support, enabled by -s or --capture=no
470909c 
References:
  * eisensheng#14 ("Integrate with --capture and -v options of pytest")
  * eisensheng#20 ("Move pytest-logging functionality to pytest-catchlog.")
@abusalimov abusalimov mentioned this pull request Nov 21, 2015
Merged
@The-Compiler
Copy link
Collaborator

It seems this breaks using LogRecord.message:

    def test_abort_typeerror(question, qtbot, mocker, caplog):
        """Test Question.abort() with .emit() raising a TypeError."""
        signal_mock = mocker.patch('qutebrowser.utils.usertypes.Question.aborted')
        signal_mock.emit.side_effect = TypeError
        with caplog.at_level(logging.ERROR, 'misc'):
            question.abort()
>       assert caplog.records[0].message == 'Error while aborting question'
E       AttributeError: 'LogRecord' object has no attribute 'message'

It seems to work when I use the caplog-rec-handler branch from #24.

The-Compiler
The-Compiler reviewed Nov 24, 2015
View reviewed changes
pytest_catchlog.py
@@ -268,6 +269,11 @@ class CallableStr(CallablePropertyMixin, str):
class CompatLogCaptureFixture(LogCaptureFixture):
"""Backward compatibility with pytest-capturelog."""

def __init__(self, handler, item):
"""Creates a new funcarg."""
Copy link
Collaborator

Choose a reason for hiding this comment

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

funcarg -> fixture 😉

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👍

@abusalimov
Copy link
Collaborator Author

It seems this breaks using LogRecord.message

Interesting enough. LogRecord.message is initialized by Formatter.format(), i.e. lazily. It is probably broken since replacing StreamHandler with simple RecordingHandler, which doesn't use formatting at all.

I'm not sure whether it is right to rely on that some Handler calls format(), but I guess, it would be better to add a workaround for this in pytest-catchlog.

I'll add a regression test for this, thank you! 👍

@The-Compiler The-Compiler mentioned this pull request Nov 24, 2015
Merged
@abusalimov abusalimov mentioned this pull request Dec 10, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
⚠ DO NOT MERGE
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Integrate with --capture and -v options of pytest
2 participants
@abusalimov @The-Compiler