Opened 6 years ago

Last modified 5 years ago

#3535 enhancement reopened

TrialRunner hides features of non-twisted test cases

Reported by: jamesh Owned by:
Priority: normal Milestone:
Component: trial Keywords:
Cc: exarkun Branch:
Author: Launchpad Bug: 297563

Description

I've been using a package called testresources (https://launchpad.net/testresources) in a project that uses trial as its test runner, and ran into some problems.

The testresources package is designed to speed up test runs by allowing test cases to share a resource, reducing the number of times the resource needs to be set up and torn down.

It does this through the use of a special TestCase subclass and special TestSuite subclass. Each of these classes can work independently in a degraded manner (resources are set up and torn down for each test case), but when used together it will reorder the tests to reduce the number of times resources are set up and torn down.

This works fine with standard test runners, but not with trial. Trial's test runner pokes inside the test suite and replaces all the test cases with _PyUnitTestCaseAdapter instances, which prevents testresources's test suite from optimising resource usage.

It would be nice if trial could do its stuff without screwing around with the tests in the suite.

Change History (16)

comment:1 Changed 6 years ago by jamesh

  • Launchpad Bug set to 297563

comment:2 Changed 6 years ago by jamesh

  • Component changed from core to trial
  • Owner changed from glyph to jml

comment:3 Changed 6 years ago by exarkun

  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #3531

comment:4 Changed 6 years ago by jamesh

That other bug is related but doesn't seem to be a duplicate.

Bug #3531 is basically asking for a way to either invoke trial against a custom TestSuite, or a way to reuse trial's option parsing outside of the trial executable.

This bug is about TrialRunner.run() screwing around with test suites, making it incompatible with other test infrastructure (testresources might not be the only package affected by its current behaviour).

comment:5 Changed 6 years ago by exarkun

  • Cc exarkun added

Trial "screwing around with test suites" isn't a specific enough problem to fix. This may not be a duplicate of #3531, but if not, there needs to be a more specific description of what's wrong and how we will know when it's fixed.

The reason I marked it as a duplicate is that working with testresource could be such a specific goal. If you've got another one in mind, please describe it and re-open the ticket.

Thanks!

comment:6 Changed 6 years ago by jamesh

Okay. Here are some specifics of the problem I ran into:

  1. I have a testresources.OptimisingTestSuite containing a number of testresources.ResourcedTestCase tests.
  2. A ResourcedTestCase has a "resources" attribute containing a list of resources that will be created during setUp() and cleaned up during tearDown().
  3. When an OptimisingTestSuite is run, it checks if any of the contained tests are ResourcedTestCases and sorts them based on the resources they use (by checking the "resources" attribute). While running its tests, the suite suppresses clean up of resources if they are used by sequential tests.

Now when I introduce TrialRunner into the mix, things break down. The decorate() call in TrialRunner.run() replaces the ResourcedTestCase instances with wrappers that don't expose the resource list to their containing suite, so none of the special behaviour of OptimisingTestSuite takes effect.

Making the wrapper class proxy attributes outside of ITestCase would probably be enough in this particular case, but it would be nice if Trial could do its thing without modifying the test suite in this way.

comment:7 follow-up: Changed 6 years ago by exarkun

First, thanks for the detailed description of how testresources and the decoration trial does are conflicting. That should help get that issue resolved more easily. I do think that that issue is #3531.

I think you're trying to make the case that the decoration done by trial is a problem in general. I could certainly imagine some project other than testresources which uses some other attribute to do something. If there really is an unresolvable conflict between the functionality provided by projects such as testresources and the functionality provided by trial then one of them has to lose (meaning that functionality is lost). That would be sad. Hopefully, if there is a conflict, it is only between how the functionality is provided. But... maybe there isn't a conflict at all?

You said testresources works by having a ResourcedTestCase class. This sounds like something for which an ITestCase adapter could be registered, an adapter which provides the behavior trial requires and the behavior testresources requires. Since the adapter registry is public, testresources (or a project using testresources and trial) could register this adapter. I'm not exactly sure what the adapter would do, but it sounds like perhaps it would do all the things _PyUnitTestCaseAdapter does, plus expose the resources attribute.

Does this sound insane?

comment:8 in reply to: ↑ 7 Changed 6 years ago by jamesh

Replying to exarkun:

Does this sound insane?

I gave this a go, subclassing _PyUnitTestCaseAdapter, adding a property to forward the "resources" attribute and register it as an ITestCase adapter for ResourcedTestCase. This seemed to get resource sharing working again, so that is the work around I'll use.

I did notice that other uses of decorate() could cause problems though: If TwistedRunner._forceGarbageCollection is set, the tests get wrapped in a way that can't be controlled by additional adapter registration.

This behaviour in Trial seems pretty heavy handed. Is there really no other way for it to implement these features?

comment:9 Changed 6 years ago by exarkun

  • Resolution duplicate deleted
  • Status changed from closed to reopened

I'm glad the custom adapter worked for your use-case. I hope this means trial can continue to be useful to you for a while longer. :)

I do think you're right about _forceGarbageCollection getting in the way, though, if you try to take advantage of that trial feature.

The approach trial takes to implementing these features was arrived at after a long, painful discussion about what the core philosophy of xUnit is. Blech. I don't know about anyone else, but I'm not personally attached to the current implementation. I'd be happy to discuss alternatives with anyone with an interest. I can also try to summarize the goals somewhere persistent (probably a pre-requisite for a useful conversation), if someone indicates they'd like such a thing. It would also be nice to get input from jml, radix, and glyph (availability permitting), as they were all also involved in the discussion which led to trial's current state.

I'm re-opening this now. I'll probably mess around with the summary and description a bit later to try to clarify the idea that the goal here is some kind of (specific, but to be specified later) interoperability with other tools and libraries.

comment:10 Changed 6 years ago by jml

FWIW, I'm also not attached to the current implementation.

comment:11 Changed 6 years ago by glyph

No attachment to the current implementation here either. In fact I suspect we all believe it could be improved :).

comment:12 Changed 6 years ago by glyph

After talking to lifeless a bit, it seems like the easy fix would be simply adding arbitrary-attribute forwarding to all the test decorators currently present in Twisted.

Since they already descend from a common superclass, this could be done by making TestDecorator into a more permissive proxy, rather than a proxyForInterface. Given that we're interposing between a (potential subclass of) TestSuite and its (potential subclass of) TestCase, we don't know whether the interface has been expanded between those two objects in their composite relationship. So we don't know if we should proxy for only one interface.

comment:13 Changed 6 years ago by exarkun

There's no need to rush to a suboptimal solution here. jamesh has a workaround for the time being. I'd like to explore a more complete solution than just forwarding attributes.

comment:14 Changed 5 years ago by robertc

# James' workaround.

import testresources
from twisted.python.components import registerAdapter
from twisted.trial.itrial import ITestCase
from twisted.trial.unittest import _PyUnitTestCaseAdapter

class ResourcedTestCaseAdapter(_PyUnitTestCaseAdapter):

"""An ITestCase adapter for ResourcedTestCase instances.

This is required in order for OptimisingTestSuite to function
correctly with the Trial test runner. See
U{http://twistedmatrix.com/trac/ticket/3535} for details.
"""
@property
def resources(self):

"""Forward the resources attribute from the original test case."""
return self._originalTest.resources

registerAdapter(

ResourcedTestCaseAdapter, testresources.ResourcedTestCase, ITestCase)

comment:15 Changed 5 years ago by jamesh

Here's the workaround again, in a form that hopefully won't be mangled by trac:

import testresources
from twisted.python.components import registerAdapter
from twisted.trial.itrial import ITestCase
from twisted.trial.unittest import _PyUnitTestCaseAdapter

class ResourcedTestCaseAdapter(_PyUnitTestCaseAdapter):
    """An `ITestCase` adapter for `ResourcedTestCase` instances.

    This is required in order for `OptimisingTestSuite` to function
    correctly with the Trial test runner.  See
    U{http://twistedmatrix.com/trac/ticket/3535} for details.
    """
    @property
    def resources(self):
        """Forward the resources attribute from the original test case."""
        return self._originalTest.resources

registerAdapter(
    ResourcedTestCaseAdapter, testresources.ResourcedTestCase, ITestCase)

comment:16 Changed 4 years ago by <automation>

  • Owner jml deleted
Note: See TracTickets for help on using tickets.