Skip to content

Commit

Permalink
Fix a bug in task logging in tests. (#5404)
Browse files Browse the repository at this point in the history
Our context.log method accepts structured messages (e.g., to
provide extra info in tooltips in an HTML report). Our regular
console reporter flattens these out.

However our TestContext class used a regular python logger,
and this misinterpreted our structured messages as format
args, leading to string formatting errors that obscured actual
test errors.

This change provides a thin wrapper around the regular logger
that flattens out the structured messages, without bringing in
our heavy reporting machinery.

It also removes a test helper function that was unused and
horribly broken.
  • Loading branch information
benjyw committed Jan 28, 2018
1 parent 74b324a commit ef15589
Showing 1 changed file with 23 additions and 26 deletions.
49 changes: 23 additions & 26 deletions tests/python/pants_test/base/context_utils.py
Expand Up @@ -14,7 +14,6 @@
from pants.base.workunit import WorkUnit
from pants.build_graph.target import Target
from pants.goal.context import Context
from pants_test.option.util.fakes import create_options


class TestContext(Context):
Expand Down Expand Up @@ -57,6 +56,28 @@ def add_misses(self, cache_name, targets, causes): pass

def report_target_info(self, scope, target, keys, val): pass


class TestLogger(logging.getLoggerClass()):
"""A logger that converts our structured records into flat ones.
This is so we can use a regular logger in tests instead of our reporting machinery.
"""

def makeRecord(self, name, lvl, fn, lno, msg, args, exc_info, func=None, extra=None):
msg = ''.join([msg] + [a[0] if isinstance(a, (list, tuple)) else a for a in args])
args = []
return super(TestContext.TestLogger, self).makeRecord(
name, lvl, fn, lno, msg, args, exc_info, func, extra)

def __init__(self, *args, **kwargs):
super(TestContext, self).__init__(*args, **kwargs)
logger_cls = logging.getLoggerClass()
try:
logging.setLoggerClass(self.TestLogger)
self._logger = logging.getLogger('test')
finally:
logging.setLoggerClass(logger_cls)

@contextmanager
def new_workunit(self, name, labels=None, cmd='', log_config=None):
"""
Expand All @@ -70,7 +91,7 @@ def log(self):
"""
:API: public
"""
return logging.getLogger('test')
return self._logger

def submit_background_work_chain(self, work_chain, parent_workunit_name=None):
"""
Expand All @@ -89,30 +110,6 @@ def subproc_map(self, f, items):
return map(f, items)


# TODO: Make Console and Workspace into subsystems, and simplify this signature.
def create_context(options=None, passthru_args=None, target_roots=None, build_graph=None,
build_file_parser=None, address_mapper=None,
console_outstream=None, workspace=None, scheduler=None):
"""Creates a ``Context`` with no options or targets by default.
:API: public
:param options: A map of scope -> (map of key to value).
Other params are as for ``Context``.
"""
options = create_options(options or {}, passthru_args=passthru_args)
return create_context_from_options(options,
passthru_args=passthru_args,
target_roots=target_roots,
build_graph=build_graph,
build_file_parser=build_file_parser,
address_mapper=address_mapper,
console_outstream=console_outstream,
workspace=workspace,
scheduler=scheduler)


def create_context_from_options(options, target_roots=None, build_graph=None,
build_file_parser=None, address_mapper=None, console_outstream=None,
workspace=None, scheduler=None):
Expand Down

0 comments on commit ef15589

Please sign in to comment.