From db03aa7e0eefb940b08820880bd94c196da98402 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Sun, 26 Nov 2023 16:19:15 +0100 Subject: [PATCH 01/11] return to original directory earlier after executing a command --- easybuild/tools/run.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py index ca33a94a9c..99b3252daf 100644 --- a/easybuild/tools/run.py +++ b/easybuild/tools/run.py @@ -505,6 +505,12 @@ def to_cmd_str(cmd): if fail_on_error: raise_run_shell_cmd_error(res) + # return to original work dir after command execution + try: + os.chdir(res.work_dir) + except OSError as err: + raise EasyBuildError(f"Failed to return to {res.work_dir} after executing command `{cmd_str}`: {err}") + if with_hooks: run_hook_kwargs = { 'exit_code': res.exit_code, From c462a05611696f3d272d1811585247c0e682f849 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Wed, 6 Dec 2023 09:11:10 +0100 Subject: [PATCH 02/11] enhance test_run_cmd_with_hooks to check whether run_cmd/complete_cmd is robust against command that removes the directory it's running in --- test/framework/run.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/test/framework/run.py b/test/framework/run.py index 23fdb562d3..a5d40ad5a7 100644 --- a/test/framework/run.py +++ b/test/framework/run.py @@ -24,7 +24,7 @@ # along with EasyBuild. If not, see . # # """ -Unit tests for filetools.py +Unit tests for run.py @author: Toon Willems (Ghent University) @author: Kenneth Hoste (Ghent University) @@ -1854,6 +1854,22 @@ def post_run_shell_cmd_hook(cmd, *args, **kwargs): ]) self.assertEqual(stdout, expected_stdout) + # also check with a command that destroys the directory it's running in + workdir = os.path.join(self.test_prefix, 'workdir') + mkdir(workdir) + + with self.mocked_stdout_stderr(): + run_shell_cmd("pwd; rm -rf %(workdir)s; echo '%(workdir)s removed'" % {'workdir': workdir}, path=workdir) + stdout = self.get_stdout() + + patterns = [ + r"pre-run hook 'pwd; rm .*/workdir removed'", + r"post-run hook 'pwd;\s*rm.*/workdir removed'' \(exit code: 0, output: '.*\n.*/workdir removed\n'\)", + ] + for pattern in patterns: + regex = re.compile(pattern, re.M) + self.assertTrue(regex.search(stdout), "Pattern '%s' should be found in: %s" % (regex.pattern, stdout)) + def suite(): """ returns all the testcases in this module """ From 9f096cbf05e68d10c5af57a0471837b24f49bef7 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Mon, 6 May 2024 17:00:11 +0200 Subject: [PATCH 03/11] test run_shell_cmd with hooks enabled with commands that remove the original workdir or jump into a new working subdirectory and remove it --- test/framework/run.py | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/test/framework/run.py b/test/framework/run.py index a5d40ad5a7..685afcad9a 100644 --- a/test/framework/run.py +++ b/test/framework/run.py @@ -1848,26 +1848,39 @@ def post_run_shell_cmd_hook(cmd, *args, **kwargs): stdout = self.get_stdout() expected_stdout = '\n'.join([ - "pre-run hook 'make' in %s" % cwd, + f"pre-run hook 'make' in {cwd}", "post-run hook 'echo make' (exit code: 0, output: 'make\n')", '', ]) self.assertEqual(stdout, expected_stdout) - # also check with a command that destroys the directory it's running in + # check commands that destroy directories inside initial working directory + # 1. test destruction of main working directory workdir = os.path.join(self.test_prefix, 'workdir') mkdir(workdir) + with self.mocked_stdout_stderr(): + cmd_workdir_rm = "echo 'Command that ends up removing working directory' && pwd && " + cmd_workdir_rm += f"rm -rf {workdir} && echo 'Working directory removed.'" + error_pattern = rf"Failed to return to {workdir} after executing command" + self.assertErrorRegex(EasyBuildError, error_pattern, run_shell_cmd, cmd_workdir_rm, work_dir=workdir) + # 2. test destruction of current working (sub)directory inside original working directory + sub_workdir = os.path.join(self.test_prefix, 'workdir', 'subworkdir') + mkdir(sub_workdir, parents=True) with self.mocked_stdout_stderr(): - run_shell_cmd("pwd; rm -rf %(workdir)s; echo '%(workdir)s removed'" % {'workdir': workdir}, path=workdir) + cmd_workdir_rm = "echo 'Command that jumps to working subdir and removes it' && " + cmd_workdir_rm += f"cd {sub_workdir} && pwd && rm -rf {sub_workdir} && " + cmd_workdir_rm += "echo 'Working directory removed.'" + run_shell_cmd(cmd_workdir_rm, work_dir=workdir) stdout = self.get_stdout() - patterns = [ - r"pre-run hook 'pwd; rm .*/workdir removed'", - r"post-run hook 'pwd;\s*rm.*/workdir removed'' \(exit code: 0, output: '.*\n.*/workdir removed\n'\)", + expected_stdout_patterns = [ + rf"pre-run hook '{cmd_workdir_rm}' in {workdir}", + (rf"post-run hook '{cmd_workdir_rm}' \(exit code: 0, " + rf"output: 'Command that jumps.*\n{sub_workdir}\nWorking directory removed\..*'\)"), ] - for pattern in patterns: - regex = re.compile(pattern, re.M) + for pattern in expected_stdout_patterns: + regex = re.compile(pattern, re.S) self.assertTrue(regex.search(stdout), "Pattern '%s' should be found in: %s" % (regex.pattern, stdout)) From 0a3091cb025b12674bf52b218b53d2384f948976 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Mon, 6 May 2024 17:43:11 +0200 Subject: [PATCH 04/11] update test_run_shell_cmd_work_dir to consider run_shell_cmd with and without work_dir set --- test/framework/run.py | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/test/framework/run.py b/test/framework/run.py index 685afcad9a..aab8d3d3ec 100644 --- a/test/framework/run.py +++ b/test/framework/run.py @@ -500,24 +500,40 @@ def test_run_shell_cmd_work_dir(self): """ Test running shell command in specific directory with run_shell_cmd function. """ - orig_wd = os.getcwd() - self.assertFalse(os.path.samefile(orig_wd, self.test_prefix)) - test_dir = os.path.join(self.test_prefix, 'test') + test_workdir = os.path.join(self.test_prefix, 'test', 'workdir') for fn in ('foo.txt', 'bar.txt'): - write_file(os.path.join(test_dir, fn), 'test') + write_file(os.path.join(test_workdir, fn), 'test') + + os.chdir(test_dir) + orig_wd = os.getcwd() + self.assertFalse(os.path.samefile(orig_wd, self.test_prefix)) cmd = "ls | sort" + + # undefined working directory + with self.mocked_stdout_stderr(): + res = run_shell_cmd(cmd) + + self.assertEqual(res.cmd, cmd) + self.assertEqual(res.exit_code, 0) + self.assertEqual(res.output, 'workdir\n') + self.assertEqual(res.stderr, None) + self.assertEqual(res.work_dir, orig_wd) + + self.assertTrue(os.path.samefile(orig_wd, os.getcwd())) + + # defined working directory with self.mocked_stdout_stderr(): - res = run_shell_cmd(cmd, work_dir=test_dir) + res = run_shell_cmd(cmd, work_dir=test_workdir) self.assertEqual(res.cmd, cmd) self.assertEqual(res.exit_code, 0) self.assertEqual(res.output, 'bar.txt\nfoo.txt\n') self.assertEqual(res.stderr, None) - self.assertEqual(res.work_dir, test_dir) + self.assertEqual(res.work_dir, test_workdir) - self.assertTrue(os.path.samefile(orig_wd, os.getcwd())) + self.assertTrue(os.path.samefile(test_workdir, os.getcwd())) def test_run_cmd_log_output(self): """Test run_cmd with log_output enabled""" From 76e38ae40c3fbac4ad2a9789f0ec14ddf1bb5b85 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Fri, 10 May 2024 12:07:31 +0200 Subject: [PATCH 05/11] only change directory if undefined CWD as safeguard after command execution in run_shell_cmd --- easybuild/tools/run.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py index 99b3252daf..0ec3e4d83f 100644 --- a/easybuild/tools/run.py +++ b/easybuild/tools/run.py @@ -505,11 +505,20 @@ def to_cmd_str(cmd): if fail_on_error: raise_run_shell_cmd_error(res) - # return to original work dir after command execution + # check that we still are in a sane environment after command execution + # safeguard against commands that leave behind the system in a borked state try: - os.chdir(res.work_dir) - except OSError as err: - raise EasyBuildError(f"Failed to return to {res.work_dir} after executing command `{cmd_str}`: {err}") + os.getcwd() + except FileNotFoundError: + try: + warn_msg = ( + f"Shell command `{cmd_str}` completed successfully but left system in a broken state. " + f"Changing back to initial working directory: {res.work_dir}" + ) + _log.warning(warn_msg) + os.chdir(res.work_dir) + except OSError as err: + raise EasyBuildError(f"Failed to return to {res.work_dir} after executing command `{cmd_str}`: {err}") if with_hooks: run_hook_kwargs = { From b7db884783dd303a1a3f0b781894057669d9e569 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Fri, 10 May 2024 12:08:46 +0200 Subject: [PATCH 06/11] add tests to run_shell_cmd_with_hooks mocking os.getcwd to raise error --- test/framework/run.py | 71 +++++++++++++++++++++++++++++++------------ 1 file changed, 51 insertions(+), 20 deletions(-) diff --git a/test/framework/run.py b/test/framework/run.py index aab8d3d3ec..f950f3cab5 100644 --- a/test/framework/run.py +++ b/test/framework/run.py @@ -32,6 +32,7 @@ """ import contextlib import glob +import mock import os import re import signal @@ -511,7 +512,7 @@ def test_run_shell_cmd_work_dir(self): cmd = "ls | sort" - # undefined working directory + # working directory is not explicitly defined with self.mocked_stdout_stderr(): res = run_shell_cmd(cmd) @@ -523,7 +524,7 @@ def test_run_shell_cmd_work_dir(self): self.assertTrue(os.path.samefile(orig_wd, os.getcwd())) - # defined working directory + # working directory is explicitly defined with self.mocked_stdout_stderr(): res = run_shell_cmd(cmd, work_dir=test_workdir) @@ -533,7 +534,7 @@ def test_run_shell_cmd_work_dir(self): self.assertEqual(res.stderr, None) self.assertEqual(res.work_dir, test_workdir) - self.assertTrue(os.path.samefile(test_workdir, os.getcwd())) + self.assertTrue(os.path.samefile(orig_wd, os.getcwd())) def test_run_cmd_log_output(self): """Test run_cmd with log_output enabled""" @@ -1870,34 +1871,64 @@ def post_run_shell_cmd_hook(cmd, *args, **kwargs): ]) self.assertEqual(stdout, expected_stdout) - # check commands that destroy directories inside initial working directory - # 1. test destruction of main working directory + # Test commands that destroy directories inside initial working directory workdir = os.path.join(self.test_prefix, 'workdir') - mkdir(workdir) - with self.mocked_stdout_stderr(): - cmd_workdir_rm = "echo 'Command that ends up removing working directory' && pwd && " - cmd_workdir_rm += f"rm -rf {workdir} && echo 'Working directory removed.'" - error_pattern = rf"Failed to return to {workdir} after executing command" - self.assertErrorRegex(EasyBuildError, error_pattern, run_shell_cmd, cmd_workdir_rm, work_dir=workdir) + sub_workdir = os.path.join(workdir, 'subworkdir') + + # 1. test destruction of CWD which is a subdirectory inside original working directory + cmd_workdir_rm = "echo 'Command that jumps to subdir and removes it' && " + cmd_workdir_rm += f"cd {sub_workdir} && pwd && rm -rf {sub_workdir} && " + cmd_workdir_rm += "echo 'Working sub-directory removed.'" + expected_stdout_patterns = [ + rf"pre-run hook '{cmd_workdir_rm}' in {workdir}", + (rf"post-run hook '{cmd_workdir_rm}' \(exit code: 0, " + rf"output: 'Command that jumps to subdir.*\n{sub_workdir}\nWorking sub-directory removed\..*'\)"), + ] + expected_stdout_patterns = [re.compile(pattern, re.S) for pattern in expected_stdout_patterns] - # 2. test destruction of current working (sub)directory inside original working directory - sub_workdir = os.path.join(self.test_prefix, 'workdir', 'subworkdir') + # in a robust system mkdir(sub_workdir, parents=True) with self.mocked_stdout_stderr(): - cmd_workdir_rm = "echo 'Command that jumps to working subdir and removes it' && " - cmd_workdir_rm += f"cd {sub_workdir} && pwd && rm -rf {sub_workdir} && " - cmd_workdir_rm += "echo 'Working directory removed.'" run_shell_cmd(cmd_workdir_rm, work_dir=workdir) stdout = self.get_stdout() + for regex in expected_stdout_patterns: + self.assertTrue(regex.search(stdout), f"Pattern '{regex.pattern}' should be found in: {stdout}") + + # in a flaky system that ends up in a broken environment after execution + mkdir(sub_workdir, parents=True) + with self.mocked_stdout_stderr(): + with mock.patch('os.getcwd') as mock_getcwd: + mock_getcwd.side_effect = FileNotFoundError() + run_shell_cmd(cmd_workdir_rm, work_dir=workdir) + stdout = self.get_stdout() + + for regex in expected_stdout_patterns: + self.assertTrue(regex.search(stdout), f"Pattern '{regex.pattern}' should be found in: {stdout}") + + # 2. test destruction of CWD which is main working directory passed to run_shell_cmd + cmd_workdir_rm = "echo 'Command that removes working directory' && pwd && " + cmd_workdir_rm += f"rm -rf {workdir} && echo 'Working directory removed.'" expected_stdout_patterns = [ rf"pre-run hook '{cmd_workdir_rm}' in {workdir}", (rf"post-run hook '{cmd_workdir_rm}' \(exit code: 0, " - rf"output: 'Command that jumps.*\n{sub_workdir}\nWorking directory removed\..*'\)"), + rf"output: 'Command that removes working.*\n{workdir}\nWorking directory removed\..*'\)"), ] - for pattern in expected_stdout_patterns: - regex = re.compile(pattern, re.S) - self.assertTrue(regex.search(stdout), "Pattern '%s' should be found in: %s" % (regex.pattern, stdout)) + expected_stdout_patterns = [re.compile(pattern, re.S) for pattern in expected_stdout_patterns] + + # in a robust system + mkdir(workdir) + with self.mocked_stdout_stderr(): + error_pattern = rf"Failed to return to {workdir} after executing command" + self.assertErrorRegex(EasyBuildError, error_pattern, run_shell_cmd, cmd_workdir_rm, work_dir=workdir) + + # in a flaky system that ends up in a broken environment after execution + mkdir(workdir) + with self.mocked_stdout_stderr(): + with mock.patch('os.getcwd') as mock_getcwd: + mock_getcwd.side_effect = FileNotFoundError() + error_pattern = rf"Failed to return to {workdir} after executing command" + self.assertErrorRegex(EasyBuildError, error_pattern, run_shell_cmd, cmd_workdir_rm, work_dir=workdir) def suite(): From 98a49daf3ad09694d27a65bdaff2e34ade1decf6 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Fri, 10 May 2024 12:22:32 +0200 Subject: [PATCH 07/11] replace mock module with unittest.mock --- test/framework/run.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/framework/run.py b/test/framework/run.py index f950f3cab5..a102fc957a 100644 --- a/test/framework/run.py +++ b/test/framework/run.py @@ -32,7 +32,6 @@ """ import contextlib import glob -import mock import os import re import signal @@ -45,7 +44,7 @@ import time from concurrent.futures import ThreadPoolExecutor from test.framework.utilities import EnhancedTestCase, TestLoaderFiltered, init_config -from unittest import TextTestRunner +from unittest import TextTestRunner, mock from easybuild.base.fancylogger import setLogLevelDebug import easybuild.tools.asyncprocess as asyncprocess From 672f6b1d5b71d25a89f13938c2c2f4ed4a546fcb Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Fri, 10 May 2024 14:33:31 +0200 Subject: [PATCH 08/11] remove redundant test from test_run_shell_cmd_with_hooks --- test/framework/run.py | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/test/framework/run.py b/test/framework/run.py index a102fc957a..f7df40156b 100644 --- a/test/framework/run.py +++ b/test/framework/run.py @@ -1885,7 +1885,7 @@ def post_run_shell_cmd_hook(cmd, *args, **kwargs): ] expected_stdout_patterns = [re.compile(pattern, re.S) for pattern in expected_stdout_patterns] - # in a robust system + # 1.a. in a robust system mkdir(sub_workdir, parents=True) with self.mocked_stdout_stderr(): run_shell_cmd(cmd_workdir_rm, work_dir=workdir) @@ -1894,7 +1894,7 @@ def post_run_shell_cmd_hook(cmd, *args, **kwargs): for regex in expected_stdout_patterns: self.assertTrue(regex.search(stdout), f"Pattern '{regex.pattern}' should be found in: {stdout}") - # in a flaky system that ends up in a broken environment after execution + # 1.b. in a flaky system that ends up in a broken environment after execution mkdir(sub_workdir, parents=True) with self.mocked_stdout_stderr(): with mock.patch('os.getcwd') as mock_getcwd: @@ -1915,20 +1915,11 @@ def post_run_shell_cmd_hook(cmd, *args, **kwargs): ] expected_stdout_patterns = [re.compile(pattern, re.S) for pattern in expected_stdout_patterns] - # in a robust system mkdir(workdir) with self.mocked_stdout_stderr(): error_pattern = rf"Failed to return to {workdir} after executing command" self.assertErrorRegex(EasyBuildError, error_pattern, run_shell_cmd, cmd_workdir_rm, work_dir=workdir) - # in a flaky system that ends up in a broken environment after execution - mkdir(workdir) - with self.mocked_stdout_stderr(): - with mock.patch('os.getcwd') as mock_getcwd: - mock_getcwd.side_effect = FileNotFoundError() - error_pattern = rf"Failed to return to {workdir} after executing command" - self.assertErrorRegex(EasyBuildError, error_pattern, run_shell_cmd, cmd_workdir_rm, work_dir=workdir) - def suite(): """ returns all the testcases in this module """ From 06c45c3869d2604d22d1dec99c2f89848867ae4a Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Mon, 10 Jun 2024 12:39:06 +0200 Subject: [PATCH 09/11] split out tests that delete cwd from test_run_shell_cmd_with_hooks into their own test --- easybuild/tools/run.py | 31 ++++++++++------- test/framework/run.py | 78 ++++++++++++++++++++++++++---------------- 2 files changed, 66 insertions(+), 43 deletions(-) diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py index 73638b835a..24715b58eb 100644 --- a/easybuild/tools/run.py +++ b/easybuild/tools/run.py @@ -361,11 +361,14 @@ def to_cmd_str(cmd): if qa_wait_patterns is None: qa_wait_patterns = [] + # keep path to current working dir in case we need to come back to it + try: + initial_work_dir = os.getcwd() + except FileNotFoundError: + raise EasyBuildError(CWD_NOTFOUND_ERROR) + if work_dir is None: - try: - work_dir = os.getcwd() - except FileNotFoundError: - raise EasyBuildError(CWD_NOTFOUND_ERROR) + work_dir = initial_work_dir if with_hooks: hooks = load_hooks(build_option('hooks')) @@ -559,19 +562,21 @@ def to_cmd_str(cmd): raise_run_shell_cmd_error(res) # check that we still are in a sane environment after command execution - # safeguard against commands that leave behind the system in a borked state + # safeguard against commands that deleted the work dir or missbehaving filesystems try: os.getcwd() except FileNotFoundError: + _log.warning( + f"Shell command `{cmd_str}` completed successfully but left the system in a unknown working directory. " + f"Changing back to initial working directory: {initial_work_dir}" + ) try: - warn_msg = ( - f"Shell command `{cmd_str}` completed successfully but left system in a broken state. " - f"Changing back to initial working directory: {res.work_dir}" - ) - _log.warning(warn_msg) - os.chdir(res.work_dir) + os.chdir(initial_work_dir) except OSError as err: - raise EasyBuildError(f"Failed to return to {res.work_dir} after executing command `{cmd_str}`: {err}") + raise EasyBuildError(f"Failed to return to {initial_work_dir} after executing command `{cmd_str}`: {err}") + else: + if not os.path.isdir(work_dir): + work_dir = initial_work_dir if with_hooks: run_hook_kwargs = { @@ -579,7 +584,7 @@ def to_cmd_str(cmd): 'interactive': interactive, 'output': res.output, 'stderr': res.stderr, - 'work_dir': res.work_dir, + 'work_dir': work_dir, } run_hook(RUN_SHELL_CMD, hooks, post_step_hook=True, args=[cmd], kwargs=run_hook_kwargs) diff --git a/test/framework/run.py b/test/framework/run.py index b464a52c2c..ab0d67e289 100644 --- a/test/framework/run.py +++ b/test/framework/run.py @@ -1976,54 +1976,72 @@ def post_run_shell_cmd_hook(cmd, *args, **kwargs): regex = re.compile('>> running shell command:\n\techo make', re.M) self.assertTrue(regex.search(stdout), "Pattern '%s' found in: %s" % (regex.pattern, stdout)) - # Test commands that destroy directories inside initial working directory + def test_run_shell_cmd_delete_cwd(self): + """ + Test commands that destroy directories inside initial working directory + """ workdir = os.path.join(self.test_prefix, 'workdir') sub_workdir = os.path.join(workdir, 'subworkdir') # 1. test destruction of CWD which is a subdirectory inside original working directory - cmd_workdir_rm = "echo 'Command that jumps to subdir and removes it' && " - cmd_workdir_rm += f"cd {sub_workdir} && pwd && rm -rf {sub_workdir} && " - cmd_workdir_rm += "echo 'Working sub-directory removed.'" - expected_stdout_patterns = [ - rf"pre-run hook '{cmd_workdir_rm}' in {workdir}", - (rf"post-run hook '{cmd_workdir_rm}' \(exit code: 0, " - rf"output: 'Command that jumps to subdir.*\n{sub_workdir}\nWorking sub-directory removed\..*'\)"), - ] - expected_stdout_patterns = [re.compile(pattern, re.S) for pattern in expected_stdout_patterns] + cmd_subworkdir_rm = ( + "echo 'Command that jumps to subdir and removes it' && " + f"cd {sub_workdir} && pwd && rm -rf {sub_workdir} && " + "echo 'Working sub-directory removed.'" + ) # 1.a. in a robust system + expected_output = ( + "Command that jumps to subdir and removes it\n" + f"{sub_workdir}\n" + "Working sub-directory removed.\n" + ) + mkdir(sub_workdir, parents=True) with self.mocked_stdout_stderr(): - run_shell_cmd(cmd_workdir_rm, work_dir=workdir) - stdout = self.get_stdout() + res = run_shell_cmd(cmd_subworkdir_rm, work_dir=workdir) - for regex in expected_stdout_patterns: - self.assertTrue(regex.search(stdout), f"Pattern '{regex.pattern}' should be found in: {stdout}") + self.assertEqual(res.cmd, cmd_subworkdir_rm) + self.assertEqual(res.exit_code, 0) + self.assertEqual(res.output, expected_output) + self.assertEqual(res.stderr, None) + self.assertEqual(res.work_dir, workdir) - # 1.b. in a flaky system that ends up in a broken environment after execution + # 1.b. in a flaky system that ends up in an unknown CWD after execution mkdir(sub_workdir, parents=True) + fd, logfile = tempfile.mkstemp(suffix='.log', prefix='eb-test-') + os.close(fd) + with self.mocked_stdout_stderr(): with mock.patch('os.getcwd') as mock_getcwd: - mock_getcwd.side_effect = FileNotFoundError() - run_shell_cmd(cmd_workdir_rm, work_dir=workdir) - stdout = self.get_stdout() + mock_getcwd.side_effect = [ + workdir, + FileNotFoundError(), + ] + init_logging(logfile, silent=True) + res = run_shell_cmd(cmd_subworkdir_rm, work_dir=workdir) + stop_logging(logfile) - for regex in expected_stdout_patterns: - self.assertTrue(regex.search(stdout), f"Pattern '{regex.pattern}' should be found in: {stdout}") + self.assertEqual(res.cmd, cmd_subworkdir_rm) + self.assertEqual(res.exit_code, 0) + self.assertEqual(res.output, expected_output) + self.assertEqual(res.stderr, None) + self.assertEqual(res.work_dir, workdir) + + expected_warning = f"Changing back to initial working directory: {workdir}\n" + logtxt = read_file(logfile) + self.assertTrue(logtxt.endswith(expected_warning)) # 2. test destruction of CWD which is main working directory passed to run_shell_cmd - cmd_workdir_rm = "echo 'Command that removes working directory' && pwd && " - cmd_workdir_rm += f"rm -rf {workdir} && echo 'Working directory removed.'" - expected_stdout_patterns = [ - rf"pre-run hook '{cmd_workdir_rm}' in {workdir}", - (rf"post-run hook '{cmd_workdir_rm}' \(exit code: 0, " - rf"output: 'Command that removes working.*\n{workdir}\nWorking directory removed\..*'\)"), - ] - expected_stdout_patterns = [re.compile(pattern, re.S) for pattern in expected_stdout_patterns] + cmd_workdir_rm = ( + "echo 'Command that removes working directory' && pwd && " + f"rm -rf {workdir} && echo 'Working directory removed.'" + ) + + error_pattern = rf"Failed to return to {workdir} after executing command" - mkdir(workdir) + mkdir(workdir, parents=True) with self.mocked_stdout_stderr(): - error_pattern = rf"Failed to return to {workdir} after executing command" self.assertErrorRegex(EasyBuildError, error_pattern, run_shell_cmd, cmd_workdir_rm, work_dir=workdir) From d45b4e5a0f5b28b2e4363c680b6e3f96eee5a9c4 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Tue, 18 Jun 2024 11:18:12 +0200 Subject: [PATCH 10/11] fix typo in log message of easybuild.tools.run Co-authored-by: Kenneth Hoste --- easybuild/tools/run.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py index 24715b58eb..57975abfeb 100644 --- a/easybuild/tools/run.py +++ b/easybuild/tools/run.py @@ -567,7 +567,7 @@ def to_cmd_str(cmd): os.getcwd() except FileNotFoundError: _log.warning( - f"Shell command `{cmd_str}` completed successfully but left the system in a unknown working directory. " + f"Shell command `{cmd_str}` completed successfully but left the system in an unknown working directory. " f"Changing back to initial working directory: {initial_work_dir}" ) try: From 38ca67d66427b967865304a6f9876b0e0d58df5b Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Wed, 19 Jun 2024 10:08:28 +0200 Subject: [PATCH 11/11] revert fallback of work dir passed to post_run_shell_cmd in case of non-existent cwd --- easybuild/tools/run.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py index 57975abfeb..fbfb36dc74 100644 --- a/easybuild/tools/run.py +++ b/easybuild/tools/run.py @@ -574,9 +574,6 @@ def to_cmd_str(cmd): os.chdir(initial_work_dir) except OSError as err: raise EasyBuildError(f"Failed to return to {initial_work_dir} after executing command `{cmd_str}`: {err}") - else: - if not os.path.isdir(work_dir): - work_dir = initial_work_dir if with_hooks: run_hook_kwargs = { @@ -584,7 +581,7 @@ def to_cmd_str(cmd): 'interactive': interactive, 'output': res.output, 'stderr': res.stderr, - 'work_dir': work_dir, + 'work_dir': res.work_dir, } run_hook(RUN_SHELL_CMD, hooks, post_step_hook=True, args=[cmd], kwargs=run_hook_kwargs)