From 0fe2b601cec6b5c08209a4cc175e8a4b36406b81 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 14 Apr 2015 14:27:26 -0400 Subject: [PATCH] Implement detect_errors attribute on XML. If present, it can be one of - "default", no-op fallback to stdio tags and erroring on standard error output. - "exit_code", error if tool exit code is not 0. (The @jmchilton recommendation). - "aggressive", error if tool exit code is not 0 or either Exception: or Error: appears in standard error/output. (The @bgruening recommendation). Refactoring and unit/functional tests to support and demonstrate this. Run functional test with: ./run_tests.sh -framework -id detect_errors_aggressive Run relevant unit tests: nosetests test/unit/tools/test_parsing.py Updated from original version to reflect comments on pull request #117 - in particular the ``detect_errors`` tag was moved from ``tool`` to ``command``. --- lib/galaxy/tools/parser/util.py | 38 ++++++++++++++ lib/galaxy/tools/parser/xml.py | 20 +++++++- lib/galaxy/tools/parser/yaml.py | 15 +----- .../tools/detect_errors_aggressive.xml | 51 +++++++++++++++++++ test/functional/tools/samples_tool_conf.xml | 1 + test/unit/tools/test_parsing.py | 32 +++++++++++- 6 files changed, 140 insertions(+), 17 deletions(-) create mode 100644 lib/galaxy/tools/parser/util.py create mode 100644 test/functional/tools/detect_errors_aggressive.xml diff --git a/lib/galaxy/tools/parser/util.py b/lib/galaxy/tools/parser/util.py new file mode 100644 index 00000000000..03a967e558d --- /dev/null +++ b/lib/galaxy/tools/parser/util.py @@ -0,0 +1,38 @@ +from .interface import ToolStdioExitCode +from .interface import ToolStdioRegex + + +def error_on_exit_code(): + exit_code_lower = ToolStdioExitCode() + exit_code_lower.range_start = float("-inf") + exit_code_lower.range_end = -1 + _set_fatal(exit_code_lower) + exit_code_high = ToolStdioExitCode() + exit_code_high.range_start = 1 + exit_code_high.range_end = float("inf") + _set_fatal(exit_code_high) + return [exit_code_lower, exit_code_high], [] + + +def aggressive_error_checks(): + exit_codes, _ = error_on_exit_code() + # these regexes are processed as case insensitive by default + regexes = [ + _error_regex("exception:"), + _error_regex("error:") + ] + return exit_codes, regexes + + +def _error_regex(match): + regex = ToolStdioRegex() + _set_fatal(regex) + regex.match = match + regex.stdout_match = True + regex.stderr_match = True + return regex + + +def _set_fatal(obj): + from galaxy.jobs.error_level import StdioErrorLevel + obj.error_level = StdioErrorLevel.FATAL diff --git a/lib/galaxy/tools/parser/xml.py b/lib/galaxy/tools/parser/xml.py index d28b0076e3f..2afe09f80db 100644 --- a/lib/galaxy/tools/parser/xml.py +++ b/lib/galaxy/tools/parser/xml.py @@ -15,6 +15,10 @@ from .interface import ( TestCollectionDef, TestCollectionOutputDef, ) +from .util import ( + error_on_exit_code, + aggressive_error_checks, +) from galaxy.util import string_as_bool, xml_text, xml_to_string from galaxy.util.odict import odict from galaxy.tools.deps import requirements @@ -229,8 +233,20 @@ class XmlToolSource(ToolSource): return output def parse_stdio(self): - parser = StdioParser(self.root) - return parser.stdio_exit_codes, parser.stdio_regexes + command_el = self._command_el + detect_errors = None + if command_el is not None: + detect_errors = command_el.get("detect_errors") + if detect_errors and detect_errors != "default": + if detect_errors == "exit_code": + return error_on_exit_code() + elif detect_errors == "aggressive": + return aggressive_error_checks() + else: + raise ValueError("Unknown detect_errors value encountered [%s]" % detect_errors) + else: + parser = StdioParser(self.root) + return parser.stdio_exit_codes, parser.stdio_regexes def parse_help(self): help_elem = self.root.find( 'help' ) diff --git a/lib/galaxy/tools/parser/yaml.py b/lib/galaxy/tools/parser/yaml.py index 2980281452a..6ab094d51f3 100644 --- a/lib/galaxy/tools/parser/yaml.py +++ b/lib/galaxy/tools/parser/yaml.py @@ -2,7 +2,7 @@ from .interface import ToolSource from .interface import PagesSource from .interface import PageSource from .interface import InputSource -from .interface import ToolStdioExitCode +from .util import error_on_exit_code from galaxy.tools.deps import requirements from galaxy.tools.parameters import output_collect @@ -57,18 +57,7 @@ class YamlToolSource(ToolSource): return PagesSource([page_source]) def parse_stdio(self): - from galaxy.jobs.error_level import StdioErrorLevel - - # New format - starting out just using exit code. - exit_code_lower = ToolStdioExitCode() - exit_code_lower.range_start = float("-inf") - exit_code_lower.range_end = -1 - exit_code_lower.error_level = StdioErrorLevel.FATAL - exit_code_high = ToolStdioExitCode() - exit_code_high.range_start = 1 - exit_code_high.range_end = float("inf") - exit_code_lower.error_level = StdioErrorLevel.FATAL - return [exit_code_lower, exit_code_high], [] + return error_on_exit_code() def parse_help(self): return self.root_dict.get("help", None) diff --git a/test/functional/tools/detect_errors_aggressive.xml b/test/functional/tools/detect_errors_aggressive.xml new file mode 100644 index 00000000000..3ece4c2705d --- /dev/null +++ b/test/functional/tools/detect_errors_aggressive.xml @@ -0,0 +1,51 @@ + + + #if $error_bool + echo "ERROR: Problem...." + #elif $exception_bool + echo "Exception: Problem..." + #else + echo "Everything is OK." + #end if + ; sh -c "exit $exit_code" + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/functional/tools/samples_tool_conf.xml b/test/functional/tools/samples_tool_conf.xml index 2c3961dd21e..40780b3d9aa 100644 --- a/test/functional/tools/samples_tool_conf.xml +++ b/test/functional/tools/samples_tool_conf.xml @@ -16,6 +16,7 @@ + diff --git a/test/unit/tools/test_parsing.py b/test/unit/tools/test_parsing.py index cf7bbeb930e..018e99993c4 100644 --- a/test/unit/tools/test_parsing.py +++ b/test/unit/tools/test_parsing.py @@ -104,8 +104,15 @@ class BaseLoaderTestCase(unittest.TestCase): @property def _tool_source(self): - path = os.path.join(self.temp_directory, self.source_file_name) - open(path, "w").write(self.source_contents) + return self._get_tool_source() + + def _get_tool_source(self, source_file_name=None, source_contents=None): + if source_file_name is None: + source_file_name = self.source_file_name + if source_contents is None: + source_contents = self.source_contents + path = os.path.join(self.temp_directory, source_file_name) + open(path, "w").write(source_contents) tool_source = get_tool_source(path) return tool_source @@ -212,6 +219,27 @@ class XmlLoaderTestCase(BaseLoaderTestCase): assert attributes1["compare"] == "sim_size" assert attributes1["lines_diff"] == 4 + def test_exit_code(self): + tool_source = self._get_tool_source(source_contents=""" + + ls + + + """) + exit, regexes = tool_source.parse_stdio() + assert len(exit) == 2, exit + assert len(regexes) == 0, regexes + + tool_source = self._get_tool_source(source_contents=""" + + ls + + + """) + exit, regexes = tool_source.parse_stdio() + assert len(exit) == 2, exit + assert len(regexes) == 2, regexes + class YamlLoaderTestCase(BaseLoaderTestCase): source_file_name = "bwa.yml"