Reporter is not extensible enough

[thedrow]

All I wanted to do is to create a simple plugin that pretty prints the traceback. Here is the final code that accomplished my goal:

import sys

from nose2 import events
from nose2.util import _WritelnDecorator
import pygments
from pygments import lexers, formatters

class TracebackPrettyPrinter(_WritelnDecorator):
    def __init__(self, stream, pretty_print_traceback, ansi256):
        super(TracebackPrettyPrinter, self).__init__(stream)
        self.pretty_print_traceback = pretty_print_traceback
        self.ansi256 = ansi256

    def write(self, arg):
        if 'Traceback (most recent call last):' in arg:
            super(TracebackPrettyPrinter, self).write(self._prettyprint_traceback(arg))
        else:
            super(TracebackPrettyPrinter, self).write(arg)

    def writeln(self, arg=None):
        if arg and 'Traceback (most recent call last):' in arg:
            super(TracebackPrettyPrinter, self).write(self._prettyprint_traceback(arg))
        else:
            super(TracebackPrettyPrinter, self).write(arg)

    def _prettyprint_traceback(self, traceback):
        lexer = 'py3tb' if sys.version_info[0] == 3 else 'pytb'
        formatter = 'terminal%s' % '256' if self.ansi256 else ''
        return pygments.highlight(traceback, lexers.get_lexer_by_name(lexer),
                                  formatters.get_formatter_by_name(formatter))

class PrettyTracebackReporter(events.Plugin):
    commandLineSwitch = (
        None, 'pretty-print-traceback', 'Prettyprint Traceback on errors and failures')
    configSection = 'pretty-print'
    alwaysOn = True

    def __init__(self):
        self.pretty_print_traceback = self.config.as_bool('pretty_print_traceback', True)
        self.ansi256 = self.config.as_bool('ansi256', True)

    def beforeErrorList(self, event):
        event.stream = TracebackPrettyPrinter(event.stream, self.pretty_print_traceback, self.ansi256)

That's pretty hackish. Instead of writing anything to any stream a hook should be called. If nothing was reported to the terminal the default reporter should report as it did. Since there is no afterErrorList hook I can't even change back the stream to not handle tracebacks and when a module can't be loaded it pretty prints the traceback nose2 raised as well. It's not what I wanted.

[jpellerin]

Could you use the outcomeDetail hook instead? That gives you access to the OutcomeDetailEvent (https://nose2.readthedocs.org/en/latest/dev/event_reference.html#nose2.events.OutcomeDetailEvent) which includes the TestOutcomeEvent which has the traceback in the exc_info tuple.

[thedrow]

Yes I can. However the real issue with this design is that the reporter actually acts instead of delegating it's own events to other reporters. There should be a PluggableResultReporter class that simply calls hooks and a default implementation that handles them if the event was not handled. It will allow better flexibility when writing reporters or replacing parts of reporters.

[jpellerin]

Can you flesh that out a bit more? What events and hooks do you see the pluggable reporter sending?

On Tue, Jul 2, 2013 at 3:22 PM, Omer Katz notifications@github.com wrote:

Yes I can. However the real issue with this design is that the reporter actually acts instead of delegating it's own events to other reporters. There should be a PluggableResultReporter class that simply calls hooks and a default implementation that handles them if the event was not handled. It will allow better flexibility when writing reporters or replacing parts of reporters.

— Reply to this email directly or view it on GitHubhttps://github.com/nose-devs/nose2/issues/96#issuecomment-20369223 .

[thedrow]

Sure I can but it's 1:17 AM :) Generally speaking, every private method that the reporter has (every method that starts with _) should be converted to a single or multiple events. We need to scan the code together and basically write down some sort of activity diagram in order to decouple the reporting itself from the actual reporting event.

[jpellerin]

Ok, I'm on board. I've tagged this as a feature needed before 1.0.

[thedrow]

So first things first. I think we should document what exactly each and every reporter method does. This piece of code is pretty obvious despite the lack of documentation so we'll start here. I still think it will be useful to add further documentation the public methods and document the private methods.

def startTest(self, event):
        """Handle startTest hook

         - prints test description if verbosity > 1
         """
        self.testsRun += 1
        self._reportStartTest(event)

#...
def _reportStartTest(self, event):
        evt = events.ReportTestEvent(event, self.stream)
        self.session.hooks.reportStartTest(evt)
        if evt.handled:
            return
        if self.session.verbosity > 1:
            # allow other plugins to override/spy on stream
            evt.stream.write(self._getDescription(event.test, errorList=False))
            evt.stream.write(' ... ')
            evt.stream.flush()
#...
def _getDescription(self, test, errorList):
        doc_first_line = test.shortDescription()
        if self.descriptions and doc_first_line:
            desc = '\n'.join((str(test), doc_first_line))
        else:
            desc = str(test)
        event = events.DescribeTestEvent(
            test, description=desc, errorList=errorList)
        self.session.hooks.describeTest(event)
        return event.description

First of all, I don't think that the reporter itself should count how many tests were run. That's the responsibility of the test runner Instead it should be passed as an attribute in the event. _reportStartTest() should be replaced with a ReportTestStarted event that will be intercepted by the default reporter. The DescribeTestEvent seems to be in it's place but the value should be passed into the ReportTestStarted event instead of being printed. In my opinion verbosity should be passed to the events as well. What do you think?

This entire issue should be turned into a section in the docs that explains how to write your own reporter.

[jpellerin]

Sounds like a good start.

On Fri, Jul 5, 2013 at 11:03 AM, Omer Katz notifications@github.com wrote:

So first things first. I think we should document what exactly each and every reporter method does.

def startTest(self, event): """Handle startTest hook

• prints test description if verbosity > 1 """ self.testsRun += 1 self._reportStartTest(event)

  ...def _reportStartTest(self, event):

  evt = events.ReportTestEvent(event, self.stream)
  self.session.hooks.reportStartTest(evt)
  if evt.handled:
      return
  if self.session.verbosity > 1:
      # allow other plugins to override/spy on stream
      evt.stream.write(self._getDescription(event.test, errorList=False))
      evt.stream.write(' ... ')
      evt.stream.flush()#...def _getDescription(self, test, errorList):
  doc_first_line = test.shortDescription()
  if self.descriptions and doc_first_line:
      desc = '\n'.join((str(test), doc_first_line))
  else:
      desc = str(test)
  event = events.DescribeTestEvent(
      test, description=desc, errorList=errorList)
  self.session.hooks.describeTest(event)
  return event.description

First of all, I don't think that the reporter itself should count how many tests were run. Instead it should be passed as an attribute in the event. _reportStartTest() should be replaced with a ReportTestStarted event that will be intercepted by the default reporter. The DescribeTestEvent seems to be in it's place but the value should be passed into the ReportTestStarted event instead of being printed. In my opinion verbosity should be passed to the events as well. What do you think?

This entire issue should be turned into a section in the docs that explains how to write your own reporter.

— Reply to this email directly or view it on GitHubhttps://github.com/nose-devs/nose2/issues/96#issuecomment-20523339 .

[thedrow]

So how do we start implementing it? Should I create a pull request with the basic structure of the new reporter?

[jpellerin]

Sure, that sounds good.

On Sun, Jul 7, 2013 at 6:14 AM, Omer Katz notifications@github.com wrote:

So how do we start implementing it? Should I create a pull request with the basic structure of the new reporter?

— Reply to this email directly or view it on GitHubhttps://github.com/nose-devs/nose2/issues/96#issuecomment-20568620 .