Code review comment for lp:~yellow/subunit/test-count

Hey guys,

Thanks for looking into this one. I don't think that this change should go into subunit. The _get_concrete_result thing is just too hideous. I wrote it originally so I hope you aren't offended by me saying so.

What testrepository needs is a way to increment testsRun on its CLITestResult instance even when tests are filtered out of the stream. The right way to do this is to pass that result into TestResultFilter somehow. Here are some options:

  class MyTestResultFilter(TestResultFilter):

    def __init__(self, result, on_filter, **kwargs):
      super(MyTestResultFilter, self).__init__(result, **kwargs)
      self._on_filter = on_filter

    def _filtered(self):
      super(MyTestResultFilter, self)._filtered()
      self._on_filter()

Or you could make the last line:

      self._on_filter(self._current_test)

Instead of writing that as a subclass, it could be a patch to TestResultFilter in subunit. In which case it would have to be an optional argument. Then you could call TestResultFilter(cli_test_result, lambda test: cli_test_result.testsRun += 1) in testrepository. (Except that won't work because of Python's statement / expression hang up.)

Alternatively, you could make a more specific version and have it live in testrepository:

  class MyTestResultFilter(TestResultFilter):

    def __init__(self, result, cli_test_result, **kwargs):
      super(MyTestResultFilter, self).__init__(result, **kwargs)
      self._cli_test_result = cli_test_result

    def _filtered(self):
      super(MyTestResultFilter, self)._filtered()
      self._cli_test_result.testsRun += 1

I'm sorry to bounce back so hard on this, but the getattr() searching is really quite fragile.

Thanks,
jml