Skip to content

Commit

Permalink
fix(tests): test_read_only is not flaky anymore.
Browse files Browse the repository at this point in the history
Fixed the `test_read_only` test based on what is done in the official Java client, we were missing some things and that led the test to fail from time to time (see comments in the code for full story).
Also renamed `test__connection.py` to `test_connection.py`, where it belongs.
  • Loading branch information
StephenSorriaux committed Apr 24, 2023
1 parent ffa0ae9 commit 6b40a43
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 5 deletions.
21 changes: 17 additions & 4 deletions kazoo/tests/test__connection.py → kazoo/tests/test_connection.py
Expand Up @@ -18,8 +18,7 @@
from kazoo.protocol.states import KazooState
from kazoo.protocol.connection import _CONNECTION_DROP
from kazoo.testing import KazooTestCase
from kazoo.tests.util import wait
from kazoo.tests.util import CI_ZK_VERSION
from kazoo.tests.util import wait, CI_ZK_VERSION, CI


class Delete(namedtuple("Delete", "path version")):
Expand Down Expand Up @@ -258,7 +257,7 @@ def back(state):
class TestReadOnlyMode(KazooTestCase):
def setUp(self):
os.environ["ZOOKEEPER_LOCAL_SESSION_RO"] = "true"
self.setup_zookeeper(read_only=True)
self.setup_zookeeper()
skip = False
if CI_ZK_VERSION and CI_ZK_VERSION < (3, 4):
skip = True
Expand All @@ -279,7 +278,15 @@ def test_read_only(self):
from kazoo.exceptions import NotReadOnlyCallError
from kazoo.protocol.states import KeeperState

client = self.client
if CI:
# force some wait to make sure the data produced during the
# `setUp()` step are replicaed to all zk members
# if not done the `get_children()` test may fail because the
# node does not exist on the node that we will keep alive
time.sleep(15)
# do not keep the client started in the `setUp` step alive
self.client.stop()
client = self._get_client(connection_retry=None, read_only=True)
states = []
ev = threading.Event()

Expand All @@ -289,6 +296,7 @@ def listen(state):
if client.client_state == KeeperState.CONNECTED_RO:
ev.set()

client.start()
try:
# stopping both nodes at the same time
# else the test seems flaky when on CI hosts
Expand All @@ -303,6 +311,11 @@ def listen(state):
thread.start()
for thread in zk_stop_threads:
thread.join()
# stopping the client is *mandatory*, else the client might try to
# reconnect using a xid that the server may endlessly refuse
# restarting the client makes sure the xid gets reset
client.stop()
client.start()
ev.wait(15)
assert ev.is_set()
assert client.client_state == KeeperState.CONNECTED_RO
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Expand Up @@ -114,11 +114,11 @@ module = [
'kazoo.testing.common',
'kazoo.testing.harness',
'kazoo.tests.conftest',
'kazoo.tests.test__connection',
'kazoo.tests.test_barrier',
'kazoo.tests.test_build',
'kazoo.tests.test_cache',
'kazoo.tests.test_client',
'kazoo.tests.test_connection',
'kazoo.tests.test_counter',
'kazoo.tests.test_election',
'kazoo.tests.test_eventlet_handler',
Expand Down

0 comments on commit 6b40a43

Please sign in to comment.