diff --git a/mig/server/grid_notify.py b/mig/server/grid_notify.py index b74c114ef..75edc0e01 100755 --- a/mig/server/grid_notify.py +++ b/mig/server/grid_notify.py @@ -130,12 +130,10 @@ def send_notifications(configuration): notify_message = "Found %s new events since: %s\n\n" \ % (total_events, timestr) \ + notify_message - status = send_email( + status = send_email(configuration, recipient, subject, - notify_message, - logger, - configuration) + notify_message) if status: logger.info("Send email with %s events to: %s" % (total_events, recipient)) diff --git a/mig/shared/conf.py b/mig/shared/conf.py index 0de37b321..2e2354a1f 100644 --- a/mig/shared/conf.py +++ b/mig/shared/conf.py @@ -32,17 +32,59 @@ import os import sys +from mig.shared.configuration import Configuration from mig.shared.defaults import MIG_ENV from mig.shared.fileio import unpickle -def get_configuration_object(config_file=None, skip_log=False, - disable_auth_log=False): +class RuntimeConfiguration: + """A proxy object to be passed in-place of a Configuration which can be + extended with information relevant only at runtime. + + Given Configuration objects are threaded into and through almost all + the necessary codepaths to make this information available, they are an + attractive place to put this - but a Configuration is currently loaded + from static per-site data. + + Resolve this dichotomy with this class - a Configuration instance will + continue to represent the static data while an object that proxies its + attributes and thus is entirely drop-in compatible with it can be handed + to callers without being mixed in with the statuc data. + """ + + def __init__(self, configuration): + object.__setattr__(self, '_configuration', configuration) + object.__setattr__(self, '_context', {}) + + def __delattr__(self, attr): + return delattr(self._configuration, attr) + + def __getattr__(self, attr): + return getattr(self._configuration, attr) + + def __setattr__(self, attr, value): + return setattr(self._configuration, attr, value) + + def context_get(self, context_key): + return self._context.get(context_key, None) + + def context_set(self, context_key, context_value): + self._context[context_key] = context_value + + @classmethod + def is_runtime_configuration(cls, obj): + """Is the given object a runtime configuration.""" + return isinstance(obj, cls) + + +def get_configuration_object(config_file=None, + skip_log=False, + disable_auth_log=False, + _raw=False): """Simple helper to call the general configuration init. Optional skip_log and disable_auth_log arguments are passed on to allow skipping the default log initialization and disabling auth log for unit tests. """ - from mig.shared.configuration import Configuration if config_file: _config_file = config_file elif os.environ.get('MIG_CONF', None): @@ -65,7 +107,11 @@ def get_configuration_object(config_file=None, skip_log=False, configuration = Configuration(_config_file, False, skip_log, disable_auth_log) - return configuration + + if _raw: + return configuration + + return RuntimeConfiguration(configuration) def get_resource_configuration(resource_home, unique_resource_name, diff --git a/mig/shared/configuration.py b/mig/shared/configuration.py index 09a869905..006812950 100644 --- a/mig/shared/configuration.py +++ b/mig/shared/configuration.py @@ -83,6 +83,7 @@ 'skip_log', 'verbose', 'logger', + '_context', ]) @@ -2921,6 +2922,10 @@ def parse_peers(self, peerfile): peerfile) return peers_dict + @classmethod + def create(cls, *args, **kwargs): + return cls(*args, **kwargs) + @staticmethod def is_configuration_like(obj): """Does the given object quack like a MiG Configuration.""" diff --git a/mig/shared/functionality/autocreate.py b/mig/shared/functionality/autocreate.py index 9d704e81c..5eccd55a2 100644 --- a/mig/shared/functionality/autocreate.py +++ b/mig/shared/functionality/autocreate.py @@ -740,8 +740,7 @@ def main(client_id, user_arguments_dict, environ=None): logger.info('Send email: to: %s, header: %s, smtp_server: %s' % (email, email_header, smtp_server)) logger.debug('email body: %s' % email_msg) - if not send_email(email, email_header, email_msg, logger, - configuration): + if not send_email(configuration, email, email_header, email_msg): output_objects.append({ 'object_type': 'error_text', 'text': """An error occurred trying to send your account welcome email. Please contact support (%s) and include the diff --git a/mig/shared/functionality/extcertaction.py b/mig/shared/functionality/extcertaction.py index fb7a63010..31f5588d9 100644 --- a/mig/shared/functionality/extcertaction.py +++ b/mig/shared/functionality/extcertaction.py @@ -316,8 +316,7 @@ def main(client_id, user_arguments_dict): logger.info('Sending email: to: %s, header: %s, msg: %s, smtp_server: %s' % (admin_email, email_header, email_msg, smtp_server)) - if not send_email(admin_email, email_header, email_msg, logger, - configuration): + if not send_email(configuration, admin_email, email_header, email_msg): output_objects.append({'object_type': 'error_text', 'text': """An error occurred trying to inform the site admins about your request for existing certificate sign up. Please contact %s site diff --git a/mig/shared/functionality/extoidaction.py b/mig/shared/functionality/extoidaction.py index cd73ba292..fba2ece57 100644 --- a/mig/shared/functionality/extoidaction.py +++ b/mig/shared/functionality/extoidaction.py @@ -282,8 +282,7 @@ def main(client_id, user_arguments_dict): logger.info('Sending email: to: %s, header: %s, msg: %s, smtp_server: %s' % (admin_email, email_header, email_msg, smtp_server)) - if not send_email(admin_email, email_header, email_msg, logger, - configuration): + if not send_email(configuration, admin_email, email_header, email_msg): output_objects.append({'object_type': 'error_text', 'text': """An error occurred trying to inform the site admins about your request for OpenID account access. Please contact %s site diff --git a/mig/shared/functionality/peersaction.py b/mig/shared/functionality/peersaction.py index bf27c3870..bb52599ca 100644 --- a/mig/shared/functionality/peersaction.py +++ b/mig/shared/functionality/peersaction.py @@ -309,8 +309,7 @@ def main(client_id, user_arguments_dict): logger.info('Sending invitation: to: %s, header: %s, msg: %s, smtp_server: %s' % (peer_email, email_header, email_msg, smtp_server)) - if send_email(peer_email, email_header, email_msg, logger, - configuration): + if send_email(configuration, peer_email, email_header, email_msg): succeeded.append(peer_email) else: failed.append(peer_email) @@ -369,8 +368,7 @@ def main(client_id, user_arguments_dict): logger.info('Sending email: to: %s, header: %s, msg: %s, smtp_server: %s' % (admin_email, email_header, email_msg, smtp_server)) - if not send_email(admin_email, email_header, email_msg, logger, - configuration): + if not send_email(configuration, admin_email, email_header, email_msg): output_objects.append({'object_type': 'error_text', 'text': ''' An error occurred trying to send the email about your %s peers to the site administrators. Please contact %s site support at %s or manually inform the diff --git a/mig/shared/functionality/reqcertaction.py b/mig/shared/functionality/reqcertaction.py index 4c5c6fb4d..b1d75a7b9 100644 --- a/mig/shared/functionality/reqcertaction.py +++ b/mig/shared/functionality/reqcertaction.py @@ -346,8 +346,7 @@ def main(client_id, user_arguments_dict): logger.info('Sending email: to: %s, header: %s, msg: %s, smtp_server: %s' % (admin_email, email_header, email_msg, smtp_server)) - if not send_email(admin_email, email_header, email_msg, logger, - configuration): + if not send_email(configuration, admin_email, email_header, email_msg): output_objects.append({'object_type': 'error_text', 'text': """An error occurred trying to inform the site admins about your request for certificate sign up. Please contact %s site diff --git a/mig/shared/functionality/reqoidaction.py b/mig/shared/functionality/reqoidaction.py index bbc05dc64..92fffa1ea 100644 --- a/mig/shared/functionality/reqoidaction.py +++ b/mig/shared/functionality/reqoidaction.py @@ -357,8 +357,7 @@ def main(client_id, user_arguments_dict): logger.info('Sending email: to: %s, header: %s, msg: %s, smtp_server: %s' % (admin_email, email_header, email_msg, smtp_server)) - if not send_email(admin_email, email_header, email_msg, logger, - configuration): + if not send_email(configuration, admin_email, email_header, email_msg): output_objects.append({'object_type': 'error_text', 'text': """An error occurred trying to inform the site admins about your request for OpenID account access. Please contact %s site diff --git a/mig/shared/functionality/reqpwresetaction.py b/mig/shared/functionality/reqpwresetaction.py index b63027a6f..96a9f5bfa 100644 --- a/mig/shared/functionality/reqpwresetaction.py +++ b/mig/shared/functionality/reqpwresetaction.py @@ -271,8 +271,7 @@ def main(client_id, user_arguments_dict): logger.info('Sending email: to: %s, header: %s, msg: %s, smtp_server: %s' % (email_to, email_header, email_msg, smtp_server)) - if not send_email(email_to, email_header, email_msg, logger, - configuration): + if not send_email(configuration, email_to, email_header, email_msg): output_objects.append({'object_type': 'error_text', 'text': """An error occurred trying to send the email for an account %s password reset request. Please contact %s site support at %s diff --git a/mig/shared/gdp/base.py b/mig/shared/gdp/base.py index ff5d68357..4c74ca3df 100644 --- a/mig/shared/gdp/base.py +++ b/mig/shared/gdp/base.py @@ -683,12 +683,10 @@ def __send_project_action_confirmation(configuration, Attached you'll find the details registered in relation to the operation. """ % mail_fill - status = send_email( + status = send_email(configuration, recipients, subject, message, - _logger, - configuration, files=[pdf_filepath], ) if status: diff --git a/mig/shared/notification.py b/mig/shared/notification.py index ca9af7912..49b189041 100644 --- a/mig/shared/notification.py +++ b/mig/shared/notification.py @@ -52,6 +52,7 @@ from mig.shared.base import force_utf8, generate_https_urls, extract_field, \ canonical_user_with_peers, cert_field_map, get_site_base_url +from mig.shared.conf import RuntimeConfiguration from mig.shared.defaults import email_keyword_list, job_output_dir, \ transfer_output_dir, keyword_auto, cert_auto_extend_days, \ oid_auto_extend_days @@ -561,11 +562,35 @@ def send_instant_message( def send_email( + configuration, + recipients, + subject, + message, + files=[], + custom_sender=None): + """Send email to recipients via the actively configured method.""" + + assert RuntimeConfiguration.is_runtime_configuration(configuration) + + notifier = configuration.context_get('notifier') + if not notifier: + notifier = Notifier(configuration) + configuration.context_set('notifier', notifier) + + return notifier.send_email( + recipients, + subject, + message, + files=files, + custom_sender=custom_sender, + ) + + +def direct_send_email( + configuration, recipients, subject, message, - logger, - configuration, files=[], custom_sender=None ): @@ -589,13 +614,14 @@ def send_email( """ _logger = configuration.logger + gpg_sign = False if configuration.site_gpg_passphrase is not None: if gnupg is None: - logger.warning("the gnupg module is required for gpg signing") + _logger.warning("the gnupg module is required for gpg signing") else: gpg_sign = True - logger.debug("enabling automatic gpg signing of email") + _logger.debug("enabling automatic gpg signing of email") if recipients.find(', ') > -1: recipients_list = recipients.split(', ') @@ -631,7 +657,7 @@ def send_email( basemsg = MIMEText(force_utf8(message), "plain", "utf8") mime_msg.attach(basemsg) if gpg_sign: - logger.info("signing message with gpg") + _logger.info("signing message with gpg") gpg = gnupg.GPG() basetext = basemsg.as_string().replace('\n', '\r\n') signature = gpg.sign(basetext, detach=True, @@ -643,7 +669,7 @@ def send_email( msg_sig.set_payload("%s" % signature) mime_msg.attach(msg_sig) else: - logger.warning("failed to create gnupg signature") + _logger.warning("failed to create gnupg signature") for name in files: part = MIMEBase('application', "octet-stream") @@ -652,7 +678,7 @@ def send_email( part.add_header('Content-Disposition', 'attachment', filename=os.path.basename(name)) mime_msg.attach(part) - logger.debug('sending email from %s to %s:\n%s' % + _logger.debug('sending email from %s to %s:\n%s' % (from_email, recipients, mime_msg.as_string())) server = smtplib.SMTP(configuration.smtp_server) server.set_debuglevel(0) @@ -664,10 +690,10 @@ def send_email( % errors) return False else: - logger.debug('Email was sent to %s' % recipients) + _logger.debug('Email was sent to %s' % recipients) return True except Exception as err: - logger.error('Sending email to %s through %s failed!: %s' + _logger.error('Sending email to %s through %s failed!: %s' % (recipients, configuration.smtp_server, err)) return False @@ -819,8 +845,8 @@ def notify_user( continue - if send_email(single_dest, header, message, logger, - configuration, custom_sender=email_sender): + if send_email(configuration, single_dest, header, message, + custom_sender=email_sender): logger.info('email sent to %s telling that %s %s' % (single_dest, jobid, status)) else: @@ -906,7 +932,7 @@ def send_resource_create_request_mail( os.path.basename(pending_file), ) - status = send_email(recipients, subject, txt, logger, configuration) + status = send_email(configuration, recipients, subject, txt) if status: msg += '\nEmail was sent to admins' else: @@ -961,3 +987,13 @@ def send_system_notification(user_id, category, message, configuration): return send_message_to_grid_notify(pickled_notification, configuration.logger, configuration) + + +class Notifier: + def __init__(self, configuration): + self.configuration = configuration + + def send_email(self, recipients, subject, message, files=[], custom_sender=None): + return direct_send_email(self.configuration, recipients, subject, message, + files=[], + custom_sender=None) diff --git a/tests/fixture/mig_shared_configuration--new.json b/tests/fixture/mig_shared_configuration--new.json index f98e271c3..5ab83c5c7 100644 --- a/tests/fixture/mig_shared_configuration--new.json +++ b/tests/fixture/mig_shared_configuration--new.json @@ -29,7 +29,6 @@ "ca_user": "mig-ca", "certs_path": "/some/place/certs", "cloud_services": [], - "config_file": null, "cputime_for_empty_jobs": 0, "default_page": [ "" @@ -104,14 +103,12 @@ ], "log_dir": "", "logfile": "", - "logger": null, "logger_obj": null, "loglevel": "", "lrmstypes": [], "mig_code_base": "", "mig_path": "/some/place/mig", "mig_server_home": "", - "mig_server_id": null, "mig_system_files": "", "mig_system_run": "", "mig_system_storage": "", diff --git a/tests/support/__init__.py b/tests/support/__init__.py index eb2c8019f..9dd002a76 100644 --- a/tests/support/__init__.py +++ b/tests/support/__init__.py @@ -48,6 +48,7 @@ from tests.support.suppconst import MIG_BASE, TEST_BASE, \ TEST_DATA_DIR, TEST_OUTPUT_DIR, ENVHELP_OUTPUT_DIR from tests.support.usersupp import UserAssertMixin +import tests.support.fakes as fakes from tests.support._env import MIG_ENV, PY2 @@ -212,11 +213,12 @@ def _make_configuration_instance(testcase, configuration_to_make): if configuration_to_make == 'fakeconfig': return FakeConfiguration(logger=testcase.logger) elif configuration_to_make == 'testconfig': - from mig.shared.conf import get_configuration_object - configuration = get_configuration_object(skip_log=True, - disable_auth_log=True) + from mig.shared.conf import get_configuration_object, RuntimeConfiguration + configuration = get_configuration_object(skip_log=True, + disable_auth_log=True, + _raw=True) configuration.logger = testcase.logger - return configuration + return RuntimeConfiguration(configuration) else: raise AssertionError( "MigTestCase: unknown configuration %r" % (configuration_to_make,)) @@ -250,6 +252,8 @@ def configuration(self): self._configuration = configuration_instance + fakes.instrument_test_case(self, self._configuration) + return configuration_instance @property diff --git a/tests/support/configsupp.py b/tests/support/configsupp.py index 0846e465d..9a81e897f 100644 --- a/tests/support/configsupp.py +++ b/tests/support/configsupp.py @@ -50,6 +50,7 @@ def _generate_namespace_kwargs(): properties_and_defaults = dict(_CONFIGURATION_PROPERTIES) properties_and_defaults['logger'] = None + properties_and_defaults['_context'] = {} return properties_and_defaults @@ -83,6 +84,12 @@ def __init__(self, logger=None, **kwargs): for k, v in kwargs.items(): setattr(self, k, v) + def context_get(self, context_key): + return self._context.get(context_key, None) + + def context_set(self, context_key, context_value): + self._context[context_key] = context_value + def reload_config(self, *args, **kwargs): """Stub defined to quack like Configuration.""" diff --git a/tests/support/fakes.py b/tests/support/fakes.py new file mode 100644 index 000000000..148810abc --- /dev/null +++ b/tests/support/fakes.py @@ -0,0 +1,98 @@ +# -*- coding: utf-8 -*- +# +# --- BEGIN_HEADER --- +# +# __init__ - package marker and core package functions +# Copyright (C) 2003-2026 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 --- +# + +import inspect +from types import SimpleNamespace + + +"""Fake implementations of various assistant functionality used by MiG logic.""" + +class FakeSendEmail: + def __init__(self): + self.calls = [] + self._checked = False + + def __call__(self, *args, **kwargs): + # Record any arguments upon a call to send email (discarding the + # leading configuration argument) + self.calls.append((args, kwargs)) + return True + + def _recipients(self): + recipients = set() + for args, kwargs in self.calls: + email_address = args[0] + recipients.add(email_address) + return recipients + + def check_empty_and_reset(self): + if self._checked: + # nothing to do + return + + has_calls = len(self.calls) > 0 + if has_calls: + suprise_recipients = [] + for args, kwargs in self.calls: + email_address = args[0] + suprise_recipients.append(email_address) + raise AssertionError('detected email sending without expectation: \n %s' + % ('\n '.join(suprise_recipients),)) + + def forgive_email(self): + self._checked = True + + @property + def called_once(self): + self._checked = True + return len(self.calls) == 1 + + def email_was_sent_to(self, email_address): + recipients = self._recipients() + assert email_address in recipients, \ + f"no email was not set to recipient: {email_address}" + self._checked = True + return email_address in recipients + + +def make_fake_notifier(mig_test_case=None): + fake_send_email = FakeSendEmail() + + if mig_test_case: + mig_test_case._register_check(fake_send_email.check_empty_and_reset) + + return SimpleNamespace(send_email=fake_send_email) + + +def instrument_test_case(mig_test_case=None, mig_configuration=None): + assert inspect.ismethod(getattr(mig_configuration, 'context_set', None)), \ + "supplied configuration must be usable at runtime" + + fakes_by_context_key = { + 'notifier': make_fake_notifier(mig_test_case=mig_test_case), + } + for content_key, context_value in fakes_by_context_key.items(): + mig_configuration.context_set(content_key, context_value) diff --git a/tests/test_mig_lib_janitor.py b/tests/test_mig_lib_janitor.py index 59a522cde..05ec319e4 100644 --- a/tests/test_mig_lib_janitor.py +++ b/tests/test_mig_lib_janitor.py @@ -329,6 +329,10 @@ def test_expire_user_pending(self): # self.assertEqual(handled, 1) self.assertEqual(handled, 2) # counted stale and expired (see above) + fake_send_email = self.configuration.context_get('notifier').send_email + self.assertTrue(fake_send_email.called_once) + self.assertTrue(fake_send_email.email_was_sent_to('test@example.com')) + def test_handle_pending_requests(self): """Test combined request handling""" # Create requests (valid, expired) @@ -379,8 +383,13 @@ def test_handle_pending_requests(self): # self.assertEqual(handled, 2) # 1 manage + 1 expire self.assertEqual(handled, 3) # 1 manage + 1 expire + 1 stale + fake_send_email = self.configuration.context_get('notifier').send_email + self.assertTrue(fake_send_email.called_once) + self.assertTrue(fake_send_email.email_was_sent_to('test@example.com')) + def test_handle_janitor_tasks_full(self): """Test full janitor task scheduler""" + self.configuration.context_get('notifier').send_email.forgive_email() # Prepare environment with pending tasks of each kind mig_stamp = time.time() - EXPIRE_STATE_DAYS * SECS_PER_DAY - 1 mig_path = os.path.join(self.configuration.mig_system_files, @@ -497,9 +506,13 @@ def test_manage_single_req_invalid(self): self.assertTrue(any('invalid account request' in msg for msg in log_capture.output)) - # TODO: enable check for removed req once skip email allows it - # self.assertFalse(os.path.exists(req_path), - # "Failed to clean invalid req for %s" % req_path) + self.assertFalse(os.path.exists(req_path), + "Failed to clean invalid req for %s" % req_path) + + # FIXME: should this really be sending email? + fake_send_email = self.configuration.context_get('notifier').send_email + self.assertTrue(fake_send_email.called_once) + self.assertTrue(fake_send_email.email_was_sent_to('test@example.com')) def test_manage_single_req_expired_token(self): """Test request handling with expired reset token""" @@ -540,12 +553,15 @@ def test_manage_single_req_expired_token(self): self.assertTrue(any('reject expired reset token' in msg for msg in log_capture.output)) - # TODO: enable check for removed req once skip email allows it - # self.assertFalse(os.path.exists(req_path), - # "Failed to clean token req for %s" % req_path) + self.assertFalse(os.path.exists(req_path), + "Failed to clean token req for %s" % req_path) + + fake_send_email = self.configuration.context_get('notifier').send_email + self.assertTrue(fake_send_email.called_once) + self.assertTrue(fake_send_email.email_was_sent_to('test@example.com')) @unittest.skip("TODO: enable once fernet decrypt err handling is improved") - def test_manage_single_req_invalid_token(self): + def test_manage_single_req_invalid_token_decrypt(self): """Test request handling with invalid reset token""" req_dict = { 'client_id': TEST_USER_DN, @@ -579,6 +595,11 @@ def test_manage_single_req_invalid_token(self): self.assertFalse(os.path.exists(req_path), "Failed to clean token req for %s" % req_path) + # FIXME: should this really be sending an email? + fake_send_email = self.configuration.context_get('notifier').send_email + self.assertTrue(fake_send_email.called_once) + self.assertTrue(fake_send_email.email_was_sent_to('test@example.com')) + def test_manage_single_req_collision(self): """Test request handling with existing user collision""" # Create collision with the already provisioned user with TEST_USER_DN @@ -612,9 +633,14 @@ def test_manage_single_req_collision(self): ) self.assertTrue(any('ID collision' in msg for msg in log_capture.output)) - # TODO: enable check for removed req once skip email allows it - # self.assertFalse(os.path.exists(req_path), - # "Failed cleanup collision for %s" % req_path) + + self.assertFalse(os.path.exists(req_path), + "Failed cleanup collision for %s" % req_path) + + fake_send_email = self.configuration.context_get('notifier').send_email + self.assertTrue(fake_send_email.called_once) + self.assertTrue(fake_send_email.email_was_sent_to('test@example.com')) + def test_manage_single_req_auth_change(self): """Test request handling with auth password change""" @@ -636,10 +662,6 @@ def test_manage_single_req_auth_change(self): saved, req_path = save_account_request(self.configuration, req_dict) req_id = os.path.basename(req_path) - # NOTE: when using real user mail we currently hit send email errors. - # We forgive those errors here and only check any known warnings. - # TODO: integrate generic skip email support and adjust here to fit - self.logger.forgive_errors() with self.assertLogs(level='INFO') as log_capture: manage_single_req( self.configuration, @@ -654,6 +676,10 @@ def test_manage_single_req_auth_change(self): self.assertFalse(os.path.exists(req_path), "Failed to clean token req for %s" % req_path) + fake_send_email = self.configuration.context_get('notifier').send_email + self.assertTrue(fake_send_email.called_once) + self.assertTrue(fake_send_email.email_was_sent_to('test@example.com')) + def test_handle_cache_updates_stub(self): """Test handle_cache_updates placeholder returns zero""" handled = handle_cache_updates(self.configuration) @@ -801,9 +827,13 @@ def test_verify_reset_token_failure_logging(self): self.assertTrue(any('wrong hash' in msg.lower() for msg in log_capture.output)) - # TODO: enable check for removed req once skip email allows it - # self.assertFalse(os.path.exists(req_path), - # "Failed cleanup invalid token for %s" % req_path) + self.assertFalse(os.path.exists(req_path), + "Failed cleanup invalid token for %s" % req_path) + + # FIXME: should this really be sending email? + fake_send_email = self.configuration.context_get('notifier').send_email + self.assertTrue(fake_send_email.called_once) + self.assertTrue(fake_send_email.email_was_sent_to('test@example.com')) def test_verify_reset_token_success(self): """Test token verification success with valid token""" @@ -843,12 +873,16 @@ def test_verify_reset_token_success(self): self.assertTrue(any('accepted' in msg.lower() for msg in log_capture.output)) - # TODO: enable check for removed req once skip email allows it - # self.assertFalse(os.path.exists(req_path), - # "Failed cleanup invalid token for %s" % req_path) + self.assertFalse(os.path.exists(req_path), + "Failed cleanup invalid token for %s" % req_path) + + fake_send_email = self.configuration.context_get('notifier').send_email + self.assertTrue(fake_send_email.called_once) + self.assertTrue(fake_send_email.email_was_sent_to('test@example.com')) def test_remind_and_expire_edge_cases(self): """Test request expiration with exact boundary timestamps""" + self.configuration.context_get('notifier').send_email.forgive_email() now = time.time() test_cases = [ ('exact_remind', now - REMIND_REQ_DAYS * SECS_PER_DAY), @@ -991,10 +1025,7 @@ def test_janitor_task_cleanup_after_reject(self): # Verify initial existence self.assertTrue(os.path.exists(req_path)) - # NOTE: when using real user mail we currently hit send email errors. - # We forgive those errors here and only check any known warnings. - # TODO: integrate generic skip email support and adjust here to fit - self.logger.forgive_errors() + manage_single_req( self.configuration, req_id, @@ -1003,9 +1034,12 @@ def test_janitor_task_cleanup_after_reject(self): time.time() ) - # TODO: enable check for removed req once skip email allows it - # self.assertFalse(os.path.exists(req_path), - # "Failed cleanup after reject for %s" % req_path) + self.assertFalse(os.path.exists(req_path), + "Failed cleanup after reject for %s" % req_path) + + fake_send_email = self.configuration.context_get('notifier').send_email + fake_send_email.email_was_sent_to('test@example.com') + def test_cleaner_with_multiple_patterns(self): """Test state cleaner with multiple filename patterns""" diff --git a/tests/test_mig_shared_accountreq.py b/tests/test_mig_shared_accountreq.py index ecba42ff3..7eee8788c 100644 --- a/tests/test_mig_shared_accountreq.py +++ b/tests/test_mig_shared_accountreq.py @@ -156,6 +156,10 @@ def test_peer_acceptance(self): self.assertTrue(success) + fake_send_email = self.configuration.context_get('notifier').send_email + self.assertTrue(fake_send_email.called_once) + self.assertTrue(fake_send_email.email_was_sent_to('peer@example.com')) + class MigSharedAccountreq__filters(MigTestCase, UserAssertMixin): """Unit tests for filter related functions within the accountreq module""" diff --git a/tests/test_mig_shared_configuration.py b/tests/test_mig_shared_configuration.py index 1108ea3e0..56eb98b8f 100644 --- a/tests/test_mig_shared_configuration.py +++ b/tests/test_mig_shared_configuration.py @@ -38,11 +38,6 @@ _CONFIGURATION_ARGUMENTS, _CONFIGURATION_PROPERTIES -def _to_dict(obj): - return {k: v for k, v in inspect.getmembers(obj) - if not (k.startswith('__') or inspect.ismethod(v) or inspect.isfunction(v))} - - class MigSharedConfiguration__static_definitions(MigTestCase): """Coverage of the static definitions underlying Configuration objects.""" @@ -349,7 +344,7 @@ def test_default_object(self): configuration.state_path = '/some/place/state' configuration.mig_path = '/some/place/mig' - actual_values = _to_dict(configuration) + actual_values = Configuration.to_dict(configuration) prepared_fixture.assertAgainstFixture(actual_values) diff --git a/tests/test_support.py b/tests/test_support.py index 56b74f594..fe55395ac 100644 --- a/tests/test_support.py +++ b/tests/test_support.py @@ -35,8 +35,7 @@ from tests.support import MigTestCase, PY2, testmain, temppath, \ AssertOver, FakeConfiguration -from mig.shared.conf import get_configuration_object -from mig.shared.configuration import Configuration +from mig.shared.conf import get_configuration_object, RuntimeConfiguration class InstrumentedAssertOver(AssertOver): @@ -165,7 +164,7 @@ def test_provides_the_test_configuration(self): configuration = self.configuration # check we have a real config object - self.assertIsInstance(configuration, Configuration) + self.assertIsInstance(configuration, RuntimeConfiguration) # check for having loaded a config file from a test config dir config_file_path_parts = configuration.config_file.split(os.path.sep) config_file_path_parts.pop() # discard file part