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

Set the log level when capturing logs in tests. #5418

Merged
merged 2 commits into from Feb 1, 2018

Conversation

Projects
None yet
3 participants
@jsirois
Copy link
Member

jsirois commented Feb 1, 2018

This allows the test writer to ensure their log statements fire when
expected in the face of non-local manipulation of the global python
logging subsystem.

Fixes #5417

Set the log level when capturing logs in tests.
This allows the test writer to ensure their log statements fire when
expected in the face of non-local manipulation of the global python
logging subsystem.

Fixes #5417

@jsirois jsirois requested a review from kwlzn Feb 1, 2018

@kwlzn

kwlzn approved these changes Feb 1, 2018

Copy link
Member

kwlzn left a comment

lgtm, thanks!

import os
import sys
from contextlib import contextmanager

import mock
import psutil

from pants.pantsd import process_manager

This comment has been minimized.

@kwlzn

kwlzn Feb 1, 2018

Member

this new import is unused/redundant, afaict

This comment has been minimized.

@jsirois

jsirois Feb 1, 2018

Member

Ah, yup - thanks.

@@ -483,9 +483,16 @@ def warnings(self):
return self._messages_for_level('WARNING')

@contextmanager
def captured_logging(self):
def captured_logging(self, level=None):

This comment has been minimized.

@kwlzn

kwlzn Feb 1, 2018

Member

could make more sense to default to level=logging.INFO here vs NOTSET below?

This comment has been minimized.

@jsirois

jsirois Feb 1, 2018

Member

I'm always maybe unjustifiably worried about someone then saying captured_logging(level=None) so I ~always defer the conversion. This probably negatively impacts pydoc though.

@jsirois jsirois merged commit 6098667 into pantsbuild:master Feb 1, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jsirois jsirois deleted the jsirois:issues/5417 branch Feb 1, 2018

stuhood added a commit that referenced this pull request Feb 23, 2018

Set the log level when capturing logs in tests. (#5418)
This allows the test writer to ensure their log statements fire when
expected in the face of non-local manipulation of the global python
logging subsystem.

Fixes #5417
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment