From 4ff3663e6dbb9aa18a875acd00784559df2352cb Mon Sep 17 00:00:00 2001
From: Kenneth Hoste <kenneth.hoste@ugent.be>
Date: Tue, 13 Feb 2024 21:26:43 +0100
Subject: [PATCH 01/17] implement basic support for qa_patterns in
 run_shell_cmd

---
 easybuild/tools/run.py | 36 +++++++++++++++++++++++++++++++-----
 test/framework/run.py  | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 5 deletions(-)

diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py
index 95391f3821..da259a8721 100644
--- a/easybuild/tools/run.py
+++ b/easybuild/tools/run.py
@@ -36,6 +36,7 @@
 * Ward Poelmans (Ghent University)
 """
 import contextlib
+import fcntl
 import functools
 import inspect
 import os
@@ -231,7 +232,7 @@ def to_cmd_str(cmd):
         return cmd_str
 
     # temporarily raise a NotImplementedError until all options are implemented
-    if qa_patterns or qa_wait_patterns:
+    if qa_wait_patterns:
         raise NotImplementedError
 
     if work_dir is None:
@@ -315,23 +316,48 @@ def to_cmd_str(cmd):
     if stdin:
         stdin = stdin.encode()
 
-    if stream_output:
+    if stream_output or qa_patterns:
+
+        if qa_patterns:
+            # make stdout, stderr, stdin non-blocking files
+            channels = [proc.stdout, proc.stdin]
+            if split_stderr:
+                channels += proc.stderr
+            for channel in channels:
+                fd = channel.fileno()
+                flags = fcntl.fcntl(fd, fcntl.F_GETFL)
+                fcntl.fcntl(fd, fcntl.F_SETFL, flags | os.O_NONBLOCK)
+
         if stdin:
             proc.stdin.write(stdin)
 
         exit_code = None
         stdout, stderr = b'', b''
 
+        # collect output piece-wise, while checking for questions to answer (if qa_patterns is provided)
         while exit_code is None:
-            exit_code = proc.poll()
 
             # use small read size (128 bytes) when streaming output, to make it stream more fluently
             # -1 means reading until EOF
             read_size = 128 if exit_code is None else -1
 
-            stdout += proc.stdout.read(read_size)
+            exit_code = proc.poll()
+
+            more_stdout = proc.stdout.read1(read_size) or b''
+            stdout += more_stdout
+
+            # note: we assume that there won't be any questions in stderr output
             if split_stderr:
-                stderr += proc.stderr.read(read_size)
+                stderr += proc.stderr.read1(read_size) or b''
+
+            # only consider answering questions if there's new output beyond additional whitespace
+            if more_stdout.strip() and qa_patterns:
+                for question, answer in qa_patterns:
+                    question += '[\s\n]*$'
+                    regex = re.compile(question.encode())
+                    if regex.search(stdout):
+                        answer += '\n'
+                        x= os.write(proc.stdin.fileno(), answer.encode())
     else:
         (stdout, stderr) = proc.communicate(input=stdin)
 
diff --git a/test/framework/run.py b/test/framework/run.py
index c86d8635ed..887a9b0030 100644
--- a/test/framework/run.py
+++ b/test/framework/run.py
@@ -750,6 +750,40 @@ def test_run_cmd_qa(self):
         self.assertTrue(out.startswith("question\nanswer\nfoo "))
         self.assertTrue(out.endswith('bar'))
 
+    def test_run_shell_cmd_qa(self):
+        """Basic test for Q&A support in run_shell_cmd function."""
+
+        cmd = '; '.join([
+            "echo question1",
+            "read x",
+            "echo $x",
+            "echo question2",
+            "read y",
+            "echo $y",
+        ])
+        qa = [
+            ('question1', 'answer1'),
+            ('question2', 'answer2'),
+        ]
+        with self.mocked_stdout_stderr():
+            res = run_shell_cmd(cmd, qa_patterns=qa)
+        self.assertEqual(res.output, "question1\nanswer1\nquestion2\nanswer2\n")
+        # no reason echo hello could fail
+        self.assertEqual(res.exit_code, 0)
+
+        # test running command that emits non-UTF8 characters
+        # this is constructed to reproduce errors like:
+        # UnicodeDecodeError: 'utf-8' codec can't decode byte 0xe2
+        test_file = os.path.join(self.test_prefix, 'foo.txt')
+        write_file(test_file, b"foo \xe2 bar")
+        cmd += "; cat %s" % test_file
+
+        with self.mocked_stdout_stderr():
+             res = run_shell_cmd(cmd, qa_patterns=qa)
+        self.assertEqual(res.exit_code, 0)
+        self.assertTrue(res.output.startswith("question1\nanswer1\nquestion2\nanswer2\nfoo "))
+        self.assertTrue(res.output.endswith('bar'))
+
     def test_run_cmd_qa_buffering(self):
         """Test whether run_cmd_qa uses unbuffered output."""
 

From 1cce87361f6465f7524b10e5d3ac9887e93dae03 Mon Sep 17 00:00:00 2001
From: Kenneth Hoste <kenneth.hoste@ugent.be>
Date: Wed, 14 Feb 2024 20:51:12 +0100
Subject: [PATCH 02/17] add support for qa_timeout in run_shell_cmd + check
 type of qa_patterns value

---
 easybuild/tools/run.py | 23 ++++++++++++++++++++-
 test/framework/run.py  | 47 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py
index da259a8721..538c952cfb 100644
--- a/easybuild/tools/run.py
+++ b/easybuild/tools/run.py
@@ -192,7 +192,7 @@ def cache_aware_func(cmd, *args, **kwargs):
 def run_shell_cmd(cmd, fail_on_error=True, split_stderr=False, stdin=None, env=None,
                   hidden=False, in_dry_run=False, verbose_dry_run=False, work_dir=None, use_bash=True,
                   output_file=True, stream_output=None, asynchronous=False, task_id=None, with_hooks=True,
-                  qa_patterns=None, qa_wait_patterns=None):
+                  qa_patterns=None, qa_wait_patterns=None, qa_timeout=10000):
     """
     Run specified (interactive) shell command, and capture output + exit code.
 
@@ -213,6 +213,8 @@ def run_shell_cmd(cmd, fail_on_error=True, split_stderr=False, stdin=None, env=N
     :param qa_patterns: list of 2-tuples with patterns for questions + corresponding answers
     :param qa_wait_patterns: list of 2-tuples with patterns for non-questions
                              and number of iterations to allow these patterns to match with end out command output
+    :param qa_timeout: amount of milliseconds to wait until more output is produced when there is no matching question
+
     :return: Named tuple with:
     - output: command output, stdout+stderr combined if split_stderr is disabled, only stdout otherwise
     - exit_code: exit code of command (integer)
@@ -231,6 +233,11 @@ def to_cmd_str(cmd):
 
         return cmd_str
 
+    # make sure that qa_patterns is a list of 2-tuples (not a dict, or something else)
+    if qa_patterns:
+        if not isinstance(qa_patterns, list) or any(not isinstance(x, tuple) or len(x) != 2 for x in qa_patterns):
+            raise EasyBuildError("qa_patterns passed to run_shell_cmd should be a list of 2-tuples!")
+
     # temporarily raise a NotImplementedError until all options are implemented
     if qa_wait_patterns:
         raise NotImplementedError
@@ -333,6 +340,8 @@ def to_cmd_str(cmd):
 
         exit_code = None
         stdout, stderr = b'', b''
+        check_interval_secs = 0.001
+        time_no_match = 0
 
         # collect output piece-wise, while checking for questions to answer (if qa_patterns is provided)
         while exit_code is None:
@@ -351,6 +360,7 @@ def to_cmd_str(cmd):
                 stderr += proc.stderr.read1(read_size) or b''
 
             # only consider answering questions if there's new output beyond additional whitespace
+            hit = False
             if more_stdout.strip() and qa_patterns:
                 for question, answer in qa_patterns:
                     question += '[\s\n]*$'
@@ -358,6 +368,17 @@ def to_cmd_str(cmd):
                     if regex.search(stdout):
                         answer += '\n'
                         x= os.write(proc.stdin.fileno(), answer.encode())
+                        hit = True
+
+            time.sleep(check_interval_secs)
+            if hit:
+                time_no_match = 0
+            else:
+                time_no_match += check_interval_secs
+                if time_no_match >= qa_timeout:
+                    error_msg = "No matching questions found for current command output, "
+                    error_msg += f"giving up after {qa_timeout} seconds!"
+                    raise EasyBuildError(error_msg)
     else:
         (stdout, stderr) = proc.communicate(input=stdin)
 
diff --git a/test/framework/run.py b/test/framework/run.py
index 887a9b0030..0dd1b4888b 100644
--- a/test/framework/run.py
+++ b/test/framework/run.py
@@ -784,6 +784,21 @@ def test_run_shell_cmd_qa(self):
         self.assertTrue(res.output.startswith("question1\nanswer1\nquestion2\nanswer2\nfoo "))
         self.assertTrue(res.output.endswith('bar'))
 
+        # check type check on qa_patterns
+        error_pattern = "qa_patterns passed to run_shell_cmd should be a list of 2-tuples!"
+        with self.mocked_stdout_stderr():
+            self.assertErrorRegex(EasyBuildError, error_pattern, run_shell_cmd, cmd, qa_patterns={'foo': 'bar'})
+            self.assertErrorRegex(EasyBuildError, error_pattern, run_shell_cmd, cmd, qa_patterns=('foo', 'bar'))
+            self.assertErrorRegex(EasyBuildError, error_pattern, run_shell_cmd, cmd, qa_patterns=(('foo', 'bar'),))
+            self.assertErrorRegex(EasyBuildError, error_pattern, run_shell_cmd, cmd, qa_patterns='foo:bar')
+            self.assertErrorRegex(EasyBuildError, error_pattern, run_shell_cmd, cmd, qa_patterns=['foo:bar'])
+
+        # validate use of qa_timeout to give up if there's no matching question for too long
+        cmd = "sleep 3; echo 'question'; read a; echo $a"
+        error_pattern = "No matching questions found for current command output, giving up after 1 seconds!"
+        with self.mocked_stdout_stderr():
+            self.assertErrorRegex(EasyBuildError, error_pattern, run_shell_cmd, cmd, qa_patterns=qa, qa_timeout=1)
+
     def test_run_cmd_qa_buffering(self):
         """Test whether run_cmd_qa uses unbuffered output."""
 
@@ -816,6 +831,38 @@ def test_run_cmd_qa_buffering(self):
         self.assertEqual(ec, 1)
         self.assertEqual(out, "Hello, I am about to exit\nERROR: I failed\n")
 
+    def test_run_shell_cmd_qa_buffering(self):
+        """Test whether run_shell_cmd uses unbuffered output when running interactive commands."""
+
+        # command that generates a lot of output before waiting for input
+        # note: bug being fixed can be reproduced reliably using 1000, but not with too high values like 100000!
+        cmd = 'for x in $(seq 1000); do echo "This is a number you can pick: $x"; done; '
+        cmd += 'echo "Pick a number: "; read number; echo "Picked number: $number"'
+        with self.mocked_stdout_stderr():
+            res = run_shell_cmd(cmd, qa_patterns=[('Pick a number: ', '42')], qa_timeout=10)
+
+        self.assertEqual(res.exit_code, 0)
+        regex = re.compile("Picked number: 42$")
+        self.assertTrue(regex.search(res.output), f"Pattern '{regex.pattern}' found in: {res.output}")
+
+        # also test with script run as interactive command that quickly exits with non-zero exit code;
+        # see https://github.com/easybuilders/easybuild-framework/issues/3593
+        script_txt = '\n'.join([
+            "#/bin/bash",
+            "echo 'Hello, I am about to exit'",
+            "echo 'ERROR: I failed' >&2",
+            "exit 1",
+        ])
+        script = os.path.join(self.test_prefix, 'test.sh')
+        write_file(script, script_txt)
+        adjust_permissions(script, stat.S_IXUSR)
+
+        with self.mocked_stdout_stderr():
+            res = run_shell_cmd(script, qa_patterns=[], fail_on_error=False)
+
+        self.assertEqual(res.exit_code, 1)
+        self.assertEqual(res.output, "Hello, I am about to exit\nERROR: I failed\n")
+
     def test_run_cmd_qa_log_all(self):
         """Test run_cmd_qa with log_output enabled"""
         with self.mocked_stdout_stderr():

From 651da9715f3f30a038a56e4d940ec5d21a3d0928 Mon Sep 17 00:00:00 2001
From: Kenneth Hoste <kenneth.hoste@ugent.be>
Date: Wed, 14 Feb 2024 20:54:34 +0100
Subject: [PATCH 03/17] fix trivial code style issues

---
 easybuild/tools/run.py | 4 ++--
 test/framework/run.py  | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py
index 538c952cfb..78aeab85d8 100644
--- a/easybuild/tools/run.py
+++ b/easybuild/tools/run.py
@@ -363,11 +363,11 @@ def to_cmd_str(cmd):
             hit = False
             if more_stdout.strip() and qa_patterns:
                 for question, answer in qa_patterns:
-                    question += '[\s\n]*$'
+                    question += r'[\s\n]*$'
                     regex = re.compile(question.encode())
                     if regex.search(stdout):
                         answer += '\n'
-                        x= os.write(proc.stdin.fileno(), answer.encode())
+                        os.write(proc.stdin.fileno(), answer.encode())
                         hit = True
 
             time.sleep(check_interval_secs)
diff --git a/test/framework/run.py b/test/framework/run.py
index 0dd1b4888b..5697f39e91 100644
--- a/test/framework/run.py
+++ b/test/framework/run.py
@@ -779,7 +779,7 @@ def test_run_shell_cmd_qa(self):
         cmd += "; cat %s" % test_file
 
         with self.mocked_stdout_stderr():
-             res = run_shell_cmd(cmd, qa_patterns=qa)
+            res = run_shell_cmd(cmd, qa_patterns=qa)
         self.assertEqual(res.exit_code, 0)
         self.assertTrue(res.output.startswith("question1\nanswer1\nquestion2\nanswer2\nfoo "))
         self.assertTrue(res.output.endswith('bar'))

From f01ef65d3cdb0bfa3a3fd80a33bbe874095dd512 Mon Sep 17 00:00:00 2001
From: Kenneth Hoste <kenneth.hoste@ugent.be>
Date: Wed, 14 Feb 2024 21:01:54 +0100
Subject: [PATCH 04/17] simplify qa_timeout code in run_shell_cmd by using
 for-else statement

---
 easybuild/tools/run.py | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py
index 78aeab85d8..93e4afa65d 100644
--- a/easybuild/tools/run.py
+++ b/easybuild/tools/run.py
@@ -360,25 +360,24 @@ def to_cmd_str(cmd):
                 stderr += proc.stderr.read1(read_size) or b''
 
             # only consider answering questions if there's new output beyond additional whitespace
-            hit = False
-            if more_stdout.strip() and qa_patterns:
+            if qa_patterns:
                 for question, answer in qa_patterns:
                     question += r'[\s\n]*$'
                     regex = re.compile(question.encode())
                     if regex.search(stdout):
                         answer += '\n'
                         os.write(proc.stdin.fileno(), answer.encode())
-                        hit = True
+                        time_no_match = 0
+                        break
+                else:
+                    # this will only run if the for loop above was *not* stopped by the break statement
+                    time_no_match += check_interval_secs
+                    if time_no_match > qa_timeout:
+                        error_msg = "No matching questions found for current command output, "
+                        error_msg += f"giving up after {qa_timeout} seconds!"
+                        raise EasyBuildError(error_msg)
 
             time.sleep(check_interval_secs)
-            if hit:
-                time_no_match = 0
-            else:
-                time_no_match += check_interval_secs
-                if time_no_match >= qa_timeout:
-                    error_msg = "No matching questions found for current command output, "
-                    error_msg += f"giving up after {qa_timeout} seconds!"
-                    raise EasyBuildError(error_msg)
     else:
         (stdout, stderr) = proc.communicate(input=stdin)
 

From 5899ce284ef45dce7c648ace17a2b95487e4765a Mon Sep 17 00:00:00 2001
From: Kenneth Hoste <kenneth.hoste@ugent.be>
Date: Wed, 14 Feb 2024 21:24:29 +0100
Subject: [PATCH 05/17] fix default value for qa_timeout (we're counting
 seconds, not milliseconds)

---
 easybuild/tools/run.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py
index 93e4afa65d..1ffcd9054d 100644
--- a/easybuild/tools/run.py
+++ b/easybuild/tools/run.py
@@ -192,7 +192,7 @@ def cache_aware_func(cmd, *args, **kwargs):
 def run_shell_cmd(cmd, fail_on_error=True, split_stderr=False, stdin=None, env=None,
                   hidden=False, in_dry_run=False, verbose_dry_run=False, work_dir=None, use_bash=True,
                   output_file=True, stream_output=None, asynchronous=False, task_id=None, with_hooks=True,
-                  qa_patterns=None, qa_wait_patterns=None, qa_timeout=10000):
+                  qa_patterns=None, qa_wait_patterns=None, qa_timeout=100):
     """
     Run specified (interactive) shell command, and capture output + exit code.
 
@@ -213,7 +213,7 @@ def run_shell_cmd(cmd, fail_on_error=True, split_stderr=False, stdin=None, env=N
     :param qa_patterns: list of 2-tuples with patterns for questions + corresponding answers
     :param qa_wait_patterns: list of 2-tuples with patterns for non-questions
                              and number of iterations to allow these patterns to match with end out command output
-    :param qa_timeout: amount of milliseconds to wait until more output is produced when there is no matching question
+    :param qa_timeout: amount of seconds to wait until more output is produced when there is no matching question
 
     :return: Named tuple with:
     - output: command output, stdout+stderr combined if split_stderr is disabled, only stdout otherwise

From cfb7f3ad9edd9cdbc7f8515a5d5157cdfe519257 Mon Sep 17 00:00:00 2001
From: Kenneth Hoste <kenneth.hoste@ugent.be>
Date: Wed, 14 Feb 2024 21:39:04 +0100
Subject: [PATCH 06/17] fix trace message for interactive commands run with
 run_shell_cmd

---
 easybuild/tools/run.py | 14 +++++++-----
 test/framework/run.py  | 48 ++++++++++++++++++++++++++++++++++++++----
 2 files changed, 53 insertions(+), 9 deletions(-)

diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py
index 1ffcd9054d..5a8bc298ff 100644
--- a/easybuild/tools/run.py
+++ b/easybuild/tools/run.py
@@ -277,7 +277,8 @@ def to_cmd_str(cmd):
     if not in_dry_run and build_option('extended_dry_run'):
         if not hidden or verbose_dry_run:
             silent = build_option('silent')
-            msg = f"  running shell command \"{cmd_str}\"\n"
+            interactive = 'interactive ' if qa_patterns else ''
+            msg = f"  running {interactive}shell command \"{cmd_str}\"\n"
             msg += f"  (in {work_dir})"
             dry_run_msg(msg, silent=silent)
 
@@ -286,7 +287,8 @@ def to_cmd_str(cmd):
 
     start_time = datetime.now()
     if not hidden:
-        _cmd_trace_msg(cmd_str, start_time, work_dir, stdin, cmd_out_fp, cmd_err_fp, thread_id)
+        _cmd_trace_msg(cmd_str, start_time, work_dir, stdin, cmd_out_fp, cmd_err_fp, thread_id,
+                       interactive=bool(qa_patterns))
 
     if stream_output:
         print_msg(f"(streaming) output for command '{cmd_str}':")
@@ -430,7 +432,7 @@ def to_cmd_str(cmd):
     return res
 
 
-def _cmd_trace_msg(cmd, start_time, work_dir, stdin, cmd_out_fp, cmd_err_fp, thread_id):
+def _cmd_trace_msg(cmd, start_time, work_dir, stdin, cmd_out_fp, cmd_err_fp, thread_id, interactive=False):
     """
     Helper function to construct and print trace message for command being run
 
@@ -441,13 +443,15 @@ def _cmd_trace_msg(cmd, start_time, work_dir, stdin, cmd_out_fp, cmd_err_fp, thr
     :param cmd_out_fp: path to output file for command
     :param cmd_err_fp: path to errors/warnings output file for command
     :param thread_id: thread ID (None when not running shell command asynchronously)
+    :param interactive: boolean indicating whether it is an interactive command, or not
     """
     start_time = start_time.strftime('%Y-%m-%d %H:%M:%S')
 
+    interactive = 'interactive ' if interactive else ''
     if thread_id:
-        run_cmd_msg = f"running shell command (asynchronously, thread ID: {thread_id}):"
+        run_cmd_msg = f"running {interactive}shell command (asynchronously, thread ID: {thread_id}):"
     else:
-        run_cmd_msg = "running shell command:"
+        run_cmd_msg = f"running {interactive}shell command:"
 
     lines = [
         run_cmd_msg,
diff --git a/test/framework/run.py b/test/framework/run.py
index 5697f39e91..669b8fa073 100644
--- a/test/framework/run.py
+++ b/test/framework/run.py
@@ -876,13 +876,22 @@ def test_run_cmd_qa_log_all(self):
         extra_pref = "# output for interactive command: echo 'n: '; read n; seq 1 $n\n\n"
         self.assertEqual(run_cmd_log_txt, extra_pref + "n: \n1\n2\n3\n4\n5\n")
 
+    def test_run_shell_cmd_qa_log(self):
+        """Test temporary log file for run_shell_cmd with qa_patterns"""
+        with self.mocked_stdout_stderr():
+            res = run_shell_cmd("echo 'n: '; read n; seq 1 $n", qa_patterns=[('n:', '5')])
+        self.assertEqual(res.exit_code, 0)
+        self.assertEqual(res.output, "n: \n1\n2\n3\n4\n5\n")
+
+        run_cmd_logs = glob.glob(os.path.join(tempfile.gettempdir(), 'run-shell-cmd-output', 'echo-*', 'out.txt'))
+        self.assertEqual(len(run_cmd_logs), 1)
+        run_cmd_log_txt = read_file(run_cmd_logs[0])
+        self.assertEqual(run_cmd_log_txt, "n: \n1\n2\n3\n4\n5\n")
+
     def test_run_cmd_qa_trace(self):
         """Test run_cmd under --trace"""
-        # replace log.experimental with log.warning to allow experimental code
-        easybuild.tools.utilities._log.experimental = easybuild.tools.utilities._log.warning
-
-        init_config(build_options={'trace': True})
 
+        # --trace is enabled by default
         self.mock_stdout(True)
         self.mock_stderr(True)
         (out, ec) = run_cmd_qa("echo 'n: '; read n; seq 1 $n", {'n: ': '5'})
@@ -910,6 +919,37 @@ def test_run_cmd_qa_trace(self):
         self.assertEqual(stdout, '')
         self.assertEqual(stderr, '')
 
+    def test_run_shell_cmd_qa_trace(self):
+        """Test run_shell_cmd with qa_patterns under --trace"""
+
+        # --trace is enabled by default
+        self.mock_stdout(True)
+        self.mock_stderr(True)
+        run_shell_cmd("echo 'n: '; read n; seq 1 $n", qa_patterns=[('n: ', '5')])
+        stdout = self.get_stdout()
+        stderr = self.get_stderr()
+        self.mock_stdout(False)
+        self.mock_stderr(False)
+        self.assertEqual(stderr, '')
+        pattern = r"^  >> running interactive shell command:\n"
+        pattern += r"\techo \'n: \'; read n; seq 1 \$n\n"
+        pattern += r"\t\[started at: .*\]\n"
+        pattern += r"\t\[working dir: .*\]\n"
+        pattern += r"\t\[output saved to .*\]\n"
+        pattern += r'  >> command completed: exit 0, ran in .*'
+        self.assertTrue(re.search(pattern, stdout), "Pattern '%s' found in: %s" % (pattern, stdout))
+
+        # trace output can be disabled on a per-command basis
+        self.mock_stdout(True)
+        self.mock_stderr(True)
+        run_shell_cmd("echo 'n: '; read n; seq 1 $n", qa_patterns=[('n: ', '5')], hidden=True)
+        stdout = self.get_stdout()
+        stderr = self.get_stderr()
+        self.mock_stdout(False)
+        self.mock_stderr(False)
+        self.assertEqual(stdout, '')
+        self.assertEqual(stderr, '')
+
     def test_run_cmd_qa_answers(self):
         """Test providing list of answers in run_cmd_qa."""
         cmd = "echo question; read x; echo $x; " * 2

From 4b63b913ef88b402036757dc2524b91f3b866bb4 Mon Sep 17 00:00:00 2001
From: Kenneth Hoste <kenneth.hoste@ugent.be>
Date: Wed, 21 Feb 2024 16:46:51 +0100
Subject: [PATCH 07/17] make sure that *all* stdout/stderr output is read when
 streaming output or running interactive commands

---
 easybuild/tools/run.py | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py
index 5a8bc298ff..09a699ca80 100644
--- a/easybuild/tools/run.py
+++ b/easybuild/tools/run.py
@@ -352,8 +352,6 @@ def to_cmd_str(cmd):
             # -1 means reading until EOF
             read_size = 128 if exit_code is None else -1
 
-            exit_code = proc.poll()
-
             more_stdout = proc.stdout.read1(read_size) or b''
             stdout += more_stdout
 
@@ -380,6 +378,12 @@ def to_cmd_str(cmd):
                         raise EasyBuildError(error_msg)
 
             time.sleep(check_interval_secs)
+
+            exit_code = proc.poll()
+
+        stdout += proc.stdout.read()
+        if split_stderr:
+            stderr += proc.stderr.read()
     else:
         (stdout, stderr) = proc.communicate(input=stdin)
 

From 57839104045f839476536330a0cb53da8d586bc4 Mon Sep 17 00:00:00 2001
From: Kenneth Hoste <kenneth.hoste@ugent.be>
Date: Wed, 6 Mar 2024 12:03:03 +0100
Subject: [PATCH 08/17] add support to run_shell_cmd for providing list of
 answers in qa_patterns + fix output for hooks triggered for interactive shell
 commands run with run_shell_cmd

---
 easybuild/tools/run.py | 20 ++++++++++++++++++--
 test/framework/run.py  | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py
index c9a4b3d06f..ba40b704ac 100644
--- a/easybuild/tools/run.py
+++ b/easybuild/tools/run.py
@@ -319,7 +319,11 @@ def to_cmd_str(cmd):
 
     if with_hooks:
         hooks = load_hooks(build_option('hooks'))
-        hook_res = run_hook(RUN_SHELL_CMD, hooks, pre_step_hook=True, args=[cmd], kwargs={'work_dir': work_dir})
+        kwargs = {
+            'interactive': bool(qa_patterns),
+            'work_dir': work_dir,
+        }
+        hook_res = run_hook(RUN_SHELL_CMD, hooks, pre_step_hook=True, args=[cmd], kwargs=kwargs)
         if hook_res:
             cmd, old_cmd = hook_res, cmd
             cmd_str = to_cmd_str(cmd)
@@ -375,10 +379,21 @@ def to_cmd_str(cmd):
 
             # only consider answering questions if there's new output beyond additional whitespace
             if qa_patterns:
-                for question, answer in qa_patterns:
+                for question, answers in qa_patterns:
+
                     question += r'[\s\n]*$'
                     regex = re.compile(question.encode())
                     if regex.search(stdout):
+                        # if answer is specified as a list, we take the first item as current answer,
+                        # and add it to the back of the list (so we cycle through answers)
+                        if isinstance(answers, list):
+                            answer = answers.pop(0)
+                            answers.append(answer)
+                        elif isinstance(answers, str):
+                            answer = answers
+                        else:
+                            raise EasyBuildError(f"Unknown type of answers encountered: {answers}")
+
                         answer += '\n'
                         os.write(proc.stdin.fileno(), answer.encode())
                         time_no_match = 0
@@ -437,6 +452,7 @@ def to_cmd_str(cmd):
     if with_hooks:
         run_hook_kwargs = {
             'exit_code': res.exit_code,
+            'interactive': bool(qa_patterns),
             'output': res.output,
             'stderr': res.stderr,
             'work_dir': res.work_dir,
diff --git a/test/framework/run.py b/test/framework/run.py
index aa22ef88fd..6fb8d2b072 100644
--- a/test/framework/run.py
+++ b/test/framework/run.py
@@ -996,6 +996,27 @@ def test_run_cmd_qa_answers(self):
         self.assertEqual(out, "question\nanswer1\nquestion\nanswer2\n" * 2)
         self.assertEqual(ec, 0)
 
+    def test_run_shell_cmd_qa_answers(self):
+        """Test providing list of answers for a question in run_shell_cmd."""
+
+        cmd = "echo question; read x; echo $x; " * 2
+        qa = [("question", ["answer1", "answer2"])]
+
+        with self.mocked_stdout_stderr():
+            res = run_shell_cmd(cmd, qa_patterns=qa)
+        self.assertEqual(res.output, "question\nanswer1\nquestion\nanswer2\n")
+        self.assertEqual(res.exit_code, 0)
+
+        with self.mocked_stdout_stderr():
+            self.assertErrorRegex(EasyBuildError, "Unknown type of answers encountered", run_shell_cmd, cmd, qa_patterns=[('question', 1)])
+
+        # test cycling of answers
+        cmd = cmd * 2
+        with self.mocked_stdout_stderr():
+            res = run_shell_cmd(cmd, qa_patterns=qa)
+        self.assertEqual(res.output, "question\nanswer1\nquestion\nanswer2\n" * 2)
+        self.assertEqual(res.exit_code, 0)
+
     def test_run_cmd_simple(self):
         """Test return value for run_cmd in 'simple' mode."""
         with self.mocked_stdout_stderr():
@@ -1124,6 +1145,14 @@ def test_run_cmd_dry_run(self):
         self.assertEqual(read_file(outfile), "This is always echoed\n")
 
         # Q&A commands
+        self.mock_stdout(True)
+        run_shell_cmd("some_qa_cmd", qa_patterns=[('question1', 'answer1')])
+        stdout = self.get_stdout()
+        self.mock_stdout(False)
+
+        expected = """  running interactive shell command "some_qa_cmd"\n"""
+        self.assertIn(expected, stdout)
+
         self.mock_stdout(True)
         run_cmd_qa("some_qa_cmd", {'question1': 'answer1'})
         stdout = self.get_stdout()
@@ -1565,6 +1594,17 @@ def post_run_shell_cmd_hook(cmd, *args, **kwargs):
         ])
         self.assertEqual(stdout, expected_stdout)
 
+        with self.mocked_stdout_stderr():
+            run_shell_cmd("sleep 2; make", qa_patterns=[('q', 'a')])
+            stdout = self.get_stdout()
+
+        expected_stdout = '\n'.join([
+            "pre-run hook interactive 'sleep 2; make' in %s" % cwd,
+            "post-run hook interactive 'sleep 2; echo make' (exit code: 0, output: 'make\n')",
+            '',
+        ])
+        self.assertEqual(stdout, expected_stdout)
+
         with self.mocked_stdout_stderr():
             run_cmd_qa("sleep 2; make", qa={})
             stdout = self.get_stdout()

From 717f55576974348aca7e96475aeff62b29a8c187 Mon Sep 17 00:00:00 2001
From: Kenneth Hoste <kenneth.hoste@ugent.be>
Date: Wed, 3 Apr 2024 12:07:02 +0200
Subject: [PATCH 09/17] implement qa_wait_patterns option in run_shell_cmd

---
 easybuild/tools/run.py | 22 +++++++++++++++++-----
 test/framework/run.py  | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py
index ba40b704ac..4376fb0220 100644
--- a/easybuild/tools/run.py
+++ b/easybuild/tools/run.py
@@ -252,9 +252,8 @@ def to_cmd_str(cmd):
         if not isinstance(qa_patterns, list) or any(not isinstance(x, tuple) or len(x) != 2 for x in qa_patterns):
             raise EasyBuildError("qa_patterns passed to run_shell_cmd should be a list of 2-tuples!")
 
-    # temporarily raise a NotImplementedError until all options are implemented
-    if qa_wait_patterns:
-        raise NotImplementedError
+    if qa_wait_patterns is None:
+        qa_wait_patterns = []
 
     if work_dir is None:
         work_dir = os.getcwd()
@@ -377,10 +376,10 @@ def to_cmd_str(cmd):
             if split_stderr:
                 stderr += proc.stderr.read1(read_size) or b''
 
-            # only consider answering questions if there's new output beyond additional whitespace
             if qa_patterns:
+                match_found = False
                 for question, answers in qa_patterns:
-
+                    # allow extra whitespace at the end
                     question += r'[\s\n]*$'
                     regex = re.compile(question.encode())
                     if regex.search(stdout):
@@ -397,8 +396,21 @@ def to_cmd_str(cmd):
                         answer += '\n'
                         os.write(proc.stdin.fileno(), answer.encode())
                         time_no_match = 0
+                        match_found = True
                         break
                 else:
+                    # if no match was found among question patterns,
+                    # take into account patterns for non-questions (qa_wait_patterns)
+                    for pattern in qa_wait_patterns:
+                        # allow extra whitespace at the end
+                        pattern += r'[\s\n]*$'
+                        regex = re.compile(pattern.encode())
+                        if regex.search(stdout):
+                            time_no_match = 0
+                            match_found = True
+                            break
+
+                if not match_found:
                     # this will only run if the for loop above was *not* stopped by the break statement
                     time_no_match += check_interval_secs
                     if time_no_match > qa_timeout:
diff --git a/test/framework/run.py b/test/framework/run.py
index 6fb8d2b072..ba9a262925 100644
--- a/test/framework/run.py
+++ b/test/framework/run.py
@@ -771,6 +771,24 @@ def test_run_cmd_qa(self):
         self.assertTrue(out.startswith("question\nanswer\nfoo "))
         self.assertTrue(out.endswith('bar'))
 
+        # test handling of output that is not actually a question
+        cmd = ';'.join([
+            "echo not-a-question-but-a-statement",
+            "sleep 3",
+            "echo question",
+            "read x",
+            "echo $x",
+        ])
+        qa = {'question': 'answer'}
+
+        # fails because non-question is encountered
+        error_pattern = "Max nohits 1 reached: end of output not-a-question-but-a-statement"
+        self.assertErrorRegex(EasyBuildError, error_pattern, run_cmd_qa, cmd, qa, maxhits=1, trace=False)
+
+        (out, ec) = run_cmd_qa(cmd, qa, no_qa=["not-a-question-but-a-statement"], maxhits=1, trace=False)
+        self.assertEqual(out, "not-a-question-but-a-statement\nquestion\nanswer\n")
+        self.assertEqual(ec, 0)
+
     def test_run_shell_cmd_qa(self):
         """Basic test for Q&A support in run_shell_cmd function."""
 
@@ -820,6 +838,27 @@ def test_run_shell_cmd_qa(self):
         with self.mocked_stdout_stderr():
             self.assertErrorRegex(EasyBuildError, error_pattern, run_shell_cmd, cmd, qa_patterns=qa, qa_timeout=1)
 
+        # test handling of output that is not actually a question
+        cmd = ';'.join([
+            "echo not-a-question-but-a-statement",
+            "sleep 3",
+            "echo question",
+            "read x",
+            "echo $x",
+        ])
+        qa = [('question', 'answer')]
+
+        # fails because non-question is encountered
+        error_pattern = "No matching questions found for current command output, giving up after 1 seconds!"
+        self.assertErrorRegex(EasyBuildError, error_pattern, run_shell_cmd, cmd, qa_patterns=qa, qa_timeout=1,
+                              hidden=True)
+
+        qa_wait_patterns = ["not-a-question-but-a-statement"]
+        with self.mocked_stdout_stderr():
+            res = run_shell_cmd(cmd, qa_patterns=qa, qa_wait_patterns=qa_wait_patterns, qa_timeout=1)
+        self.assertEqual(res.exit_code, 0)
+        self.assertEqual(res.output, "not-a-question-but-a-statement\nquestion\nanswer\n")
+
     def test_run_cmd_qa_buffering(self):
         """Test whether run_cmd_qa uses unbuffered output."""
 

From f020bf901f71c6a3bad2b65318c751deb9e71c51 Mon Sep 17 00:00:00 2001
From: Kenneth Hoste <kenneth.hoste@ugent.be>
Date: Wed, 3 Apr 2024 12:08:11 +0200
Subject: [PATCH 10/17] fix excessively long line in
 test_run_shell_cmd_qa_answers

---
 test/framework/run.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/test/framework/run.py b/test/framework/run.py
index ba9a262925..0000a8efbd 100644
--- a/test/framework/run.py
+++ b/test/framework/run.py
@@ -1047,7 +1047,8 @@ def test_run_shell_cmd_qa_answers(self):
         self.assertEqual(res.exit_code, 0)
 
         with self.mocked_stdout_stderr():
-            self.assertErrorRegex(EasyBuildError, "Unknown type of answers encountered", run_shell_cmd, cmd, qa_patterns=[('question', 1)])
+            self.assertErrorRegex(EasyBuildError, "Unknown type of answers encountered", run_shell_cmd, cmd,
+                                  qa_patterns=[('question', 1)])
 
         # test cycling of answers
         cmd = cmd * 2

From 7d5c7ce51c9a4a852082df743dbbd4c90171a546 Mon Sep 17 00:00:00 2001
From: Kenneth Hoste <kenneth.hoste@ugent.be>
Date: Wed, 3 Apr 2024 12:21:58 +0200
Subject: [PATCH 11/17] add support to run_shell_cmd for completing answer via
 pattern extracted from question

---
 easybuild/tools/run.py |  5 ++++-
 test/framework/run.py  | 12 ++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py
index 4376fb0220..7fffe2ba6f 100644
--- a/easybuild/tools/run.py
+++ b/easybuild/tools/run.py
@@ -382,7 +382,8 @@ def to_cmd_str(cmd):
                     # allow extra whitespace at the end
                     question += r'[\s\n]*$'
                     regex = re.compile(question.encode())
-                    if regex.search(stdout):
+                    res = regex.search(stdout)
+                    if res:
                         # if answer is specified as a list, we take the first item as current answer,
                         # and add it to the back of the list (so we cycle through answers)
                         if isinstance(answers, list):
@@ -393,6 +394,8 @@ def to_cmd_str(cmd):
                         else:
                             raise EasyBuildError(f"Unknown type of answers encountered: {answers}")
 
+                        # answer may need to be completed via pattern extracted from question
+                        answer = answer % {k: v.decode() for (k, v) in res.groupdict().items()}
                         answer += '\n'
                         os.write(proc.stdin.fileno(), answer.encode())
                         time_no_match = 0
diff --git a/test/framework/run.py b/test/framework/run.py
index 0000a8efbd..6ec6ebfb10 100644
--- a/test/framework/run.py
+++ b/test/framework/run.py
@@ -838,6 +838,18 @@ def test_run_shell_cmd_qa(self):
         with self.mocked_stdout_stderr():
             self.assertErrorRegex(EasyBuildError, error_pattern, run_shell_cmd, cmd, qa_patterns=qa, qa_timeout=1)
 
+        # check using answer that is completed via pattern extracted from question
+        cmd = ';'.join([
+            "echo 'and the magic number is: 42'",
+            "read magic_number",
+            "echo $magic_number",
+        ])
+        qa = [("and the magic number is: (?P<nr>[0-9]+)", "%(nr)s")]
+        with self.mocked_stdout_stderr():
+            res = run_shell_cmd(cmd, qa_patterns=qa)
+        self.assertEqual(res.exit_code, 0)
+        self.assertEqual(res.output, "and the magic number is: 42\n42\n")
+
         # test handling of output that is not actually a question
         cmd = ';'.join([
             "echo not-a-question-but-a-statement",

From 7145ca9676404eba51cef4e815b7c5745209f569 Mon Sep 17 00:00:00 2001
From: Kenneth Hoste <kenneth.hoste@ugent.be>
Date: Wed, 3 Apr 2024 16:40:25 +0200
Subject: [PATCH 12/17] improve logging in Q&A part of run_shell_cmd

---
 easybuild/tools/run.py | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py
index 7fffe2ba6f..26f2deed5c 100644
--- a/easybuild/tools/run.py
+++ b/easybuild/tools/run.py
@@ -286,12 +286,14 @@ def to_cmd_str(cmd):
     else:
         cmd_out_fp, cmd_err_fp = None, None
 
+    interactive = bool(qa_patterns)
+    interactive_msg = 'interactive ' if interactive else ''
+
     # early exit in 'dry run' mode, after printing the command that would be run (unless 'hidden' is enabled)
     if not in_dry_run and build_option('extended_dry_run'):
         if not hidden or verbose_dry_run:
             silent = build_option('silent')
-            interactive = 'interactive ' if qa_patterns else ''
-            msg = f"  running {interactive}shell command \"{cmd_str}\"\n"
+            msg = f"  running {interactive_msg}shell command \"{cmd_str}\"\n"
             msg += f"  (in {work_dir})"
             dry_run_msg(msg, silent=silent)
 
@@ -300,8 +302,7 @@ def to_cmd_str(cmd):
 
     start_time = datetime.now()
     if not hidden:
-        _cmd_trace_msg(cmd_str, start_time, work_dir, stdin, cmd_out_fp, cmd_err_fp, thread_id,
-                       interactive=bool(qa_patterns))
+        _cmd_trace_msg(cmd_str, start_time, work_dir, stdin, cmd_out_fp, cmd_err_fp, thread_id, interactive=interactive)
 
     if stream_output:
         print_msg(f"(streaming) output for command '{cmd_str}':")
@@ -319,7 +320,7 @@ def to_cmd_str(cmd):
     if with_hooks:
         hooks = load_hooks(build_option('hooks'))
         kwargs = {
-            'interactive': bool(qa_patterns),
+            'interactive': interactive,
             'work_dir': work_dir,
         }
         hook_res = run_hook(RUN_SHELL_CMD, hooks, pre_step_hook=True, args=[cmd], kwargs=kwargs)
@@ -330,7 +331,7 @@ def to_cmd_str(cmd):
 
     stderr = subprocess.PIPE if split_stderr else subprocess.STDOUT
 
-    log_msg = f"Running shell command '{cmd_str}' in {work_dir}"
+    log_msg = f"Running {interactive_msg}shell command '{cmd_str}' in {work_dir}"
     if thread_id:
         log_msg += f" (via thread with ID {thread_id})"
     _log.info(log_msg)
@@ -384,6 +385,7 @@ def to_cmd_str(cmd):
                     regex = re.compile(question.encode())
                     res = regex.search(stdout)
                     if res:
+                        _log.debug(f"Found match for question pattern '{question}' at end of: {stdout}")
                         # if answer is specified as a list, we take the first item as current answer,
                         # and add it to the back of the list (so we cycle through answers)
                         if isinstance(answers, list):
@@ -395,13 +397,16 @@ def to_cmd_str(cmd):
                             raise EasyBuildError(f"Unknown type of answers encountered: {answers}")
 
                         # answer may need to be completed via pattern extracted from question
+                        _log.debug(f"Raw answer for question pattern '{question}': {answer}")
                         answer = answer % {k: v.decode() for (k, v) in res.groupdict().items()}
                         answer += '\n'
+                        _log.info(f"Found match for question pattern '{question}', replying with: {answer}")
                         os.write(proc.stdin.fileno(), answer.encode())
                         time_no_match = 0
                         match_found = True
                         break
                 else:
+                    _log.info("No match found for question patterns, considering question wait patterns")
                     # if no match was found among question patterns,
                     # take into account patterns for non-questions (qa_wait_patterns)
                     for pattern in qa_wait_patterns:
@@ -409,17 +414,24 @@ def to_cmd_str(cmd):
                         pattern += r'[\s\n]*$'
                         regex = re.compile(pattern.encode())
                         if regex.search(stdout):
+                            _log.info(f"Found match for question wait pattern '{pattern}'")
+                            _log.debug(f"Found match for question wait pattern '{pattern}' at end of: {stdout}")
                             time_no_match = 0
                             match_found = True
                             break
+                    else:
+                        _log.info("No match found for question wait patterns")
 
                 if not match_found:
+                    _log.debug(f"No match found in question or wait patterns at end of current output: {stdout}")
                     # this will only run if the for loop above was *not* stopped by the break statement
                     time_no_match += check_interval_secs
                     if time_no_match > qa_timeout:
                         error_msg = "No matching questions found for current command output, "
                         error_msg += f"giving up after {qa_timeout} seconds!"
                         raise EasyBuildError(error_msg)
+                    else:
+                        _log.debug(f"{time_no_match} seconds without match in output of interactive shell command")
 
             time.sleep(check_interval_secs)
 
@@ -467,7 +479,7 @@ def to_cmd_str(cmd):
     if with_hooks:
         run_hook_kwargs = {
             'exit_code': res.exit_code,
-            'interactive': bool(qa_patterns),
+            'interactive': interactive,
             'output': res.output,
             'stderr': res.stderr,
             'work_dir': res.work_dir,

From f3e5a650d7dacd52e72c052dbe963b2483280b00 Mon Sep 17 00:00:00 2001
From: Kenneth Hoste <kenneth.hoste@ugent.be>
Date: Wed, 3 Apr 2024 17:09:56 +0200
Subject: [PATCH 13/17] only show last 1000 characters in debug log message
 when answering questions for interactive commands in run_shell_cmd function

---
 easybuild/tools/run.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py
index 26f2deed5c..b63852b87c 100644
--- a/easybuild/tools/run.py
+++ b/easybuild/tools/run.py
@@ -385,7 +385,7 @@ def to_cmd_str(cmd):
                     regex = re.compile(question.encode())
                     res = regex.search(stdout)
                     if res:
-                        _log.debug(f"Found match for question pattern '{question}' at end of: {stdout}")
+                        _log.debug(f"Found match for question pattern '{question}' at end of stdout: {stdout[:1000]}")
                         # if answer is specified as a list, we take the first item as current answer,
                         # and add it to the back of the list (so we cycle through answers)
                         if isinstance(answers, list):
@@ -414,8 +414,8 @@ def to_cmd_str(cmd):
                         pattern += r'[\s\n]*$'
                         regex = re.compile(pattern.encode())
                         if regex.search(stdout):
-                            _log.info(f"Found match for question wait pattern '{pattern}'")
-                            _log.debug(f"Found match for question wait pattern '{pattern}' at end of: {stdout}")
+                            _log.info(f"Found match for wait pattern '{pattern}'")
+                            _log.debug(f"Found match for wait pattern '{pattern}' at end of stdout: {stdout[:1000]}")
                             time_no_match = 0
                             match_found = True
                             break
@@ -423,7 +423,7 @@ def to_cmd_str(cmd):
                         _log.info("No match found for question wait patterns")
 
                 if not match_found:
-                    _log.debug(f"No match found in question or wait patterns at end of current output: {stdout}")
+                    _log.debug(f"No match found in question/wait patterns at end of stdout: {stdout[:1000]}")
                     # this will only run if the for loop above was *not* stopped by the break statement
                     time_no_match += check_interval_secs
                     if time_no_match > qa_timeout:

From ca21ff655675c0a28bbbf41a47d515ba93be4c4b Mon Sep 17 00:00:00 2001
From: Kenneth Hoste <kenneth.hoste@ugent.be>
Date: Wed, 3 Apr 2024 18:13:45 +0200
Subject: [PATCH 14/17] only check for questions to be answered every 0.1s

---
 easybuild/tools/run.py | 4 ++--
 test/framework/run.py  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py
index b63852b87c..bb26dc9482 100644
--- a/easybuild/tools/run.py
+++ b/easybuild/tools/run.py
@@ -360,7 +360,7 @@ def to_cmd_str(cmd):
 
         exit_code = None
         stdout, stderr = b'', b''
-        check_interval_secs = 0.001
+        check_interval_secs = 0.1
         time_no_match = 0
 
         # collect output piece-wise, while checking for questions to answer (if qa_patterns is provided)
@@ -431,7 +431,7 @@ def to_cmd_str(cmd):
                         error_msg += f"giving up after {qa_timeout} seconds!"
                         raise EasyBuildError(error_msg)
                     else:
-                        _log.debug(f"{time_no_match} seconds without match in output of interactive shell command")
+                        _log.debug(f"{time_no_match:0.1f} seconds without match in output of interactive shell command")
 
             time.sleep(check_interval_secs)
 
diff --git a/test/framework/run.py b/test/framework/run.py
index 6ec6ebfb10..6e64764805 100644
--- a/test/framework/run.py
+++ b/test/framework/run.py
@@ -907,8 +907,8 @@ def test_run_shell_cmd_qa_buffering(self):
         """Test whether run_shell_cmd uses unbuffered output when running interactive commands."""
 
         # command that generates a lot of output before waiting for input
-        # note: bug being fixed can be reproduced reliably using 1000, but not with too high values like 100000!
-        cmd = 'for x in $(seq 1000); do echo "This is a number you can pick: $x"; done; '
+        # note: bug being fixed can be reproduced reliably using 100, but not with too high values like 100000!
+        cmd = 'for x in $(seq 100); do echo "This is a number you can pick: $x"; done; '
         cmd += 'echo "Pick a number: "; read number; echo "Picked number: $number"'
         with self.mocked_stdout_stderr():
             res = run_shell_cmd(cmd, qa_patterns=[('Pick a number: ', '42')], qa_timeout=10)

From 2abb706a839e49c022a2b621f572d6fae2e686eb Mon Sep 17 00:00:00 2001
From: Kenneth Hoste <kenneth.hoste@ugent.be>
Date: Wed, 3 Apr 2024 18:26:23 +0200
Subject: [PATCH 15/17] flesh out code to try and answer question raised by
 interactive shell command from run_shell_cmd into private helper function
 _answer_question

---
 easybuild/tools/run.py | 104 +++++++++++++++++++++++------------------
 1 file changed, 59 insertions(+), 45 deletions(-)

diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py
index bb26dc9482..24e860054b 100644
--- a/easybuild/tools/run.py
+++ b/easybuild/tools/run.py
@@ -202,6 +202,63 @@ def fileprefix_from_cmd(cmd, allowed_chars=False):
     return ''.join([c for c in cmd if c in allowed_chars])
 
 
+def _answer_question(stdout, proc, qa_patterns, qa_wait_patterns):
+    """
+    Private helper function to try and answer questions raised in interactive shell commands.
+    """
+    match_found = False
+
+    for question, answers in qa_patterns:
+        # allow extra whitespace at the end
+        question += r'[\s\n]*$'
+        regex = re.compile(question.encode())
+        res = regex.search(stdout)
+        if res:
+            _log.debug(f"Found match for question pattern '{question}' at end of stdout: {stdout[:1000]}")
+            # if answer is specified as a list, we take the first item as current answer,
+            # and add it to the back of the list (so we cycle through answers)
+            if isinstance(answers, list):
+                answer = answers.pop(0)
+                answers.append(answer)
+            elif isinstance(answers, str):
+                answer = answers
+            else:
+                raise EasyBuildError(f"Unknown type of answers encountered: {answers}")
+
+            # answer may need to be completed via pattern extracted from question
+            _log.debug(f"Raw answer for question pattern '{question}': {answer}")
+            answer = answer % {k: v.decode() for (k, v) in res.groupdict().items()}
+            answer += '\n'
+            _log.info(f"Found match for question pattern '{question}', replying with: {answer}")
+
+            try:
+                os.write(proc.stdin.fileno(), answer.encode())
+            except OSError as err:
+                raise EasyBuildError("Failed to answer question raised by interactive command: %s", err)
+
+            time_no_match = 0
+            match_found = True
+            break
+    else:
+        _log.info("No match found for question patterns, considering question wait patterns")
+        # if no match was found among question patterns,
+        # take into account patterns for non-questions (qa_wait_patterns)
+        for pattern in qa_wait_patterns:
+            # allow extra whitespace at the end
+            pattern += r'[\s\n]*$'
+            regex = re.compile(pattern.encode())
+            if regex.search(stdout):
+                _log.info(f"Found match for wait pattern '{pattern}'")
+                _log.debug(f"Found match for wait pattern '{pattern}' at end of stdout: {stdout[:1000]}")
+                time_no_match = 0
+                match_found = True
+                break
+        else:
+            _log.info("No match found for question wait patterns")
+
+    return match_found
+
+
 @run_shell_cmd_cache
 def run_shell_cmd(cmd, fail_on_error=True, split_stderr=False, stdin=None, env=None,
                   hidden=False, in_dry_run=False, verbose_dry_run=False, work_dir=None, use_bash=True,
@@ -378,51 +435,7 @@ def to_cmd_str(cmd):
                 stderr += proc.stderr.read1(read_size) or b''
 
             if qa_patterns:
-                match_found = False
-                for question, answers in qa_patterns:
-                    # allow extra whitespace at the end
-                    question += r'[\s\n]*$'
-                    regex = re.compile(question.encode())
-                    res = regex.search(stdout)
-                    if res:
-                        _log.debug(f"Found match for question pattern '{question}' at end of stdout: {stdout[:1000]}")
-                        # if answer is specified as a list, we take the first item as current answer,
-                        # and add it to the back of the list (so we cycle through answers)
-                        if isinstance(answers, list):
-                            answer = answers.pop(0)
-                            answers.append(answer)
-                        elif isinstance(answers, str):
-                            answer = answers
-                        else:
-                            raise EasyBuildError(f"Unknown type of answers encountered: {answers}")
-
-                        # answer may need to be completed via pattern extracted from question
-                        _log.debug(f"Raw answer for question pattern '{question}': {answer}")
-                        answer = answer % {k: v.decode() for (k, v) in res.groupdict().items()}
-                        answer += '\n'
-                        _log.info(f"Found match for question pattern '{question}', replying with: {answer}")
-                        os.write(proc.stdin.fileno(), answer.encode())
-                        time_no_match = 0
-                        match_found = True
-                        break
-                else:
-                    _log.info("No match found for question patterns, considering question wait patterns")
-                    # if no match was found among question patterns,
-                    # take into account patterns for non-questions (qa_wait_patterns)
-                    for pattern in qa_wait_patterns:
-                        # allow extra whitespace at the end
-                        pattern += r'[\s\n]*$'
-                        regex = re.compile(pattern.encode())
-                        if regex.search(stdout):
-                            _log.info(f"Found match for wait pattern '{pattern}'")
-                            _log.debug(f"Found match for wait pattern '{pattern}' at end of stdout: {stdout[:1000]}")
-                            time_no_match = 0
-                            match_found = True
-                            break
-                    else:
-                        _log.info("No match found for question wait patterns")
-
-                if not match_found:
+                if not _answer_question(stdout, proc, qa_patterns, qa_wait_patterns):
                     _log.debug(f"No match found in question/wait patterns at end of stdout: {stdout[:1000]}")
                     # this will only run if the for loop above was *not* stopped by the break statement
                     time_no_match += check_interval_secs
@@ -437,6 +450,7 @@ def to_cmd_str(cmd):
 
             exit_code = proc.poll()
 
+        # collect last bit of output once processed has exited
         stdout += proc.stdout.read()
         if split_stderr:
             stderr += proc.stderr.read()

From 23727d605563ce760a82ea4ba1fc9bc2b7306bad Mon Sep 17 00:00:00 2001
From: Kenneth Hoste <kenneth.hoste@ugent.be>
Date: Wed, 3 Apr 2024 18:27:38 +0200
Subject: [PATCH 16/17] don't reset time_no_match in _answer_question, do it in
 run_shell_cmd

---
 easybuild/tools/run.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py
index 24e860054b..c1d13d01d0 100644
--- a/easybuild/tools/run.py
+++ b/easybuild/tools/run.py
@@ -236,7 +236,6 @@ def _answer_question(stdout, proc, qa_patterns, qa_wait_patterns):
             except OSError as err:
                 raise EasyBuildError("Failed to answer question raised by interactive command: %s", err)
 
-            time_no_match = 0
             match_found = True
             break
     else:
@@ -250,7 +249,6 @@ def _answer_question(stdout, proc, qa_patterns, qa_wait_patterns):
             if regex.search(stdout):
                 _log.info(f"Found match for wait pattern '{pattern}'")
                 _log.debug(f"Found match for wait pattern '{pattern}' at end of stdout: {stdout[:1000]}")
-                time_no_match = 0
                 match_found = True
                 break
         else:
@@ -435,7 +433,9 @@ def to_cmd_str(cmd):
                 stderr += proc.stderr.read1(read_size) or b''
 
             if qa_patterns:
-                if not _answer_question(stdout, proc, qa_patterns, qa_wait_patterns):
+                if _answer_question(stdout, proc, qa_patterns, qa_wait_patterns):
+                    time_no_match = 0
+                else:
                     _log.debug(f"No match found in question/wait patterns at end of stdout: {stdout[:1000]}")
                     # this will only run if the for loop above was *not* stopped by the break statement
                     time_no_match += check_interval_secs

From 204f3287b10974ba875337152ff0985fd3ae0083 Mon Sep 17 00:00:00 2001
From: Kenneth Hoste <kenneth.hoste@ugent.be>
Date: Sat, 6 Apr 2024 10:19:47 +0200
Subject: [PATCH 17/17] tackle Simon's suggestions for run_shell_cmd

---
 easybuild/tools/run.py | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/easybuild/tools/run.py b/easybuild/tools/run.py
index c1d13d01d0..3253a372b1 100644
--- a/easybuild/tools/run.py
+++ b/easybuild/tools/run.py
@@ -223,7 +223,7 @@ def _answer_question(stdout, proc, qa_patterns, qa_wait_patterns):
             elif isinstance(answers, str):
                 answer = answers
             else:
-                raise EasyBuildError(f"Unknown type of answers encountered: {answers}")
+                raise EasyBuildError(f"Unknown type of answers encountered for question ({question}): {answers}")
 
             # answer may need to be completed via pattern extracted from question
             _log.debug(f"Raw answer for question pattern '{question}': {answer}")
@@ -280,8 +280,7 @@ def run_shell_cmd(cmd, fail_on_error=True, split_stderr=False, stdin=None, env=N
     :param task_id: task ID for specified shell command (included in return value)
     :param with_hooks: trigger pre/post run_shell_cmd hooks (if defined)
     :param qa_patterns: list of 2-tuples with patterns for questions + corresponding answers
-    :param qa_wait_patterns: list of 2-tuples with patterns for non-questions
-                             and number of iterations to allow these patterns to match with end out command output
+    :param qa_wait_patterns: list of strings with patterns for non-questions
     :param qa_timeout: amount of seconds to wait until more output is produced when there is no matching question
 
     :return: Named tuple with: