Error when frameworks installation script fails (#685)

* Error when frameworks installation script fails

* Fix loading of frameworks for tests

* Do not force usage of -e

Author: PGijsbers

amlb/benchmark.py

@@ -193,12 +193,17 @@ def setup(self, mode: SetupMode):
         self._mark_setup_start()
 
         if hasattr(self.framework_module, "setup"):
-            self.framework_module.setup(
-                *self.framework_def.setup_args,
-                _shell_=False,  # prevents #arg from being interpreted as comment
-                _live_output_=rconfig().setup.live_output,
-                _activity_timeout_=rconfig().setup.activity_timeout,
-            )
+            try:
+                self.framework_module.setup(
+                    *self.framework_def.setup_args,
+                    _shell_=False,  # prevents #arg from being interpreted as comment
+                    _live_output_=rconfig().setup.live_output,
+                    _activity_timeout_=rconfig().setup.activity_timeout,
+                )
+            except Exception as e:
+                raise JobError(
+                    f"Setup of framework {self.framework_name} failed."
+                ) from e
 
         if self.framework_def.setup_script is not None:
             run_script(

tests/conftest.py

@@ -12,6 +12,14 @@ def load_default_resources(tmp_path):
         os.path.join(default_dirs.root_dir, "resources", "config.yaml")
     )
     config_default_dirs = default_dirs
+    config_test = Namespace(
+        frameworks=Namespace(
+            definition_file=[
+                "{root}/resources/frameworks.yaml",
+                "{root}/tests/resources/frameworks.yaml",
+            ]
+        )
+    )
     # allowing config override from user_dir: useful to define custom benchmarks and frameworks for example.
     config_user = Namespace()
     # config listing properties set by command line
@@ -30,7 +38,7 @@ def load_default_resources(tmp_path):
     config_args = Namespace({k: v for k, v in config_args if v is not None})
     # merging all configuration files and saving to the global variable
     resources.from_configs(
-        config_default, config_default_dirs, config_user, config_args
+        config_default, config_test, config_default_dirs, config_user, config_args
     )
 
 

tests/resources/frameworks.yaml

@@ -0,0 +1,3 @@
+setup_fail:
+  description: "used for tests"
+  module: tests.resources.frameworks.setup_fail

tests/resources/frameworks/setup_fail/__init__.py

@@ -0,0 +1,5 @@
+from amlb.utils import call_script_in_same_dir
+
+
+def setup(*args, **kwargs):
+    call_script_in_same_dir(__file__, "setup.sh", *args, **kwargs)

tests/resources/frameworks/setup_fail/setup.sh

@@ -0,0 +1,6 @@
+#!/bin/bash
+set -e
+
+command_that_fails
+
+echo "Command that succeeds"

tests/unit/amlb/benchmarks/test_benchmark.py

@@ -1,8 +1,10 @@
 from pathlib import Path
+from subprocess import SubprocessError
 
 import pytest
 
 from amlb import Benchmark, SetupMode, resources, DockerBenchmark, SingularityBenchmark
+from amlb.job import JobError
 from amlb.utils import Namespace
 
 
@@ -112,3 +114,20 @@ def test_singularity_image_name_as_docker(
         as_docker_image=True,
     )
     assert result == expected
+
+
+def test_benchmark_setup_errors_if_framework_does_not_install(
+    load_default_resources,
+) -> None:
+    benchmark = Benchmark(
+        framework_name="setup_fail",
+        benchmark_name="test",
+        constraint_name="test",
+        job_history=None,
+    )
+
+    with pytest.raises(JobError) as exc_info:
+        benchmark.setup(SetupMode.force)
+    assert "setup" in str(exc_info.value)
+    assert isinstance(exc_info.value.__cause__, SubprocessError)
+    assert "command_that_fails" in exc_info.value.__cause__.stderr