Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

tests: Factor "Open another ClientStorage to the same server" into generic _new_storage_client() #170

Merged
merged 1 commit into from
Apr 20, 2021
Merged

tests: Factor "Open another ClientStorage to the same server" into generic _new_storage_client() #170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 1 addition & 19 deletions src/ZEO/tests/CommitLockTests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,6 @@

ZERO = b'\0'*8

class DummyDB(object):
def invalidate(self, *args, **kwargs):
pass

transform_record_data = untransform_record_data = lambda self, data: data

class WorkerThread(TestThread):

# run the entire test in a thread so that the blocking call for
Expand Down Expand Up @@ -103,7 +97,7 @@ def _begin_threads(self):
self._threads = []

for i in range(self.NUM_CLIENTS):
storage = self._duplicate_client()
storage = self._new_storage_client()
txn = TransactionMetaData()
tid = self._get_timestamp()

Expand All @@ -122,18 +116,6 @@ def _finish_threads(self):
for t in self._threads:
t.cleanup()

def _duplicate_client(self):
"Open another ClientStorage to the same server."
# It's hard to find the actual address.
# The rpc mgr addr attribute is a list. Each element in the
# list is a socket domain (AF_INET, AF_UNIX, etc.) and an
# address.
addr = self._storage._addr
new = ZEO.ClientStorage.ClientStorage(
addr, wait=1, **self._client_options())
new.registerDB(DummyDB())
return new

def _get_timestamp(self):
t = time.time()
t = TimeStamp(*time.gmtime(t)[:5]+(t%60,))
Expand Down
21 changes: 13 additions & 8 deletions src/ZEO/tests/testZEO.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,7 @@ def checkLargeUpdate(self):
self._dostore(data=obj)

def checkZEOInvalidation(self):
addr = self._storage._addr
storage2 = self._wrap_client(
ClientStorage(addr, wait=1, **self._client_options()))
storage2 = self._new_storage_client()
try:
oid = self._storage.new_oid()
ob = MinPO('first')
Expand Down Expand Up @@ -220,14 +218,13 @@ def checkZEOInvalidation(self):
def checkVolatileCacheWithImmediateLastTransaction(self):
# Earlier, a ClientStorage would not have the last transaction id
# available right after successful connection, this is required now.
addr = self._storage._addr
storage2 = ClientStorage(addr, **self._client_options())
storage2 = self._new_storage_client()
self.assertTrue(storage2.is_connected())
self.assertEqual(ZODB.utils.z64, storage2.lastTransaction())
storage2.close()

self._dostore()
storage3 = ClientStorage(addr, **self._client_options())
storage3 = self._new_storage_client()
self.assertTrue(storage3.is_connected())
self.assertEqual(8, len(storage3.lastTransaction()))
self.assertNotEqual(ZODB.utils.z64, storage3.lastTransaction())
Expand Down Expand Up @@ -262,6 +259,15 @@ def setUp(self):
)
self._storage.registerDB(DummyDB())

# _new_storage_client opens another ClientStorage to the same storage server
# self._storage is connected to. It is used by both ZEO and ZODB tests.
def _new_storage_client(self):
client = ZEO.ClientStorage.ClientStorage(
self._storage._addr, wait=1, **self._client_options())
client = self._wrap_client(client)
client.registerDB(DummyDB())
return client
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the storage really already register with a DB? In some situation, we might want an MVCCAdapter to be registered.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@d-maurer, thanks for feedback. I've thought about that too, and my logic here is:

  • the original code being factored-out had that registerDB(DummyDB()). There is also the same registerDB(DummyDB()) in GenericTestBase.setUp which prepares ._storage client for practically most of the tests:

ZEO/src/ZEO/tests/testZEO.py

Lines 236 to 263 in 707316c

class GenericTestBase(
# Base class for all ZODB tests
StorageTestBase.StorageTestBase):
shared_blob_dir = False
blob_cache_dir = None
server_debug = False
def setUp(self):
StorageTestBase.StorageTestBase.setUp(self)
logger.info("setUp() %s", self.id())
zport, stop = forker.start_zeo_server(
self.getConfig(), self.getZEOConfig(), debug=self.server_debug)
self._servers = [stop]
if not self.blob_cache_dir:
# This is the blob cache for ClientStorage
self.blob_cache_dir = tempfile.mkdtemp(
'blob_cache',
dir=os.path.abspath(os.getcwd()))
self._storage = self._wrap_client(
ClientStorage(
zport, '1', cache_size=20000000,
min_disconnect_poll=0.5, wait=1,
wait_timeout=60, blob_dir=self.blob_cache_dir,
shared_blob_dir=self.shared_blob_dir,
**self._client_options()),
)
self._storage.registerDB(DummyDB())

  • the DB.registerDB, when called for the second time, forgets what was registered first. This way when/if MVCCAdapter registers into ClientStorage, the registration of DummyDB is dropped.

The code in _new_storage_client is this OK, and is in line with the way ClientStorage for main ._storage is prepared.


def getZEOConfig(self):
return forker.ZEOConfig(('127.0.0.1', 0))

Expand Down Expand Up @@ -320,8 +326,7 @@ def checkSortKey(self):
def _do_store_in_separate_thread(self, oid, revid, voted):

def do_store():
store = ZEO.ClientStorage.ClientStorage(
self._storage._addr, **self._client_options())
store = self._new_storage_client()
try:
t = transaction.get()
store.tpc_begin(t)
Expand Down