From 220cacd95f5932567856ab5ca1726d0c12e9e196 Mon Sep 17 00:00:00 2001 From: Polina Bungina <27892524+hughcapet@users.noreply.github.com> Date: Tue, 26 Sep 2023 15:52:00 +0200 Subject: [PATCH] Don't call socket functions from tests (#2886) We used to call `socket` module's functions from the config_generator tests to later compare with the output produced by --generate-config. That however sometimes ends up with the whole test module failure if gethostname() returned None. Also includes a little code deduplication (NO_VALUE_MSG imported directly from the config_generator module) and removes debug maxDiff option --- patroni/config_generator.py | 24 +++++++++++------------ tests/test_config_generator.py | 36 +++++++++++++++------------------- 2 files changed, 28 insertions(+), 32 deletions(-) diff --git a/patroni/config_generator.py b/patroni/config_generator.py index 0269c49a..4bcb2819 100644 --- a/patroni/config_generator.py +++ b/patroni/config_generator.py @@ -38,7 +38,7 @@ _AUTH_ALLOWED_PARAMETERS_MAPPING = { 'gssencmode': 'PGGSSENCMODE', 'channel_binding': 'PGCHANNELBINDING' } -_NO_VALUE_MSG = '#FIXME' +NO_VALUE_MSG = '#FIXME' def get_address() -> Tuple[str, str]: @@ -50,7 +50,7 @@ def get_address() -> Tuple[str, str]: :returns: tuple consisting of the hostname returned by :func:`~socket.gethostname` and the first element in the sorted list of the addresses returned by :func:`~socket.getaddrinfo`. Sorting guarantees it will prefer IPv4. - If an exception occured, hostname and ip values are equal to :data:`~patroni.config_generator._NO_VALUE_MSG`. + If an exception occured, hostname and ip values are equal to :data:`~patroni.config_generator.NO_VALUE_MSG`. """ hostname = None try: @@ -59,7 +59,7 @@ def get_address() -> Tuple[str, str]: key=lambda x: x[0])[0][4][0] except Exception as err: logging.warning('Failed to obtain address: %r', err) - return _NO_VALUE_MSG, _NO_VALUE_MSG + return NO_VALUE_MSG, NO_VALUE_MSG class AbstractConfigGenerator(abc.ABC): @@ -88,24 +88,24 @@ class AbstractConfigGenerator(abc.ABC): """Generate a template config for further extension (e.g. in the inherited classes). :returns: dictionary with the values gathered from Patroni env, hopefully defined hostname and ip address - (otherwise set to :data:`~patroni.config_generator._NO_VALUE_MSG`), and some sane defaults. + (otherwise set to :data:`~patroni.config_generator.NO_VALUE_MSG`), and some sane defaults. """ template_config: Dict[str, Any] = { - 'scope': _NO_VALUE_MSG, + 'scope': NO_VALUE_MSG, 'name': cls._HOSTNAME, 'postgresql': { - 'data_dir': _NO_VALUE_MSG, - 'connect_address': _NO_VALUE_MSG + ':5432', - 'listen': _NO_VALUE_MSG + ':5432', + 'data_dir': NO_VALUE_MSG, + 'connect_address': NO_VALUE_MSG + ':5432', + 'listen': NO_VALUE_MSG + ':5432', 'bin_dir': '', 'authentication': { 'superuser': { 'username': 'postgres', - 'password': _NO_VALUE_MSG + 'password': NO_VALUE_MSG }, 'replication': { 'username': 'replicator', - 'password': _NO_VALUE_MSG + 'password': NO_VALUE_MSG } } }, @@ -185,7 +185,7 @@ class SampleConfigGenerator(AbstractConfigGenerator): self.config['bootstrap']['dcs']['postgresql']['use_pg_rewind'] = True if self.pg_major >= 110000: self.config['postgresql']['authentication'].setdefault( - 'rewind', {'username': 'rewind_user'}).setdefault('password', _NO_VALUE_MSG) + 'rewind', {'username': 'rewind_user'}).setdefault('password', NO_VALUE_MSG) class RunningClusterConfigGenerator(AbstractConfigGenerator): @@ -335,7 +335,7 @@ class RunningClusterConfigGenerator(AbstractConfigGenerator): getpass('Please enter the user password:') self.config['postgresql']['authentication'] = { 'superuser': su_params, - 'replication': {'username': _NO_VALUE_MSG, 'password': _NO_VALUE_MSG} + 'replication': {'username': NO_VALUE_MSG, 'password': NO_VALUE_MSG} } def _set_conf_files(self) -> None: diff --git a/tests/test_config_generator.py b/tests/test_config_generator.py index 49799c01..e702f957 100644 --- a/tests/test_config_generator.py +++ b/tests/test_config_generator.py @@ -1,6 +1,5 @@ import os import psutil -import socket import unittest from . import MockConnect, MockCursor, MockConnectionInfo @@ -9,28 +8,25 @@ from mock import MagicMock, Mock, PropertyMock, mock_open, patch from patroni.__main__ import main as _main from patroni.config import Config -from patroni.config_generator import AbstractConfigGenerator, get_address - +from patroni.config_generator import AbstractConfigGenerator, get_address, NO_VALUE_MSG from patroni.utils import patch_config from . import psycopg_connect +HOSTNAME = 'test_hostname' +IP = '1.9.8.4' + @patch('patroni.psycopg.connect', psycopg_connect) -@patch('socket.getaddrinfo', Mock(return_value=[(0, 0, 0, 0, ('1.9.8.4', 1984))])) @patch('builtins.open', MagicMock()) @patch('subprocess.check_output', Mock(return_value=b"postgres (PostgreSQL) 16.2")) @patch('psutil.Process.exe', Mock(return_value='/bin/dir/from/running/postgres')) @patch('psutil.Process.__init__', Mock(return_value=None)) +@patch.object(AbstractConfigGenerator, '_HOSTNAME', HOSTNAME) +@patch.object(AbstractConfigGenerator, '_IP', IP) class TestGenerateConfig(unittest.TestCase): - no_value_msg = '#FIXME' - _HOSTNAME = socket.gethostname() - _IP = sorted(socket.getaddrinfo(_HOSTNAME, 0, socket.AF_UNSPEC, socket.SOCK_STREAM, 0), key=lambda x: x[0])[0][4][0] - def setUp(self): - self.maxDiff = None - os.environ['PATRONI_SCOPE'] = 'scope_from_env' os.environ['PATRONI_POSTGRESQL_BIN_DIR'] = '/bin/from/env' os.environ['PATRONI_SUPERUSER_USERNAME'] = 'su_user_from_env' @@ -54,14 +50,14 @@ class TestGenerateConfig(unittest.TestCase): self.config = { 'scope': self.environ['PATRONI_SCOPE'], - 'name': self._HOSTNAME, + 'name': HOSTNAME, 'bootstrap': { 'dcs': dynamic_config }, 'postgresql': { - 'connect_address': self.no_value_msg + ':5432', - 'data_dir': self.no_value_msg, - 'listen': self.no_value_msg + ':5432', + 'connect_address': NO_VALUE_MSG + ':5432', + 'data_dir': NO_VALUE_MSG, + 'listen': NO_VALUE_MSG + ':5432', 'pg_hba': ['host all all all md5', f'host replication {self.environ["PATRONI_REPLICATION_USERNAME"]} all md5'], 'authentication': {'superuser': {'username': self.environ['PATRONI_SUPERUSER_USERNAME'], @@ -99,7 +95,7 @@ class TestGenerateConfig(unittest.TestCase): } }, 'postgresql': { - 'connect_address': f'{self._IP}:bar', + 'connect_address': f'{IP}:bar', 'listen': '6.6.6.6:1984', 'data_dir': 'data', 'bin_dir': '/bin/dir/from/running', @@ -118,8 +114,8 @@ class TestGenerateConfig(unittest.TestCase): 'sslmode': 'prefer' }, 'replication': { - 'username': self.no_value_msg, - 'password': self.no_value_msg + 'username': NO_VALUE_MSG, + 'password': NO_VALUE_MSG }, 'rewind': None }, @@ -179,7 +175,7 @@ class TestGenerateConfig(unittest.TestCase): 'authentication': { 'rewind': { 'username': self.environ['PATRONI_REWIND_USERNAME'], - 'password': self.no_value_msg} + 'password': NO_VALUE_MSG} }, } } @@ -230,7 +226,7 @@ class TestGenerateConfig(unittest.TestCase): } }, 'postgresql': { - 'connect_address': f'{self._IP}:1984', + 'connect_address': f'{IP}:1984', 'authentication': { 'superuser': { 'username': self.environ['PGUSER'], @@ -330,5 +326,5 @@ class TestGenerateConfig(unittest.TestCase): def test_get_address(self): with patch('socket.getaddrinfo', Mock(side_effect=Exception)), \ patch('logging.warning') as mock_warning: - self.assertEqual(get_address(), (self.no_value_msg, self.no_value_msg)) + self.assertEqual(get_address(), (NO_VALUE_MSG, NO_VALUE_MSG)) self.assertIn('Failed to obtain address: %r', mock_warning.call_args_list[0][0])