Merge pull request #139 from zalando/codequality

remove unused code

patroni/postgresql.py

@@ -526,8 +526,7 @@ recovery_target_timeline = 'latest'
                         result[name] = val
         except IOError:
             logger.exception('Error when reading postmaster.opts')
-        finally:
-            return result
+        return result
 
     def single_user_mode(self, command=None, options=None):
         """ run a given command in a single-user mode. If the command is empty - then just start and stop """
@@ -560,47 +559,45 @@ recovery_target_timeline = 'latest'
             logger.exception("Unable to list %s", status_dir)
 
     def follow(self, leader, recovery=False):
-        if not self.check_recovery_conf(leader) or recovery:
-            change_role = (self.role == 'master')
-
-            self._need_rewind = (self._need_rewind or change_role) and self.can_rewind
-            if self._need_rewind:
-                logger.info("set the rewind flag after demote")
-            self.write_recovery_conf(leader)
-            if not leader or not self._need_rewind:  # do not rewind until the leader becomes available
-                ret = self.restart()
-            else:  # we have a leader and need to rewind
-                if self.is_running():
-                    self.stop()
-                # at present, pg_rewind only runs when the cluster is shut down cleanly
-                # and not shutdown in recovery. We have to remove the recovery.conf if present
-                # and start/shutdown in a single user mode to emulate this.
-                # XXX: if recovery.conf is linked, it will be written anew as a normal file.
-                if os.path.islink(self.recovery_conf):
-                    os.unlink(self.recovery_conf)
-                else:
-                    os.remove(self.recovery_conf)
-                # Archived segments might be useful to pg_rewind,
-                # clean the flags that tell we should remove them.
-                self.cleanup_archive_status()
-                # Start in a single user mode and stop to produce a clean shutdown
-                opts = self.read_postmaster_opts()
-                opts['archive_mode'] = 'on'
-                opts['archive_command'] = 'false'
-                self.single_user_mode(options=opts)
-                if self.rewind(leader):
-                    ret = self.start()
-                else:
-                    logger.error("unable to rewind the former master")
-                    self.remove_data_directory()
-                    ret = True
-                self._need_rewind = False
-            if change_role and ret:
-                self.call_nowait(ACTION_ON_ROLE_CHANGE)
-            return ret
-        else:
+        if self.check_recovery_conf(leader) and not recovery:
             return True
 
+        change_role = self.role == 'master'
+        self._need_rewind = (self._need_rewind or change_role) and self.can_rewind
+        if self._need_rewind:
+            logger.info("set the rewind flag after demote")
+        self.write_recovery_conf(leader)
+        if leader and self._need_rewind:  # we have a leader and need to rewind
+            if self.is_running():
+                self.stop()
+            # at present, pg_rewind only runs when the cluster is shut down cleanly
+            # and not shutdown in recovery. We have to remove the recovery.conf if present
+            # and start/shutdown in a single user mode to emulate this.
+            # XXX: if recovery.conf is linked, it will be written anew as a normal file.
+            if os.path.islink(self.recovery_conf):
+                os.unlink(self.recovery_conf)
+            else:
+                os.remove(self.recovery_conf)
+            # Archived segments might be useful to pg_rewind,
+            # clean the flags that tell we should remove them.
+            self.cleanup_archive_status()
+            # Start in a single user mode and stop to produce a clean shutdown
+            opts = self.read_postmaster_opts()
+            opts.update({'archive_mode': 'on', 'archive_command': 'false'})
+            self.single_user_mode(options=opts)
+            if self.rewind(leader):
+                ret = self.start()
+            else:
+                logger.error("unable to rewind the former master")
+                self.remove_data_directory()
+                ret = True
+            self._need_rewind = False
+        else:  # do not rewind until the leader becomes available
+            ret = self.restart()
+        if change_role and ret:
+            self.call_nowait(ACTION_ON_ROLE_CHANGE)
+        return ret
+
     def save_configuration_files(self):
         """
             copy postgresql.conf to postgresql.conf.backup to be able to retrive configuration files

tests/test_api.py

@@ -101,10 +101,10 @@ class TestRestApiHandler(unittest.TestCase):
             MockRestApiServer(RestApiHandler, b'GET /master')
         with patch.object(MockHa, 'restart_scheduled', Mock(return_value=True)):
             MockRestApiServer(RestApiHandler, b'GET /master')
-        MockRestApiServer(RestApiHandler, b'GET /master')
+        self.assertIsNotNone(MockRestApiServer(RestApiHandler, b'GET /master'))
 
     def test_do_OPTIONS(self):
-        MockRestApiServer(RestApiHandler, b'OPTIONS / HTTP/1.0')
+        self.assertIsNotNone(MockRestApiServer(RestApiHandler, b'OPTIONS / HTTP/1.0'))
 
         with patch.object(BaseHTTPRequestHandler, 'handle_one_request') as mock_handle_request:
             mock_handle_request.side_effect = socket.error("foo")
@@ -118,15 +118,15 @@ class TestRestApiHandler(unittest.TestCase):
             MockRestApiServer(RestApiHandler, b'OPTIONS / HTTP/1.0')
 
     def test_do_GET_patroni(self):
-        MockRestApiServer(RestApiHandler, b'GET /patroni')
+        self.assertIsNotNone(MockRestApiServer(RestApiHandler, b'GET /patroni'))
 
     def test_basicauth(self):
-        MockRestApiServer(RestApiHandler, b'POST /restart HTTP/1.0')
+        self.assertIsNotNone(MockRestApiServer(RestApiHandler, b'POST /restart HTTP/1.0'))
         MockRestApiServer(RestApiHandler, b'POST /restart HTTP/1.0\nAuthorization:')
 
     def test_do_POST_restart(self):
         request = b'POST /restart HTTP/1.0\nAuthorization: Basic dGVzdDp0ZXN0'
-        MockRestApiServer(RestApiHandler, request)
+        self.assertIsNotNone(MockRestApiServer(RestApiHandler, request))
         with patch.object(MockHa, 'restart', Mock(side_effect=Exception)):
             MockRestApiServer(RestApiHandler, request)
 
@@ -140,14 +140,14 @@ class TestRestApiHandler(unittest.TestCase):
         with patch.object(MockHa, 'schedule_reinitialize', Mock(return_value=None)):
             MockRestApiServer(RestApiHandler, request)
         cluster.leader.name = 'test'
-        MockRestApiServer(RestApiHandler, request)
+        self.assertIsNotNone(MockRestApiServer(RestApiHandler, request))
 
     @patch('time.sleep', Mock())
     def test_RestApiServer_query(self):
         with patch.object(MockCursor, 'execute', Mock(side_effect=psycopg2.OperationalError)):
-            MockRestApiServer(RestApiHandler, b'GET /patroni')
+            self.assertIsNotNone(MockRestApiServer(RestApiHandler, b'GET /patroni'))
         with patch.object(MockPostgresql, 'connection', Mock(side_effect=psycopg2.OperationalError)):
-            MockRestApiServer(RestApiHandler, b'GET /patroni')
+            self.assertIsNotNone(MockRestApiServer(RestApiHandler, b'GET /patroni'))
 
     @patch('time.sleep', Mock())
     @patch.object(MockHa, 'dcs')
@@ -195,4 +195,4 @@ class TestRestApiHandler(unittest.TestCase):
         # Invalid date
         request = b'POST /failover HTTP/1.0\nAuthorization: Basic dGVzdDp0ZXN0\nContent-Length: 103\n\n{"leader": ' +\
                   b'"postgresql1", "member": "postgresql2", "scheduled_at": "2010-02-29T18:13:30.568224+01:00"}'
-        MockRestApiServer(RestApiHandler, request)
+        self.assertIsNotNone(MockRestApiServer(RestApiHandler, request))

tests/test_ctl.py

@@ -1,25 +1,19 @@
-#!/usr/bin/env python
-# -*- coding: utf-8 -*-
-
 import os
 import pytest
+import requests.exceptions
 import unittest
-import psycopg2
-import requests
-import patroni.exceptions
-import etcd
-from mock import patch, Mock, MagicMock
-
 
 from click.testing import CliRunner
+from etcd import EtcdException
+from mock import patch, Mock, MagicMock
 from patroni.ctl import ctl, members, store_config, load_config, output_members, post_patroni, get_dcs, \
     wait_for_leader, get_all_members, get_any_member, get_cursor, query_member, configure
-from patroni.ha import Ha
 from patroni.etcd import Etcd, Client
-from test_ha import get_cluster_initialized_without_leader, get_cluster_initialized_with_leader, \
-    get_cluster_initialized_with_only_leader, MockPostgresql, MockPatroni, run_async, \
-    get_cluster_not_initialized_without_leader
+from patroni.exceptions import PatroniCtlException
+from psycopg2 import OperationalError
 from test_etcd import etcd_read, etcd_write, requests_get, socket_getaddrinfo, MockResponse
+from test_ha import get_cluster_initialized_without_leader, get_cluster_initialized_with_leader, \
+    get_cluster_initialized_with_only_leader
 from test_postgresql import MockConnect, psycopg2_connect
 
 CONFIG_FILE_PATH = './test-ctl.yaml'
@@ -56,37 +50,27 @@ class TestCtl(unittest.TestCase):
         self.runner = CliRunner()
         with patch.object(Client, 'machines') as mock_machines:
             mock_machines.__get__ = Mock(return_value=['http://remotehost:2379'])
-            self.p = MockPostgresql()
             self.e = Etcd('foo', {'ttl': 30, 'host': 'ok:2379', 'scope': 'test'})
             self.e.client.read = etcd_read
             self.e.client.write = etcd_write
-            self.e.client.delete = Mock(side_effect=etcd.EtcdException())
-            self.ha = Ha(MockPatroni(self.p, self.e))
-            self.ha._async_executor.run_async = run_async
-            self.ha.old_cluster = self.e.get_cluster()
-            self.ha.cluster = get_cluster_not_initialized_without_leader()
-            self.ha.load_cluster_from_dcs = Mock()
+            self.e.client.delete = Mock(side_effect=EtcdException)
 
     @patch('psycopg2.connect', psycopg2_connect)
     def test_get_cursor(self):
-        c = get_cursor(get_cluster_initialized_without_leader(), role='master')
-        assert c is None
+        self.assertIsNone(get_cursor(get_cluster_initialized_without_leader(), role='master'))
 
-        c = get_cursor(get_cluster_initialized_with_leader(), role='master')
-        assert c is not None
+        self.assertIsNotNone(get_cursor(get_cluster_initialized_with_leader(), role='master'))
 
-        c = get_cursor(get_cluster_initialized_with_leader(), role='replica')
-        # # MockCursor returns pg_is_in_recovery as false
-        assert c is None
+        # MockCursor returns pg_is_in_recovery as false
+        self.assertIsNone(get_cursor(get_cluster_initialized_with_leader(), role='replica'))
 
-        c = get_cursor(get_cluster_initialized_with_leader(), role='any')
-        assert c is not None
+        self.assertIsNotNone(get_cursor(get_cluster_initialized_with_leader(), role='any'))
 
     def test_output_members(self):
         cluster = get_cluster_initialized_with_leader()
-        output_members(cluster, name='abc', fmt='pretty')
-        output_members(cluster, name='abc', fmt='json')
-        output_members(cluster, name='abc', fmt='tsv')
+        self.assertIsNone(output_members(cluster, name='abc', fmt='pretty'))
+        self.assertIsNone(output_members(cluster, name='abc', fmt='json'))
+        self.assertIsNone(output_members(cluster, name='abc', fmt='tsv'))
 
     @patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_with_leader()))
     @patch('patroni.etcd.Etcd.get_etcd_client', Mock(return_value=None))
@@ -190,7 +174,7 @@ y''')
             assert 'Failover failed' in result.output
 
     def test_(self):
-        self.assertRaises(patroni.exceptions.PatroniCtlException, get_dcs, {'scheme': 'dummy'}, 'dummy')
+        self.assertRaises(PatroniCtlException, get_dcs, {'scheme': 'dummy'}, 'dummy')
 
     @patch('psycopg2.connect', psycopg2_connect)
     @patch('patroni.ctl.query_member', Mock(return_value=([['mock column']], None)))
@@ -240,22 +224,22 @@ y''')
     @patch('patroni.ctl.get_cursor', Mock(return_value=MockConnect().cursor()))
     def test_query_member(self):
         rows = query_member(None, None, None, 'master', 'SELECT pg_is_in_recovery()')
-        assert 'False' in str(rows)
+        self.assertTrue('False' in str(rows))
 
         rows = query_member(None, None, None, 'replica', 'SELECT pg_is_in_recovery()')
-        assert rows == (None, None)
+        self.assertEquals(rows, (None, None))
 
         with patch('patroni.ctl.get_cursor', Mock(return_value=None)):
             rows = query_member(None, None, None, None, 'SELECT pg_is_in_recovery()')
-            assert 'No connection to' in str(rows)
+            self.assertTrue('No connection to' in str(rows))
 
             rows = query_member(None, None, None, 'replica', 'SELECT pg_is_in_recovery()')
-            assert 'No connection to' in str(rows)
+            self.assertTrue('No connection to' in str(rows))
 
-        with patch('patroni.ctl.get_cursor', Mock(side_effect=psycopg2.OperationalError('bla'))):
+        with patch('patroni.ctl.get_cursor', Mock(side_effect=OperationalError('bla'))):
             rows = query_member(None, None, None, 'replica', 'SELECT pg_is_in_recovery()')
 
-        with patch('test_postgresql.MockCursor.execute', Mock(side_effect=psycopg2.OperationalError('bla'))):
+        with patch('test_postgresql.MockCursor.execute', Mock(side_effect=OperationalError('bla'))):
             rows = query_member(None, None, None, 'replica', 'SELECT pg_is_in_recovery()')
 
     @patch('patroni.dcs.AbstractDCS.get_cluster', Mock(return_value=get_cluster_initialized_with_leader()))
@@ -345,7 +329,7 @@ leader''')
     @patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_with_leader()))
     def test_wait_for_leader(self):
         dcs = self.e
-        self.assertRaises(patroni.exceptions.PatroniCtlException, wait_for_leader, dcs, 0)
+        self.assertRaises(PatroniCtlException, wait_for_leader, dcs, 0)
 
         cluster = wait_for_leader(dcs=dcs, timeout=2)
         assert cluster.leader.member.name == 'leader'
@@ -362,26 +346,23 @@ leader''')
         assert 'Usage:' in result.output
 
     def test_get_any_member(self):
-        m = get_any_member(get_cluster_initialized_without_leader(), role='master')
-        assert m is None
+        self.assertIsNone(get_any_member(get_cluster_initialized_without_leader(), role='master'))
 
         m = get_any_member(get_cluster_initialized_with_leader(), role='master')
-        assert m.name == 'leader'
+        self.assertEquals(m.name, 'leader')
 
     def test_get_all_members(self):
-        r = list(get_all_members(get_cluster_initialized_without_leader(), role='master'))
-        assert len(r) == 0
+        self.assertEquals(list(get_all_members(get_cluster_initialized_without_leader(), role='master')), [])
 
         r = list(get_all_members(get_cluster_initialized_with_leader(), role='master'))
-        assert len(r) == 1
-        assert r[0].name == 'leader'
+        self.assertEquals(len(r), 1)
+        self.assertEquals(r[0].name, 'leader')
 
         r = list(get_all_members(get_cluster_initialized_with_leader(), role='replica'))
-        assert len(r) == 1
-        assert r[0].name == 'other'
+        self.assertEquals(len(r), 1)
+        self.assertEquals(r[0].name, 'other')
 
-        r = list(get_all_members(get_cluster_initialized_without_leader(), role='replica'))
-        assert len(r) == 2
+        self.assertEquals(len(list(get_all_members(get_cluster_initialized_without_leader(), role='replica'))), 2)
 
     @patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_with_leader()))
     @patch('patroni.etcd.Etcd.get_etcd_client', Mock(return_value=None))

tests/test_ha.py

@@ -1,13 +1,14 @@
-import etcd
 import unittest
 import datetime
 import pytz
 
+from etcd import EtcdException
 from mock import Mock, MagicMock, patch
 from patroni.dcs import Cluster, Failover, Leader, Member
 from patroni.etcd import Client, Etcd
 from patroni.exceptions import DCSError, PostgresException
 from patroni.ha import Ha
+from patroni.postgresql import Postgresql
 from test_etcd import socket_getaddrinfo, etcd_read, etcd_write, requests_get
 
 
@@ -45,56 +46,6 @@ def get_cluster_initialized_with_only_leader(failover=None):
     return get_cluster(True, l, [l], failover)
 
 
-class MockPostgresql(Mock):
-
-    name = 'postgresql0'
-    role = 'replica'
-    state = 'running'
-    connection_string = 'postgres://foo@bar/postgres'
-    server_version = '999999'
-    scope = 'dummy'
-
-    @staticmethod
-    def is_healthy():
-        return True
-
-    @staticmethod
-    def start():
-        return True
-
-    @staticmethod
-    def is_healthiest_node(members):
-        return True
-
-    @staticmethod
-    def is_leader():
-        return True
-
-    @staticmethod
-    def xlog_position():
-        return 0
-
-    @staticmethod
-    def last_operation():
-        return 0
-
-    @staticmethod
-    def data_directory_empty():
-        return False
-
-    @staticmethod
-    def bootstrap(*args, **kwargs):
-        return True
-
-    @staticmethod
-    def check_replication_lag(last_leader_operation):
-        return True
-
-    @staticmethod
-    def check_recovery_conf(leader):
-        return False
-
-
 class MockPatroni(object):
 
     def __init__(self, p, d):
@@ -111,18 +62,35 @@ def run_async(func, args=()):
     return func(*args) if args else func()
 
 
+@patch.object(Postgresql, 'is_running', Mock(return_value=True))
+@patch.object(Postgresql, 'is_leader', Mock(return_value=True))
+@patch.object(Postgresql, 'xlog_position', Mock(return_value=0))
+@patch.object(Postgresql, 'call_nowait', Mock(return_value=True))
+@patch.object(Postgresql, 'data_directory_empty', Mock(return_value=False))
+@patch.object(Postgresql, 'controldata', Mock(return_value={'Database system identifier': '1234567890'}))
+@patch.object(Postgresql, 'sync_replication_slots', Mock())
+@patch.object(Postgresql, 'write_pg_hba', Mock())
+@patch.object(Postgresql, 'write_pgpass', Mock())
+@patch.object(Postgresql, 'write_recovery_conf', Mock())
+@patch.object(Postgresql, 'query', Mock())
+@patch.object(Postgresql, 'checkpoint', Mock())
+@patch('subprocess.call', Mock(return_value=0))
 class TestHa(unittest.TestCase):
 
     @patch('socket.getaddrinfo', socket_getaddrinfo)
     def setUp(self):
         with patch.object(Client, 'machines') as mock_machines:
             mock_machines.__get__ = Mock(return_value=['http://remotehost:2379'])
-            self.p = MockPostgresql()
+            self.p = Postgresql({'name': 'postgresql0', 'scope': 'dummy', 'listen': '127.0.0.1:5432',
+                                 'data_dir': 'data/postgresql0', 'superuser': {}, 'admin': {},
+                                 'replication': {'username': '', 'password': '', 'network': ''}})
+            self.p.set_state('running')
+            self.p.check_replication_lag = true
             self.p.can_create_replica_without_leader = MagicMock(return_value=False)
             self.e = Etcd('foo', {'ttl': 30, 'host': 'ok:2379', 'scope': 'test'})
             self.e.client.read = etcd_read
             self.e.client.write = etcd_write
-            self.e.client.delete = Mock(side_effect=etcd.EtcdException())
+            self.e.client.delete = Mock(side_effect=EtcdException())
             self.ha = Ha(MockPatroni(self.p, self.e))
             self.ha._async_executor.run_async = run_async
             self.ha.old_cluster = self.e.get_cluster()
@@ -154,7 +122,7 @@ class TestHa(unittest.TestCase):
         self.p.is_healthy = false
         self.p.is_running = false
         self.ha.has_lock = true
-        self.p.role = 'master'
+        self.p.set_role('master')
         self.p.controldata = lambda: {'Database cluster state': 'in production'}
         self.assertEquals(self.ha.run_cycle(), 'started as readonly because i had the session lock')
         self.assertEquals(self.ha.run_cycle(), 'removed leader key after trying and failing to start postgres')
@@ -302,57 +270,60 @@ class TestHa(unittest.TestCase):
         self.ha.has_lock = true
         self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', '', None))
         self.assertEquals(self.ha.run_cycle(), 'no action.  i am the leader with the lock')
-        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, '', MockPostgresql.name, None))
+        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, '', self.p.name, None))
         self.assertEquals(self.ha.run_cycle(), 'no action.  i am the leader with the lock')
         self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, '', 'blabla', None))
         self.assertEquals(self.ha.run_cycle(), 'no action.  i am the leader with the lock')
-        f = Failover(0, MockPostgresql.name, '', None)
+        f = Failover(0, self.p.name, '', None)
         self.ha.cluster = get_cluster_initialized_with_leader(f)
         self.assertEquals(self.ha.run_cycle(), 'manual failover: demoting myself')
         self.ha.fetch_node_status = lambda e: (e, True, True, 0, {'nofailover': 'True'})
         self.assertEquals(self.ha.run_cycle(), 'no action.  i am the leader with the lock')
         # manual failover from the previous leader to us won't happen if we hold the nofailover flag
-        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', MockPostgresql.name, None))
+        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', self.p.name, None))
         self.assertEquals(self.ha.run_cycle(), 'no action.  i am the leader with the lock')
 
         # Failover scheduled time must include timezone
         scheduled = datetime.datetime.now()
-        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', MockPostgresql.name, scheduled))
+        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', self.p.name, scheduled))
         self.ha.run_cycle()
 
         scheduled = datetime.datetime.utcnow().replace(tzinfo=pytz.UTC)
-        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', MockPostgresql.name, scheduled))
+        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', self.p.name, scheduled))
         self.assertEquals('no action.  i am the leader with the lock', self.ha.run_cycle())
 
         scheduled = scheduled + datetime.timedelta(seconds=30)
-        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', MockPostgresql.name, scheduled))
+        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', self.p.name, scheduled))
         self.assertEquals('no action.  i am the leader with the lock', self.ha.run_cycle())
 
         scheduled = scheduled + datetime.timedelta(seconds=-600)
-        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', MockPostgresql.name, scheduled))
+        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', self.p.name, scheduled))
         self.assertEquals('no action.  i am the leader with the lock', self.ha.run_cycle())
 
         scheduled = None
-        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', MockPostgresql.name, scheduled))
+        self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', self.p.name, scheduled))
         self.assertEquals('no action.  i am the leader with the lock', self.ha.run_cycle())
 
     @patch('requests.get', requests_get)
     def test_manual_failover_process_no_leader(self):
         self.p.is_leader = false
-        self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', MockPostgresql.name, None))
+        self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', self.p.name, None))
         self.assertEquals(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock')
         self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'leader', None))
+        self.p.set_role('replica')
         self.assertEquals(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock')
         self.ha.fetch_node_status = lambda e: (e, True, True, 0, {})  # accessible, in_recovery
         self.assertEquals(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node')
-        self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, MockPostgresql.name, '', None))
+        self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, self.p.name, '', None))
         self.assertEquals(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node')
         self.ha.fetch_node_status = lambda e: (e, False, True, 0, {})  # inaccessible, in_recovery
+        self.p.set_role('replica')
         self.assertEquals(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock')
         # set failover flag to True for all members of the cluster
         # this should elect the current member, as we are not going to call the API for it.
         self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'other', None))
         self.ha.fetch_node_status = lambda e: (e, True, True, 0, {'nofailover': 'True'})  # accessible, in_recovery
+        self.p.set_role('replica')
         self.assertEquals(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock')
         # same as previous, but set the current member to nofailover. In no case it should be elected as a leader
         self.ha.patroni.nofailover = True

tests/test_patroni.py

@@ -15,10 +15,6 @@ from test_postgresql import Postgresql, psycopg2_connect
 from test_zookeeper import MockKazooClient
 
 
-def time_sleep(*args):
-    raise SleepException()
-
-
 @patch('time.sleep', Mock())
 @patch('subprocess.call', Mock(return_value=0))
 @patch('psycopg2.connect', psycopg2_connect)
@@ -62,7 +58,7 @@ class TestPatroni(unittest.TestCase):
 
     @patch('time.sleep', Mock(side_effect=SleepException()))
     def test_run(self):
-        self.p.ha.dcs.watch = time_sleep
+        self.p.ha.dcs.watch = Mock(side_effect=SleepException())
         self.assertRaises(SleepException, self.p.run)
 
         self.p.ha.state_handler.is_leader = Mock(return_value=False)

tests/test_postgresql.py

@@ -2,21 +2,16 @@ import mock  # for the mock.call method, importing it without a namespace breaks
 import os
 import psycopg2
 import shutil
+import subprocess
 import unittest
 
-from six.moves import builtins
 from mock import Mock, MagicMock, PropertyMock, patch, mock_open
 from patroni.dcs import Cluster, Leader, Member
 from patroni.exceptions import PostgresException, PostgresConnectionException
 from patroni.postgresql import Postgresql
 from patroni.utils import RetryFailedError
+from six.moves import builtins
 from test_ha import false
-import subprocess
-
-
-def is_file_raise_on_backup(*args, **kwargs):
-    if args[0].endswith('.backup'):
-        raise Exception("foo")
 
 
 class MockCursor(object):
@@ -283,7 +278,7 @@ class TestPostgresql(unittest.TestCase):
         self.p.pg_rewind = tmp
         with mock.patch('subprocess.call', MagicMock(return_value=1)):
             self.assertFalse(self.p.can_rewind)
-        with mock.patch('subprocess.call', side_effect=OSError("foo")):
+        with mock.patch('subprocess.call', side_effect=OSError):
             self.assertFalse(self.p.can_rewind)
         tmp = self.p.controldata
         self.p.controldata = lambda: {'wal_log_hints setting': 'on'}
@@ -292,7 +287,7 @@ class TestPostgresql(unittest.TestCase):
 
     @patch('time.sleep', Mock())
     def test_create_replica(self):
-        self.p.delete_trigger_file = Mock(side_effect=OSError())
+        self.p.delete_trigger_file = Mock(side_effect=OSError)
         with patch('subprocess.call', Mock(side_effect=[1, 0])):
             self.assertEquals(self.p.create_replica(self.leader, ''), 0)
         with patch('subprocess.call', Mock(side_effect=[Exception(), 0])):
@@ -349,7 +344,7 @@ class TestPostgresql(unittest.TestCase):
     def test_last_operation(self):
         self.assertEquals(self.p.last_operation(), '0')
 
-    @patch('subprocess.Popen', Mock(side_effect=OSError()))
+    @patch('subprocess.Popen', Mock(side_effect=OSError))
     def test_call_nowait(self):
         self.assertFalse(self.p.call_nowait('on_start'))
 
@@ -369,7 +364,7 @@ class TestPostgresql(unittest.TestCase):
     def test_move_data_directory(self):
         self.p.is_running = false
         self.p.move_data_directory()
-        with patch('os.rename', Mock(side_effect=OSError())):
+        with patch('os.rename', Mock(side_effect=OSError)):
             self.p.move_data_directory()
 
     @patch('patroni.postgresql.Postgresql.write_pgpass', MagicMock(return_value=dict()))
@@ -411,13 +406,10 @@ class TestPostgresql(unittest.TestCase):
             self.assertEquals(int(data['max_replication_slots']), 5)
             self.assertEqual(data.get('D'), None)
 
-            m.side_effect = IOError("foo")
+            m.side_effect = IOError
             data = self.p.read_postmaster_opts()
             self.assertEqual(data, dict())
 
-            m.side_effect = Exception("foo")
-            self.assertRaises(Exception, self.p.read_postmaster_opts())
-
     @patch('subprocess.Popen')
     @patch.object(builtins, 'open', MagicMock(return_value=42))
     def test_single_user_mode(self, subprocess_popen_mock):

tests/test_utils.py

@@ -16,14 +16,14 @@ class TestUtils(unittest.TestCase):
 
     @patch('time.sleep', Mock())
     def test_reap_children(self):
-        reap_children()
+        self.assertIsNone(reap_children())
         with patch('os.waitpid', Mock(return_value=(0, 0))):
             sigchld_handler(None, None)
-            reap_children()
+            self.assertIsNone(reap_children())
 
     @patch('time.sleep', time_sleep)
     def test_sleep(self):
-        sleep(0.01)
+        self.assertIsNone(sleep(0.01))
 
 
 @patch('time.sleep', Mock())