From 648ccd2e3ee2f7a2b855531f638490fe148eb83a Mon Sep 17 00:00:00 2001
From: Jonathan Karlsen <JONAK@equinor.com>
Date: Wed, 18 Sep 2024 15:05:06 +0200
Subject: [PATCH] Have ForwardModelRunner validate executable_file earlier

This commit fixes the issue where the ForwardModelRunner on the job_runner side would crash when validating the existence of a ForwardModelStep executable; causing the drivers to hang while waiting for a never coming return code from JobRunner.
---
 src/_ert/forward_model_runner/io/__init__.py  |  9 +++++----
 src/_ert/forward_model_runner/job.py          | 10 +++++-----
 .../forward_model_runner/test_job.py          | 20 +++++++++----------
 3 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/src/_ert/forward_model_runner/io/__init__.py b/src/_ert/forward_model_runner/io/__init__.py
index 6e9aa15d117..0fa6efb8814 100644
--- a/src/_ert/forward_model_runner/io/__init__.py
+++ b/src/_ert/forward_model_runner/io/__init__.py
@@ -6,7 +6,7 @@ def cond_unlink(file):
         os.unlink(file)
 
 
-def assert_file_executable(fname):
+def check_executable(fname) -> str:
     """The function raises an IOError if the given file is either not a file or
     not an executable.
 
@@ -17,7 +17,7 @@ def assert_file_executable(fname):
 
     """
     if not fname:
-        raise IOError("No executable provided!")
+        return "No executable provided!"
     fname = os.path.expanduser(fname)
 
     potential_executables = [os.path.abspath(fname)]
@@ -28,7 +28,8 @@ def assert_file_executable(fname):
         ]
 
     if not any(map(os.path.isfile, potential_executables)):
-        raise IOError(f"{fname} is not a file!")
+        return f"{fname} is not a file!"
 
     if not any(os.access(fn, os.X_OK) for fn in potential_executables):
-        raise IOError(f"{fname} is not an executable!")
+        return f"{fname} is not an executable!"
+    return ""
diff --git a/src/_ert/forward_model_runner/job.py b/src/_ert/forward_model_runner/job.py
index c57c9c9a556..fd76e4609e0 100644
--- a/src/_ert/forward_model_runner/job.py
+++ b/src/_ert/forward_model_runner/job.py
@@ -14,7 +14,7 @@
 
 from psutil import AccessDenied, NoSuchProcess, Process, TimeoutExpired, ZombieProcess
 
-from .io import assert_file_executable
+from .io import check_executable
 from .reporting.message import (
     Exited,
     MemoryStatus,
@@ -97,10 +97,7 @@ def run(self):
 
         yield start_message
 
-        executable = self.job_data.get("executable")
-        assert_file_executable(executable)
-
-        arg_list = [executable]
+        arg_list = [self.job_data.get("executable")]
         if self.job_data.get("argList"):
             arg_list += self.job_data["argList"]
 
@@ -313,6 +310,9 @@ def _check_job_files(self):
         ):
             os.unlink(self.job_data.get("error_file"))
 
+        if executable_error := check_executable(self.job_data.get("executable")):
+            errors.append(str(executable_error))
+
         return errors
 
     def _check_target_file_is_written(self, target_file_mtime: int, timeout=5):
diff --git a/tests/unit_tests/forward_model_runner/test_job.py b/tests/unit_tests/forward_model_runner/test_job.py
index d217087061c..937daa87629 100644
--- a/tests/unit_tests/forward_model_runner/test_job.py
+++ b/tests/unit_tests/forward_model_runner/test_job.py
@@ -15,14 +15,13 @@
 from _ert.forward_model_runner.reporting.message import Exited, Running, Start
 
 
-@patch("_ert.forward_model_runner.job.assert_file_executable")
+@patch("_ert.forward_model_runner.job.check_executable")
 @patch("_ert.forward_model_runner.job.Popen")
 @patch("_ert.forward_model_runner.job.Process")
 @pytest.mark.usefixtures("use_tmpdir")
-def test_run_with_process_failing(
-    mock_process, mock_popen, mock_assert_file_executable
-):
+def test_run_with_process_failing(mock_process, mock_popen, mock_check_executable):
     job = Job({}, 0)
+    mock_check_executable.return_value = ""
     type(mock_process.return_value.memory_info.return_value).rss = PropertyMock(
         return_value=10
     )
@@ -219,9 +218,9 @@ def test_run_with_defined_executable_but_missing():
         0,
     )
 
-    with pytest.raises(IOError):
-        for _ in job.run():
-            pass
+    start_message = next(job.run())
+    assert isinstance(start_message, Start)
+    assert "this/is/not/a/file is not a file" in start_message.error_message
 
 
 @pytest.mark.usefixtures("use_tmpdir")
@@ -265,10 +264,9 @@ def test_run_with_defined_executable_no_exec_bit():
         },
         0,
     )
-
-    with pytest.raises(IOError):
-        for _ in job.run():
-            pass
+    start_message = next(job.run())
+    assert isinstance(start_message, Start)
+    assert "foo is not an executable" in start_message.error_message
 
 
 def test_init_job_no_std():