From b451cff890538a8f1a0085d39402a5fb78e63ee1 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Fri, 4 Apr 2025 11:37:30 +0200 Subject: [PATCH] Make FakeConfiguration a representative configuration object. As of this commit fake configuration instances are populated with the same properties, including the same default values, as a genuine Configuration object instance. This ensures that logic under test is going to behave far more as it would with a real configuration object which means a great deal more assurance in the tests. Achieve this by using the dictionary of defaults that was split out previously to initialize the FakeConfiguration which itself is now a SimpleNamespace. Making it a namespace ensures that attribute lookup, something that normal Configuration objects support, work correctly but in addition forces the attributes to be set "up front". This keeps us honest in the properties we expose. Since a FakeConfiguration needs to track the real Configuration we also prevent the addition of attributes that not keys of a real configuration. Unfortunarely it seems that a lot of properties are set dynamically as part of loading a configuration, but laythe first steps to a canonical configuration object by making a couple of properties used by existing tests static; existing defaults are re-used to avoid functional change. --- mig/shared/compat.py | 13 +++++ mig/shared/configuration.py | 37 ++++++++++--- .../mig_shared_configuration--new.json | 4 ++ tests/support/configsupp.py | 48 +++++++++++++--- tests/test_mig_shared_configuration.py | 16 ++++-- tests/test_tests_support_configsupp.py | 55 +++++++++++++++++++ 6 files changed, 151 insertions(+), 22 deletions(-) create mode 100644 tests/test_tests_support_configsupp.py diff --git a/mig/shared/compat.py b/mig/shared/compat.py index 01069ce43..a9fe5a34d 100644 --- a/mig/shared/compat.py +++ b/mig/shared/compat.py @@ -34,6 +34,7 @@ from past.builtins import basestring import codecs +import inspect import io import sys # NOTE: StringIO is only available in python2 @@ -55,6 +56,9 @@ def __getattribute__(self, name): return dict(**self) return self[name] + + def __setattr__(self, name, value): + self[name] = value else: from types import SimpleNamespace @@ -93,6 +97,15 @@ def ensure_native_string(string_or_bytes): return textual_output +def inspect_args(func): + """Wrapper to return the arguments of a function.""" + + if PY2: + return inspect.getargspec(func).args + else: + return inspect.getfullargspec(func).args + + def NativeStringIO(initial_value=''): """Mock StringIO pseudo-class to create a StringIO matching the native string coding form. That is a BytesIO with utf8 on python 2 and unicode diff --git a/mig/shared/configuration.py b/mig/shared/configuration.py index 5dd5b145b..4e9613b23 100644 --- a/mig/shared/configuration.py +++ b/mig/shared/configuration.py @@ -59,6 +59,7 @@ # NOTE: protect migrid import from autopep8 reordering try: from mig.shared.base import force_native_str + from mig.shared.compat import inspect_args from mig.shared.defaults import CSRF_MINIMAL, CSRF_WARN, CSRF_MEDIUM, \ CSRF_FULL, POLICY_NONE, POLICY_WEAK, POLICY_MEDIUM, POLICY_HIGH, \ POLICY_MODERN, POLICY_CUSTOM, freeze_flavors, cert_field_order, \ @@ -73,6 +74,17 @@ print("could not import migrid modules") +_CONFIGURATION_NOFORWARD_KEYS = set([ + 'self', + 'config_file', + 'mig_server_id', + 'disable_auth_log', + 'skip_log', + 'verbose', + 'logger', +]) + + def expand_external_sources(logger, val): """Expand a string containing ENV::NAME, FILE::PATH or FILE::PATH$$CACHE references to fill in the content of the corresponding environment, file or @@ -392,6 +404,10 @@ def fix_missing(config_file, verbose=True): fd.close() +def _without_noforward_keys(d): + return { k: v for k, v in d.items() if k not in _CONFIGURATION_NOFORWARD_KEYS } + + class NativeConfigParser(ConfigParser): """Wraps configparser.ConfigParser to force get method to return native string instead of always returning unicode. @@ -426,6 +442,7 @@ def get(self, *args, **kwargs): 'ca_smtp': '', 'ca_user': 'mig-ca', 'resource_home': '', + 'short_title': 'MiG', 'vgrid_home': '', 'vgrid_public_base': '', 'vgrid_private_base': '', @@ -470,6 +487,7 @@ def get(self, *args, **kwargs): 'workflows_vgrid_patterns_home': '', 'workflows_vgrid_recipes_home': '', 'workflows_vgrid_history_home': '', + 'site_user_id_format': DEFAULT_USER_ID_FORMAT, 'site_prefer_python3': False, 'site_autolaunch_page': '', 'site_landing_page': '', @@ -681,6 +699,7 @@ def get(self, *args, **kwargs): 'expire_peer': 600, 'language': ['English'], 'user_interface': ['V2', 'V3'], + 'new_user_default_ui': 'V2', 'submitui': ['fields', 'textarea', 'files'], # Init user default page with no selection to use site landing page 'default_page': [''], @@ -703,6 +722,8 @@ def get(self, *args, **kwargs): # fyrgrid, benedict. Otherwise, ldap://bla.bla:2135/... 'arc_clusters': [], + + 'cloud_services': [], } @@ -893,8 +914,6 @@ def reload_config(self, verbose, skip_log=False, disable_auth_log=False, self.site_title = "Minimum intrusion Grid" if config.has_option('SITE', 'short_title'): self.short_title = config.get('SITE', 'short_title') - else: - self.short_title = "MiG" if config.has_option('SITE', 'user_interface'): self.user_interface = config.get( 'SITE', 'user_interface').split() @@ -904,8 +923,6 @@ def reload_config(self, verbose, skip_log=False, disable_auth_log=False, if config.has_option('SITE', 'new_user_default_ui'): self.new_user_default_ui = config.get( 'SITE', 'new_user_default_ui').strip() - else: - self.new_user_default_ui = self.user_interface[0] if config.has_option('GLOBAL', 'state_path'): self.state_path = config.get('GLOBAL', 'state_path') @@ -1682,7 +1699,6 @@ def reload_config(self, verbose, skip_log=False, disable_auth_log=False, for option in config.options(section)}) - self.cloud_services = [] # List of service options with default and override map override_map_keys = ['service_user', 'service_max_user_instances', 'service_image_alias', 'service_allowed_images', @@ -1693,6 +1709,7 @@ def reload_config(self, verbose, skip_log=False, disable_auth_log=False, 'service_jumphost_address', 'service_jumphost_user', 'service_jumphost_key'] + # Load generated cloud sections for section in config.sections(): if 'CLOUD_' in section: @@ -1913,8 +1930,6 @@ def reload_config(self, verbose, skip_log=False, disable_auth_log=False, logger.warning("invalid user_id_format %r - using default" % self.site_user_id_format) self.site_user_id_format = DEFAULT_USER_ID_FORMAT - else: - self.site_user_id_format = DEFAULT_USER_ID_FORMAT if config.has_option('SITE', 'autolaunch_page'): self.site_autolaunch_page = config.get('SITE', 'autolaunch_page') else: @@ -2742,6 +2757,14 @@ def parse_peers(self, peerfile): peerfile) return peers_dict + @staticmethod + def as_dict(thing): + assert isinstance(thing, Configuration) + return _without_noforward_keys(thing.__dict__) + + +_CONFIGURATION_ARGUMENTS = set(_CONFIGURATION_DEFAULTS.keys()) - _CONFIGURATION_NOFORWARD_KEYS + if '__main__' == __name__: conf = Configuration(os.path.expanduser('~/mig/server/MiGserver.conf'), diff --git a/tests/fixture/mig_shared_configuration--new.json b/tests/fixture/mig_shared_configuration--new.json index e5740118c..e435b49ca 100644 --- a/tests/fixture/mig_shared_configuration--new.json +++ b/tests/fixture/mig_shared_configuration--new.json @@ -29,6 +29,7 @@ "ca_smtp": "", "ca_user": "mig-ca", "certs_path": "/some/place/certs", + "cloud_services": [], "config_file": null, "cputime_for_empty_jobs": 0, "default_page": [ @@ -130,6 +131,7 @@ "min_seconds_between_live_update_requests": 0, "mrsl_files_dir": "", "myfiles_py_location": "", + "new_user_default_ui": "V2", "notify_home": "", "openid_store": "", "paraview_home": "", @@ -154,6 +156,7 @@ "sessid_to_jupyter_mount_link_home": "", "sessid_to_mrsl_link_home": "", "sharelink_home": "", + "short_title": "MiG", "site_advanced_vgrid_links": [], "site_autolaunch_page": "", "site_cloud_access": [ @@ -187,6 +190,7 @@ "extcert" ], "site_skin": "", + "site_user_id_format": "X509", "site_vgrid_creators": [ [ "distinguished_name", diff --git a/tests/support/configsupp.py b/tests/support/configsupp.py index a1c5c700a..047e71098 100644 --- a/tests/support/configsupp.py +++ b/tests/support/configsupp.py @@ -29,19 +29,49 @@ from tests.support.loggersupp import FakeLogger +from mig.shared.compat import SimpleNamespace +from mig.shared.configuration import _without_noforward_keys, \ + _CONFIGURATION_ARGUMENTS, _CONFIGURATION_DEFAULTS + + +def _generate_namespace_kwargs(): + d = dict(_CONFIGURATION_DEFAULTS) + d['logger'] = None + return d + + +def _ensure_only_configuration_keys(d): + """Check the dictionary arguments contains only premitted keys.""" + + unknown_keys = set(d.keys()) - set(_CONFIGURATION_ARGUMENTS) + assert len(unknown_keys) == 0, \ + "non-Configuration keys: %s" % (', '.join(unknown_keys),) + + +class FakeConfiguration(SimpleNamespace): + """A simple helper to pretend we have a Configuration object populated + with defaults overlaid with any explicitly supplied attributes. -class FakeConfiguration: - """A simple helper to pretend we have a real Configuration object with any - required attributes explicitly passed. Automatically attaches a FakeLogger instance if no logger is provided in kwargs. """ def __init__(self, **kwargs): - """Initialise instance attributes to be any named args provided and a - FakeLogger instance attached if not provided. + """Initialise instance attributes based on the defaults plus any + supplied additional options. """ - self.__dict__.update(kwargs) - if not 'logger' in self.__dict__: - dummy_logger = FakeLogger() - self.__dict__.update({'logger': dummy_logger}) + + SimpleNamespace.__init__(self, **_generate_namespace_kwargs()) + + if kwargs: + _ensure_only_configuration_keys(kwargs) + for k, v in kwargs.items(): + setattr(self, k, v) + + if 'logger' not in kwargs: + self.logger = FakeLogger() + + @staticmethod + def as_dict(thing): + assert isinstance(thing, FakeConfiguration) + return _without_noforward_keys(thing.__dict__) diff --git a/tests/test_mig_shared_configuration.py b/tests/test_mig_shared_configuration.py index a9609a12a..1e6152f67 100644 --- a/tests/test_mig_shared_configuration.py +++ b/tests/test_mig_shared_configuration.py @@ -32,21 +32,25 @@ import unittest from tests.support import MigTestCase, TEST_DATA_DIR, PY2, testmain -from mig.shared.configuration import Configuration - - -def _is_method(value): - return type(value).__name__ == 'method' +from mig.shared.configuration import Configuration, \ + _CONFIGURATION_ARGUMENTS, _CONFIGURATION_DEFAULTS def _to_dict(obj): return {k: v for k, v in inspect.getmembers(obj) - if not (k.startswith('__') or _is_method(v))} + if not (k.startswith('__') or inspect.ismethod(v) or inspect.isfunction(v))} class MigSharedConfiguration(MigTestCase): """Wrap unit tests for the corresponding module""" + def test_consistent_parameters(self): + configuration_defaults_keys = set(_CONFIGURATION_DEFAULTS.keys()) + mismatched = _CONFIGURATION_ARGUMENTS - configuration_defaults_keys + + self.assertEqual(len(mismatched), 0, + "configuration defaults do not match arguments") + def test_argument_storage_protocols(self): test_conf_file = os.path.join( TEST_DATA_DIR, 'MiGserver--customised.conf') diff --git a/tests/test_tests_support_configsupp.py b/tests/test_tests_support_configsupp.py new file mode 100644 index 000000000..241d1a877 --- /dev/null +++ b/tests/test_tests_support_configsupp.py @@ -0,0 +1,55 @@ +# -*- coding: utf-8 -*- +# +# --- BEGIN_HEADER --- +# +# test_tests_support_configsupp - unit test of the corresponding tests module +# Copyright (C) 2003-2024 The MiG Project by the Science HPC Center at UCPH +# +# This file is part of MiG. +# +# MiG is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# MiG is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, +# USA. +# +# --- END_HEADER --- +# + +"""Unit tests for the tests module pointed to in the filename""" + +from tests.support import MigTestCase, testmain +from tests.support.configsupp import FakeConfiguration + +from mig.shared.configuration import Configuration, \ + _CONFIGURATION_ARGUMENTS, _CONFIGURATION_DEFAULTS, \ + _CONFIGURATION_NOFORWARD_KEYS, _without_noforward_keys + + +class MigSharedInstall_FakeConfiguration(MigTestCase): + def test_consistent_parameters(self): + default_configuration = Configuration(None) + fake_configuration = FakeConfiguration() + + self.maxDiff = None + self.assertEqual( + Configuration.as_dict(default_configuration), + FakeConfiguration.as_dict(fake_configuration) + ) + + def test_only_configuration_keys(self): + with self.assertRaises(AssertionError): + FakeConfiguration(bar='1') + + +if __name__ == '__main__': + testmain()