Merge pull request #13026 from nsoranzo/misc_fixes

Misc fixes - round 4

Makefile

@@ -141,7 +141,7 @@ $(CWL_TARGETS):
 
 generate-cwl-conformance-tests: $(CWL_TARGETS)  ## Initialise CWL conformance tests
 
-clean-cwl-conformance-tests:  # Clean CWL conformance tests
+clean-cwl-conformance-tests:  ## Clean CWL conformance tests
 	for f in $(CWL_TARGETS); do \
 		if [ $$(basename "$$f") = conformance_tests.yaml ]; then \
 			rm -rf $$(dirname "$$f"); \

lib/galaxy/app_unittest_utils/tools_support.py

@@ -94,15 +94,14 @@ class UsesTools(UsesApp):
                 self.__write_tool(extra_file_contents, path=os.path.join(self.test_directory, extra_file_path))
         else:
             self.tool_file = tool_path
-        self._init_app_for_tools()
         return self.__setup_tool()
 
     def _init_tool_for_path(self, tool_file):
-        self._init_app_for_tools()
         self.tool_file = tool_file
         return self.__setup_tool()
 
-    def _init_app_for_tools(self):
+    def setup_app(self):
+        super().setup_app()
         self.app.config.drmaa_external_runjob_script = ""
         self.app.config.tool_secret = "testsecret"
         self.app.config.track_jobs_in_database = False
@@ -110,12 +109,9 @@ class UsesTools(UsesApp):
 
     def __setup_tool(self):
         tool_source = get_tool_source(self.tool_file)
-        try:
-            self.tool = create_tool_from_source(self.app, tool_source, config_file=self.tool_file)
-            self.tool.assert_finalized()
-        except Exception:
-            self.tool = None
-        if getattr(self, "tool_action", None) and self.tool:
+        self.tool = create_tool_from_source(self.app, tool_source, config_file=self.tool_file)
+        self.tool.assert_finalized()
+        if getattr(self, "tool_action", None):
             self.tool.tool_action = self.tool_action
         return self.tool
 

lib/galaxy/jobs/runners/util/cli/__init__.py

@@ -75,19 +75,20 @@ class CliInterface:
         shell_plugin = shell_params.get('plugin', DEFAULT_SHELL_PLUGIN)
         requested_shell_settings = json.dumps(shell_params, sort_keys=True)
         if requested_shell_settings not in self.active_cli_shells:
-            self.active_cli_shells[requested_shell_settings] = self.cli_shells[shell_plugin](**shell_params)
+            shell_plugin_class = self.cli_shells.get(shell_plugin)
+            if not shell_plugin_class:
+                raise ValueError(f"Unknown shell_plugin [{shell_plugin}], available plugins are {list(self.cli_shells.keys())}")
+            self.active_cli_shells[requested_shell_settings] = shell_plugin_class(**shell_params)
         return self.active_cli_shells[requested_shell_settings]
 
     def get_job_interface(self, job_params):
-        job_plugin = job_params.get('plugin', None)
+        job_plugin = job_params.get('plugin')
         if not job_plugin:
             raise ValueError(ERROR_MESSAGE_NO_JOB_PLUGIN)
-        job_plugin_class = self.cli_job_interfaces.get(job_plugin, None)
+        job_plugin_class = self.cli_job_interfaces.get(job_plugin)
         if not job_plugin_class:
             raise ValueError(ERROR_MESSAGE_NO_SUCH_JOB_PLUGIN % (job_plugin, list(self.cli_job_interfaces.keys())))
-        job_interface = job_plugin_class(**job_params)
-
-        return job_interface
+        return job_plugin_class(**job_params)
 
 
 def split_params(params):

lib/galaxy/jobs/runners/util/cli/job/__init__.py

@@ -22,11 +22,11 @@ except ImportError:
 
 class BaseJobExec(metaclass=ABCMeta):
 
-    @abstractmethod
     def __init__(self, **params):
         """
         Constructor for CLI job executor.
         """
+        self.params = params.copy()
 
     def job_script_kwargs(self, ofile, efile, job_name):
         """ Return extra keyword argument for consumption by job script

lib/galaxy/jobs/runners/util/cli/job/lsf.py

@@ -19,11 +19,6 @@ argmap = {
 
 class LSF(BaseJobExec):
 
-    def __init__(self, **params):
-        self.params = {}
-        for k, v in params.items():
-            self.params[k] = v
-
     def job_script_kwargs(self, ofile, efile, job_name):
         scriptargs = {'-o': ofile,
                       '-e': efile,

lib/galaxy/jobs/runners/util/cli/job/slurm.py

@@ -15,11 +15,6 @@ argmap = {
 
 class Slurm(BaseJobExec):
 
-    def __init__(self, **params):
-        self.params = {}
-        for k, v in params.items():
-            self.params[k] = v
-
     def job_script_kwargs(self, ofile, efile, job_name):
         scriptargs = {'-o': ofile,
                       '-e': efile,

lib/galaxy/jobs/runners/util/cli/job/torque.py

@@ -31,11 +31,6 @@ argmap = {'destination': '-q',
 
 class Torque(BaseJobExec):
 
-    def __init__(self, **params):
-        self.params = {}
-        for k, v in params.items():
-            self.params[k] = v
-
     def job_script_kwargs(self, ofile, efile, job_name):
         pbsargs = {'-o': ofile,
                    '-e': efile,

test/unit/app/tools/test_toolbox.py

@@ -281,7 +281,9 @@ class ToolBoxTestCase(BaseToolBoxTestCase):
 
     def test_enforce_tool_profile(self):
         self._init_tool(filename="old_tool.xml", version="1.0", profile="17.01", tool_id="test_old_tool_profile")
-        self._init_tool(filename="new_tool.xml", version="2.0", profile="27.01", tool_id="test_new_tool_profile")
+        with self.assertRaisesRegex(Exception, r"The tool \[test_new_tool_profile\] targets version 37\.01 of Galaxy"):
+            # This will write the file but fail to load the tool
+            self._init_tool(filename="new_tool.xml", version="2.0", profile="37.01", tool_id="test_new_tool_profile")
         self._add_config("""<toolbox><tool file="old_tool.xml"/><tool file="new_tool.xml"/></toolbox>""")
         toolbox = self.toolbox
         assert toolbox.get_tool("test_old_tool_profile") is not None