From 04a710a853a1138d7d8d108ce8b0a9ddbcf99fb1 Mon Sep 17 00:00:00 2001 From: Irene Simo Munoz Date: Tue, 29 Oct 2024 16:24:48 +0100 Subject: [PATCH] Add all expid_dir_path functions to autosubmit.py and fix tests !509 All previous definitions all expid_path, tmp_path, log_path and aslogs_path (and similar) have been removed and their calls have been switched for BasicConfig.foo() calls. Impacted tests are mainly affected in terms of adding Mock objects since BasicConfig.foo() functions are not available in autosubmitconfigparser yet. No new tests added --- autosubmit/notifications/mail_notifier.py | 54 ++++++---- test/unit/conftest.py | 27 ++--- test/unit/test_catlog.py | 46 +++++---- test/unit/test_describe.py | 13 ++- test/unit/test_mail.py | 116 ++++++++++++++++++++++ test/unit/test_scheduler_general.py | 1 + 6 files changed, 207 insertions(+), 50 deletions(-) create mode 100644 test/unit/test_mail.py diff --git a/autosubmit/notifications/mail_notifier.py b/autosubmit/notifications/mail_notifier.py index e6a5f2c43..bfcea3c68 100644 --- a/autosubmit/notifications/mail_notifier.py +++ b/autosubmit/notifications/mail_notifier.py @@ -20,7 +20,11 @@ import smtplib import email.utils from email.mime.text import MIMEText +from email.mime.multipart import MIMEMultipart +from email.mime.application import MIMEApplication from log.log import Log +from pathlib import Path +from autosubmitconfigparser.config.basicconfig import BasicConfig class MailNotifier: def __init__(self, basic_config): @@ -28,10 +32,18 @@ def __init__(self, basic_config): def notify_experiment_status(self, exp_id,mail_to,platform): message_text = self._generate_message_experiment_status(exp_id, platform) - message = MIMEText(message_text) + message = MIMEMultipart() message['From'] = email.utils.formataddr(('Autosubmit', self.config.MAIL_FROM)) - message['Subject'] = f'[Autosubmit] Warning a remote platform is malfunctioning' + message['Subject'] = f'[Autosubmit] Warning: a remote platform is malfunctioning' message['Date'] = email.utils.formatdate(localtime=True) + message.attach(MIMEText(message_text)) + + try: + files = [f for f in BasicConfig.expid_aslog_dir(exp_id).glob('*_run.log') if Path(f).is_file()] + self._attach_files(message, files) + except BaseException as e: + Log.printlog('An error has occurred while attaching log files to a warning email about remote_platforms ', 6011) + for mail in mail_to: message['To'] = email.utils.formataddr((mail, mail)) try: @@ -53,26 +65,34 @@ def notify_status_change(self, exp_id, job_name, prev_status, status, mail_to): Log.printlog('Trace:{0}\nAn error has occurred while sending a mail for the job {0}'.format(e,job_name), 6011) def _send_mail(self, mail_from, mail_to, message): - server = smtplib.SMTP(self.config.SMTP_SERVER,timeout=60) + server = smtplib.SMTP(self.config.SMTP_SERVER, timeout=60) server.sendmail(mail_from, mail_to, message.as_string()) server.quit() + def _attach_files(self, message, files): + for f in files or []: + with open(f, "rb") as file: + part = MIMEApplication(file.read(), Name = Path(f).name) + part['Content-Disposition'] = 'attachment; filename="%s"' % Path(f).name + message.attach(part) + + @staticmethod def _generate_message_text(exp_id, job_name, prev_status, status): - return f'Autosubmit notification\n' \ - f'-------------------------\n\n' \ - f'Experiment id: {str(exp_id)} \n\n' \ - + f'Job name: {str(job_name)} \n\n' \ - f'The job status has changed from: {str(prev_status)} to {str(status)} \n\n\n\n\n' \ - f'INFO: This message was auto generated by Autosubmit, '\ - f'remember that you can disable these messages on Autosubmit config file. \n' + return f'''Autosubmit notification\n + -------------------------\n\n + Experiment id: {str(exp_id)} \n\n + Job name: {str(job_name)} \n\n + The job status has changed from: {str(prev_status)} to {str(status)} \n\n\n\n\n + INFO: This message was auto generated by Autosubmit, + remember that you can disable these messages on Autosubmit config file. \n''' @staticmethod def _generate_message_experiment_status(exp_id, platform=""): - return f'Autosubmit notification: Remote Platform issues\n' \ - f'-------------------------\n\n' \ - f'Experiment id:{str(exp_id)} \n\n' \ - + f'Platform affected:{str(platform.name)} using as host:{str(platform.host)} \n\n' \ - f'[WARN] Autosubmit encountered an issue with an remote_platform.\n It will resume itself, whenever is possible\n If issue persist, you can change the host IP or put multiple hosts in the platform.yml' + '\n\n\n\n\n' \ - f'INFO: This message was auto generated by Autosubmit, '\ - f'remember that you can disable these messages on Autosubmit config file. \n' + return f'''Autosubmit notification: Remote Platform issues\n-------------------------\n + Experiment id: {str(exp_id)} + Logs and errors: {BasicConfig.expid_aslog_dir(exp_id)} + Attached to this message you will find the related _run.log files. + + Platform affected: {str(platform.name)} using as host: {str(platform.host)}\n\n[WARN] Autosubmit encountered an issue with a remote platform.\n It will resume itself, whenever is possible\n If this issue persists, you can change the host IP or put multiple hosts in the platform.yml file' + '\n\n\n\n\nINFO: This message was auto generated by Autosubmit, + remember that you can disable these messages on Autosubmit config file.\n''' diff --git a/test/unit/conftest.py b/test/unit/conftest.py index f2ab6ab27..f01a8cb84 100644 --- a/test/unit/conftest.py +++ b/test/unit/conftest.py @@ -8,6 +8,7 @@ from shutil import rmtree from tempfile import TemporaryDirectory from typing import Any, Dict, Callable, List, Protocol, Optional +import os from autosubmit.autosubmit import Autosubmit from autosubmit.platforms.slurmplatform import SlurmPlatform, ParamikoPlatform @@ -27,7 +28,6 @@ class AutosubmitExperiment: status_dir: Path platform: ParamikoPlatform - @pytest.fixture(scope='function') def autosubmit_exp(autosubmit: Autosubmit, request: pytest.FixtureRequest) -> Callable: """Create an instance of ``Autosubmit`` with an experiment.""" @@ -36,17 +36,21 @@ def autosubmit_exp(autosubmit: Autosubmit, request: pytest.FixtureRequest) -> Ca tmp_dir = TemporaryDirectory() tmp_path = Path(tmp_dir.name) + def _create_autosubmit_exp(expid: str): - # directories used when searching for logs to cat root_dir = tmp_path BasicConfig.LOCAL_ROOT_DIR = str(root_dir) - exp_path = root_dir / expid - exp_tmp_dir = exp_path / BasicConfig.LOCAL_TMP_DIR - aslogs_dir = exp_tmp_dir / BasicConfig.LOCAL_ASLOG_DIR - status_dir = exp_path / 'status' - aslogs_dir.mkdir(parents=True, exist_ok=True) - status_dir.mkdir(parents=True, exist_ok=True) - + exp_path = BasicConfig.expid_dir(expid) + + # directories used when searching for logs to cat + exp_tmp_dir = BasicConfig.expid_tmp_dir(expid) + aslogs_dir = BasicConfig.expid_aslog_dir(expid) + status_dir =exp_path / 'status' + if not os.path.exists(aslogs_dir): + os.makedirs(aslogs_dir) + if not os.path.exists(status_dir): + os.makedirs(status_dir) + platform_config = { "LOCAL_ROOT_DIR": BasicConfig.LOCAL_ROOT_DIR, "LOCAL_TMP_DIR": str(exp_tmp_dir), @@ -59,7 +63,7 @@ def _create_autosubmit_exp(expid: str): 'QUEUING': [], 'FAILED': [] } - submit_platform_script = aslogs_dir / 'submit_local.sh' + submit_platform_script = aslogs_dir.joinpath('submit_local.sh') submit_platform_script.touch(exist_ok=True) return AutosubmitExperiment( @@ -94,7 +98,7 @@ def autosubmit() -> Autosubmit: @pytest.fixture(scope='function') def create_as_conf() -> Callable: # May need to be changed to use the autosubmit_config one def _create_as_conf(autosubmit_exp: AutosubmitExperiment, yaml_files: List[Path], experiment_data: Dict[str, Any]): - conf_dir = autosubmit_exp.exp_path / 'conf' + conf_dir = autosubmit_exp.exp_path.joinpath('conf') conf_dir.mkdir(parents=False, exist_ok=False) basic_config = BasicConfig parser_factory = YAMLParserFactory() @@ -117,7 +121,6 @@ def _create_as_conf(autosubmit_exp: AutosubmitExperiment, yaml_files: List[Path] return _create_as_conf - class AutosubmitConfigFactory(Protocol): # Copied from the autosubmit config parser, that I believe is a revised one from the create_as_conf def __call__(self, expid: str, experiment_data: Optional[Dict], *args: Any, **kwargs: Any) -> AutosubmitConfig: ... diff --git a/test/unit/test_catlog.py b/test/unit/test_catlog.py index f7b2be15e..f9c68a6a6 100644 --- a/test/unit/test_catlog.py +++ b/test/unit/test_catlog.py @@ -6,25 +6,33 @@ from pathlib import Path from tempfile import TemporaryDirectory from unittest.mock import patch +import pytest from autosubmit.autosubmit import Autosubmit, AutosubmitCritical from autosubmitconfigparser.config.basicconfig import BasicConfig - class TestJob(TestCase): def setUp(self): - self.autosubmit = Autosubmit() - # directories used when searching for logs to cat self.original_root_dir = BasicConfig.LOCAL_ROOT_DIR self.root_dir = TemporaryDirectory() BasicConfig.LOCAL_ROOT_DIR = self.root_dir.name - self.exp_path = Path(self.root_dir.name, 'a000') - self.tmp_dir = self.exp_path / BasicConfig.LOCAL_TMP_DIR - self.aslogs_dir = self.tmp_dir / BasicConfig.LOCAL_ASLOG_DIR - self.status_path = self.exp_path / 'status' - self.aslogs_dir.mkdir(parents=True) - self.status_path.mkdir() + + self.exp_path = BasicConfig.expid_dir('a000') + self.tmp_dir = BasicConfig.expid_tmp_dir('a000') + self.log_dir = BasicConfig.expid_log_dir('a000') + self.aslogs_dir = BasicConfig.expid_aslog_dir('a000') + + self.autosubmit = Autosubmit() + # directories used when searching for logs to cat + + self.status_path = self.exp_path / 'status' + if not self.aslogs_dir.exists(): + self.aslogs_dir.mkdir(parents = True, exist_ok = True) + if not self.status_path.exists(): + self.status_path.mkdir(parents = True, exist_ok = True) + if not self.log_dir.exists(): + self.log_dir.mkdir(parents=True, exist_ok=True) def tearDown(self) -> None: BasicConfig.LOCAL_ROOT_DIR = self.original_root_dir @@ -55,15 +63,17 @@ def test_is_workflow_not_found(self, Log): assert Log.info.call_args[0][0] == 'No logs found.' def test_is_workflow_log_is_dir(self): - log_file_actually_dir = Path(self.aslogs_dir, 'log_run.log') - log_file_actually_dir.mkdir() + log_file_actually_dir = self.aslogs_dir / 'log_run.log' + log_file_actually_dir.mkdir(parents=True) def _fn(): self.autosubmit.cat_log('a000', 'o', 'c') self.assertRaises(AutosubmitCritical, _fn) @patch('subprocess.Popen') def test_is_workflow_out_cat(self, popen): - log_file = Path(self.aslogs_dir, 'log_run.log') + log_file = self.aslogs_dir / 'log_run.log' + if log_file.is_dir(): # dir is created in previous test + log_file.rmdir() with open(log_file, 'w') as f: f.write('as test') f.flush() @@ -75,7 +85,7 @@ def test_is_workflow_out_cat(self, popen): @patch('subprocess.Popen') def test_is_workflow_status_tail(self, popen): - log_file = Path(self.status_path, 'a000_anything.txt') + log_file = self.status_path / 'a000_anything.txt' with open(log_file, 'w') as f: f.write('as test') f.flush() @@ -93,9 +103,9 @@ def test_is_jobs_not_found(self, Log): self.autosubmit.cat_log('a000_INI', file=file, mode='c') assert Log.info.called assert Log.info.call_args[0][0] == 'No logs found.' - + def test_is_jobs_log_is_dir(self): - log_file_actually_dir = Path(self.tmp_dir, 'LOG_a000/a000_INI.20000101.out') + log_file_actually_dir = self.log_dir / 'a000_INI.20000101.out' log_file_actually_dir.mkdir(parents=True) def _fn(): self.autosubmit.cat_log('a000_INI', 'o', 'c') @@ -103,9 +113,9 @@ def _fn(): @patch('subprocess.Popen') def test_is_jobs_out_tail(self, popen): - log_dir = self.tmp_dir / 'LOG_a000' - log_dir.mkdir() - log_file = log_dir / 'a000_INI.20200101.out' + log_file = self.log_dir / 'a000_INI.20200101.out' + if log_file.is_dir(): # dir is created in previous test + log_file.rmdir() with open(log_file, 'w') as f: f.write('as test') f.flush() diff --git a/test/unit/test_describe.py b/test/unit/test_describe.py index 97f6946f2..f63077d47 100644 --- a/test/unit/test_describe.py +++ b/test/unit/test_describe.py @@ -33,14 +33,18 @@ def test_describe( if expid not in _EXPIDS: continue exp = autosubmit_exp(expid) - + basic_config = mocker.MagicMock() + # TODO: Whenever autosubmitconfigparser gets released with BasicConfig.expid_dir() and similar functions, the following line and a lot of mocks need to be removed. This line is especially delicate because it "overmocks" the basic_config mock, thus making the last assertion of this file "assert f'Location: {exp.exp_path}' in log_result_output" a dummy assertion. The rest of the test is still useful. + basic_config.expid_dir.side_effect = lambda expid: exp.exp_path config_values = { 'LOCAL_ROOT_DIR': str(exp.exp_path.parent), 'LOCAL_ASLOG_DIR': str(exp.aslogs_dir) } + for key, value in config_values.items(): basic_config.__setattr__(key, value) + basic_config.get.side_effect = lambda key, default='': config_values.get(key, default) for basic_config_location in [ 'autosubmit.autosubmit.BasicConfig', @@ -48,14 +52,16 @@ def test_describe( ]: # TODO: Better design how ``BasicConfig`` is used/injected/IOC/etc.. mocker.patch(basic_config_location, basic_config) + mocked_get_submitter = mocker.patch.object(Autosubmit, '_get_submitter') submitter = mocker.MagicMock() + mocked_get_submitter.return_value = submitter submitter.platforms = [1, 2] get_experiment_descrip = mocker.patch('autosubmit.autosubmit.get_experiment_descrip') get_experiment_descrip.return_value = [[f'{expid} description']] - + create_as_conf( autosubmit_exp=exp, yaml_files=[ @@ -69,6 +75,7 @@ def test_describe( } ) + Autosubmit.describe( input_experiment_list=input_experiment_list, get_from_user=get_from_user @@ -84,4 +91,4 @@ def test_describe( ] root_dir = exp.exp_path.parent for expid in expids: - assert f'Location: {str(root_dir / expid)}' in log_result_output + assert f'Location: {exp.exp_path}' in log_result_output diff --git a/test/unit/test_mail.py b/test/unit/test_mail.py new file mode 100644 index 000000000..a87ce3b35 --- /dev/null +++ b/test/unit/test_mail.py @@ -0,0 +1,116 @@ +import pytest +import tempfile +from unittest import mock +from pathlib import Path +from autosubmit.notifications.mail_notifier import MailNotifier +from autosubmitconfigparser.config.basicconfig import BasicConfig +from log.log import Log + +@pytest.fixture +def mock_basic_config(): + mock_config = mock.Mock() + mock_config.MAIL_FROM = "test@example.com" + mock_config.SMTP_SERVER = "smtp.example.com" + return mock_config + +@pytest.fixture +def mail_notifier(mock_basic_config): + return MailNotifier(mock_basic_config) + +def test_send_mail(mail_notifier): + with mock.patch('smtplib.SMTP') as mock_smtp: + mock_server = mock.Mock() + mock_smtp.return_value = mock_server + mock_server.sendmail = mock.Mock() + + mail_notifier._send_mail('sender@example.com', 'recipient@example.com', mock.Mock()) + + mock_server.sendmail.assert_called_once_with('sender@example.com', 'recipient@example.com', mock.ANY) + +def test_attach_files(mail_notifier): + with tempfile.TemporaryDirectory() as temp_dir: + file1 = Path(temp_dir) / "file1_run.log" + file2 = Path(temp_dir) / "file2_run.log" + file1.write_text("file data 1") + file2.write_text("file data 2") + + assert file1.exists() + assert file2.exists() + + mock_files = [file1, file2] + + with mock.patch("builtins.open", mock.mock_open(read_data="file data")): + message = mock.Mock() + mail_notifier._attach_files(message, mock_files) + + assert message.attach.call_count == len(mock_files) +@pytest.mark.parametrize( + "mock_glob_side_effect, send_mail_side_effect, expected_log_message, expected_call_count", + [ + # Normal case: No errors, should not log anything + (None, None, None, 0), # No logs are expected, everything works fine + + # Log connection error: Simulate an error while sending email + (None, Exception("SMTP server error"), + 'An error has occurred while sending a mail for warn about remote_platform', 1), + + # Log attachment error: Simulate failure in file listing (glob raises exception) + (Exception("Failed to list files"), None, + 'An error has occurred while attaching log files to a warning email about remote_platforms ', 1) + ], + ids=[ + "Normal case: No errors", + "Log connection error (SMTP server error)", + "Log attachment error (Failed to list files)" + ] +) +def test_notify_experiment_status(mail_notifier, mock_glob_side_effect, send_mail_side_effect, expected_log_message, expected_call_count): + original_local_root_dir = BasicConfig.LOCAL_ROOT_DIR + with tempfile.TemporaryDirectory() as temp_dir: + BasicConfig.LOCAL_ROOT_DIR = temp_dir + exp_id = '1234' + aslogs_dir = Path(temp_dir) / exp_id / "tmp" / "ASLOGS" + aslogs_dir.mkdir(parents=True, exist_ok=True) + + file1 = aslogs_dir / "1234_run.log" + file2 = aslogs_dir / "1235_run.log" + + file1.write_text("file data 1") + file2.write_text("file data 2") + + assert file1.exists() + assert file2.exists() + + with mock.patch.object(mail_notifier, '_generate_message_experiment_status', return_value="Test message"), \ + mock.patch.object(mail_notifier, '_send_mail') as mock_send_mail, \ + mock.patch.object(mail_notifier, '_attach_files') as mock_attach_files, \ + mock.patch.object(Log, 'printlog') as mock_printlog: + + mail_to = ['recipient@example.com'] + platform = mock.Mock() + platform.name = "Test Platform" + platform.host = "test.host.com" + + mock_send_mail.side_effect = send_mail_side_effect + + if mock_glob_side_effect is not None: + # Log attachment error: Simulate failure in file listing (glob raises exception) + with mock.patch.object(Path, 'glob', side_effect = mock_glob_side_effect): + mail_notifier.notify_experiment_status(exp_id, mail_to, platform) + else: + mail_notifier.notify_experiment_status(exp_id, mail_to, platform) + + mail_notifier._generate_message_experiment_status.assert_called_once_with(exp_id, platform) + mock_send_mail.assert_called_once_with(mail_notifier.config.MAIL_FROM, 'recipient@example.com', mock.ANY) + + if mock_glob_side_effect is None: mock_attach_files.assert_called_once_with(mock.ANY, [file1, file2]) + + if expected_log_message: + mock_printlog.assert_called_once_with(expected_log_message, 6011) + log_calls = [call[0][0] for call in mock_printlog.call_args_list] + assert 'Traceback' not in log_calls + else: + mock_printlog.assert_not_called() # No logs should be called for normal execution + + # Reset the local root dir. + BasicConfig.LOCAL_ROOT_DIR = original_local_root_dir diff --git a/test/unit/test_scheduler_general.py b/test/unit/test_scheduler_general.py index 4dded6f9a..dc152a56e 100644 --- a/test/unit/test_scheduler_general.py +++ b/test/unit/test_scheduler_general.py @@ -201,6 +201,7 @@ def generate_cmds(prepare_scheduler): ('slurm', 'horizontal_vertical'), ('slurm', 'vertical_horizontal') ]) + def test_scheduler_job_types(scheduler, job_type, generate_cmds): # Test code that uses scheduler and job_typedef test_default_parameters(scheduler: str, job_type: str, generate_cmds): """