PR #281 broke tests in other libraries

cutz commented 5 years ago

For instance there are now tests in nti.app.products.courseware_admin (among others) failing with the following assertion failure.

Error in test test_admin_views (nti.app.products.courseware_admin.tests.test_management_views.TestCourseManagement)
Traceback (most recent call last):
  File "/Users/cutz/pypy2.7-7.1/lib-python/2.7/unittest/case.py", line 358, in run
    self.tearDown()
  File "/Users/cutz/Documents/NextThought/nti.dataserver-pypy/sources/nti.dataserver/src/nti/dataserver/tests/mock_dataserver.py", line 138, in f
    ds = factory(base_storage=_base_storage)
  File "/Users/cutz/Documents/NextThought/nti.dataserver-pypy/sources/nti.dataserver/src/nti/dataserver/tests/mock_dataserver.py", line 55, in __init__
    super(ChangePassingMockDataserver,self).__init__(*args, **kwargs)
  File "/Users/cutz/Documents/NextThought/nti.dataserver-pypy/sources/nti.dataserver/src/nti/dataserver/_Dataserver.py", line 457, in __init__
    super(Dataserver, self).__init__(parentDir)
  File "/Users/cutz/Documents/NextThought/nti.dataserver-pypy/sources/nti.dataserver/src/nti/dataserver/_Dataserver.py", line 125, in __init__
    self._open_dbs()
  File "/Users/cutz/Documents/NextThought/nti.dataserver-pypy/sources/nti.dataserver/src/nti/dataserver/_Dataserver.py", line 145, in _open_dbs
    assert not transaction.manager.explicit
AssertionError

jamadden commented 5 years ago

Hmm. Why are we trying to create a new MockDataserver on test tearDown? That doesn't seem at all right. (Line numbers in mock_dataserver.py traceback don't match master.)

jamadden commented 5 years ago

Most likely what's going on here is the shortcut taken in the connection tween, as described by the last sentence: https://github.com/NextThought/nti.dataserver/blob/26d1637a59ce57a78460fe5f88787879b5226cae/src/nti/appserver/tweens/zodb_connection_tween.py#L52-L57

That's true for the real appserver, but not true for app-layer tests. The nti.testing hooks to reset the global manager to implicit don't get a chance to run in app-layer tests (because they use shared setup).

It'll cost a tiny bit of speed, but probably the simplest thing is to have the tween reverse that change on the way out.

cutz commented 5 years ago

On master I'm getting past the AssertionError and now I'm on to test assertion errors in the actual tests. That's some progress.....

OpenNTI / nti.dataserver

PR #281 broke tests in other libraries #282