treq.testing.StubTreq does not handle twisted.web.server.Session objects correctly.

[bennr01]

Hi,

I observed a bug when using StubTreq for testing a twisted.web.Resource using request.getSession() to store data between requests. Below is a rather dry report and my proposal for fixing this bug.

Expected behavior

When using treq.testing.StubTreq(some_resource) to make requests to the wrapped resource, the wrapped resource should be able to obtain the same twisted.web.server.Session instance every time request.getSession() is called in one of its render*(request) methods (provided that the same cookies parameter is supplied for every request).

Observed behavior

Even with proper cookies being set, calling request.getSession() in the wrapped resource returns a twisted.web.server.Session instance with a different uid. I confirmed that the TWISTED_SESSION cookie was used for both requests.

Debugging result

Each twisted.web.server.Session instance is stored in the twisted.web.server.Site instance. treq.testing.StubTreq, or more precisely treq.testing.RequestTraversalAgent creates a new twisted.web.server.Site instance for each request. Thus, the sessions aren't stored between requests.

Fix

Changing treq.testing.RequestTraversalAgent so that the serverFactory variable is moved from .request() to .__init__() and made an attribute of the treq.testing.RequestTraversalAgent shared between requests seems to fix this. Of course, the subsequent references to serverFactory must also be changed to the new attribute instead.

However, this introduces a new semi-problem: When using StubTreq for testing (which it likely will be used for) in combination with sessions makes it easy to leave a dirty reactor behind. This is caused by the DelayedCalls to Session.expire(), thus manual cleanup is needed. Changing StubTreq to make the _agent also a permanent attribute allows manual cleanup by directly expiring the sessions on tearDown() . This, however, is a rather 'ugly' affair. Thus, I want to propose to also add a cleanSessions() (or a more generic clean()) method to treq.testing.StubTreq to offer the user a simple way of cleaning the reactor when sessions were used.

[twm]

Released in treq 21.5.0