Add -r/--recursive options to planemo lint.

jmchilton · jmchilton · commit 01f2af9923ff · 2015-04-25T18:57:39.000-04:00
Shed lint with --tools will always work this way since I think the Tool Shed is pretty aggressive about finding tools. Mentioning #139 - though this doesn't implement the multiple paths on the command-line API change.
diff --git a/planemo/commands/cmd_lint.py b/planemo/commands/cmd_lint.py
@@ -15,11 +15,17 @@
 @options.fail_level_option()
 @options.skip_option()
 @options.lint_xsd_option()
+@options.recursive_option()
 @pass_context
 def cli(ctx, path, **kwds):
     """Check specified tool(s) for common errors and adherence to best
     practices.
     """
     lint_args = build_lint_args(ctx, **kwds)
-    exit = lint_tools_on_path(ctx, path, lint_args)
+    exit = lint_tools_on_path(
+        ctx,
+        path,
+        lint_args,
+        recursive=kwds["recursive"]
+    )
     sys.exit(exit)
diff --git a/planemo/options.py b/planemo/options.py
@@ -298,9 +298,15 @@ def fail_level_option():
 
 
 def recursive_shed_option():
+    return recursive_option(
+        "Recursively perform command for nested repository directories.",
+    )
+
+
+def recursive_option(help="Recursively perform command for subdirectories."):
     return click.option(
         '-r',
         '--recursive',
         is_flag=True,
-        help="Recursively perform command for nested repository directories.",
+        help=help,
     )
diff --git a/planemo/shed_lint.py b/planemo/shed_lint.py
@@ -62,7 +62,8 @@ def lint_repository(ctx, path, **kwds):
         path,
     )
     if kwds["tools"]:
-        for (tool_path, tool_xml) in yield_tool_xmls(ctx, path):
+        for (tool_path, tool_xml) in yield_tool_xmls(ctx, path,
+                                                     recursive=True):
             info("+Linting tool %s" % tool_path)
             lint_xml_with(
                 lint_ctx,
diff --git a/planemo/tool_lint.py b/planemo/tool_lint.py
@@ -1,24 +1,24 @@
 import os
-import sys
-import traceback
 
 from planemo.io import info
 from planemo.io import error
 
 import planemo.linters.xsd
 
-from galaxy.tools.loader_directory import load_tool_elements_from_path
+from planemo.tools import load_tool_elements_from_path
 from galaxy.tools.lint import lint_xml
 
 SKIP_XML_MESSAGE = "Skipping XML file - does not appear to be a tool %s."
 LINTING_TOOL_MESSAGE = "Linting tool %s"
 SHED_FILES = ["tool_dependencies.xml", "repository_dependencies.xml"]
 
 
-def lint_tools_on_path(ctx, path, lint_args, assert_tools=True):
+def lint_tools_on_path(ctx, path, lint_args, **kwds):
+    assert_tools = kwds.get("assert_tools", True)
+    recursive = kwds.get("recursive", False)
     exit = 0
     valid_tools = 0
-    for (tool_path, tool_xml) in yield_tool_xmls(ctx, path):
+    for (tool_path, tool_xml) in yield_tool_xmls(ctx, path, recursive):
         info("Linting tool %s" % tool_path)
         if not lint_xml(tool_xml, **lint_args):
             error("Failed linting")
@@ -30,8 +30,11 @@ def lint_tools_on_path(ctx, path, lint_args, assert_tools=True):
     return exit
 
 
-def yield_tool_xmls(ctx, path):
-    tools = load_tool_elements_from_path(path, load_exception_handler)
+def yield_tool_xmls(ctx, path, recursive=False):
+    tools = load_tool_elements_from_path(
+        path,
+        recursive,
+    )
     for (tool_path, tool_xml) in tools:
         if not _is_tool_xml(ctx, tool_path, tool_xml):
             continue
@@ -57,11 +60,6 @@ def build_lint_args(ctx, **kwds):
     return lint_args
 
 
-def load_exception_handler(path, exc_info):
-    error("Error loading tool with path %s" % path)
-    traceback.print_exception(*exc_info, limit=1, file=sys.stderr)
-
-
 def _lint_extra_modules(**kwds):
     xsd = kwds.get("xsd", False)
     if xsd:
diff --git a/planemo/tools.py b/planemo/tools.py
@@ -0,0 +1,18 @@
+import sys
+import traceback
+from planemo.io import error
+
+from galaxy.tools import loader_directory
+
+
+def load_tool_elements_from_path(path, recursive):
+    return loader_directory.load_tool_elements_from_path(
+        path,
+        load_exception_handler,
+        recursive,
+    )
+
+
+def load_exception_handler(path, exc_info):
+    error("Error loading tool with path %s" % path)
+    traceback.print_exception(*exc_info, limit=1, file=sys.stderr)
diff --git a/planemo_ext/galaxy/tools/loader_directory.py b/planemo_ext/galaxy/tools/loader_directory.py
@@ -1,3 +1,4 @@
+import fnmatch
 import glob
 import os
 from ..tools import loader
@@ -8,16 +9,21 @@
 log = logging.getLogger(__name__)
 
 PATH_DOES_NOT_EXIST_ERROR = "Could not load tools from path [%s] - this path does not exist."
+PATH_AND_RECURSIVE_ERROR = "Cannot specify a single file and recursive."
 LOAD_FAILURE_ERROR = "Failed to load tool with path %s."
 
 
 def load_exception_handler(path, exc_info):
     log.warn(LOAD_FAILURE_ERROR % path, exc_info=exc_info)
 
 
-def load_tool_elements_from_path(path, load_exception_handler=load_exception_handler):
+def load_tool_elements_from_path(
+    path,
+    load_exception_handler=load_exception_handler,
+    recursive=False,
+):
     tool_elements = []
-    for file in __find_tool_files(path):
+    for file in __find_tool_files(path, recursive=recursive):
         try:
             looks_like_a_tool = __looks_like_a_tool(file)
         except IOError:
@@ -45,10 +51,30 @@ def __looks_like_a_tool(path):
     return False
 
 
-def __find_tool_files(path):
+def __find_tool_files(path, recursive):
+    is_file = not os.path.isdir(path)
     if not os.path.exists(path):
         raise Exception(PATH_DOES_NOT_EXIST_ERROR)
-    if not os.path.isdir(path):
+    elif is_file and recursive:
+        raise Exception(PATH_AND_RECURSIVE_ERROR)
+    elif is_file:
         return [os.path.abspath(path)]
     else:
-        return map(os.path.abspath, glob.glob(path + "/**.xml"))
+        if not recursive:
+            files = glob.glob(path + "/*.xml")
+        else:
+            files = _find_files(path, "*.xml")
+        return map(os.path.abspath, files)
+
+
+def _find_files(directory, pattern='*'):
+    if not os.path.exists(directory):
+        raise ValueError("Directory not found {}".format(directory))
+
+    matches = []
+    for root, dirnames, filenames in os.walk(directory):
+        for filename in filenames:
+            full_path = os.path.join(root, filename)
+            if fnmatch.filter([full_path], pattern):
+                matches.append(os.path.join(root, filename))
+    return matches
diff --git a/tests/data/repos/multi_repos_nested/cat1/cat1.xml b/tests/data/repos/multi_repos_nested/cat1/cat1.xml
@@ -13,7 +13,7 @@
         </repeat>
     </inputs>
     <outputs>
-        <data name="out_file1" format="input" metadata_source="input1"/>
+        <data name="out_file1" format_source="input1" metadata_source="input1"/>
     </outputs>
     <tests>
       <test>
@@ -22,5 +22,6 @@
       </test>
     </tests>
     <help>
+        Copy a dataset.
     </help>
 </tool>
diff --git a/tests/data/repos/multi_repos_nested/cat2/cat2.xml b/tests/data/repos/multi_repos_nested/cat2/cat2.xml
@@ -13,7 +13,7 @@
         </repeat>
     </inputs>
     <outputs>
-        <data name="out_file1" format="input" metadata_source="input1"/>
+        <data name="out_file1" format_source="input1" metadata_source="input1"/>
     </outputs>
     <tests>
       <test>
@@ -22,5 +22,6 @@
       </test>
     </tests>
     <help>
+        Copy a dataset.
     </help>
 </tool>
diff --git a/tests/test_lint.py b/tests/test_lint.py
@@ -1,8 +1,11 @@
 import os
 import glob
 
-from .test_utils import CliTestCase
-from .test_utils import TEST_TOOLS_DIR
+from .test_utils import (
+    CliTestCase,
+    TEST_TOOLS_DIR,
+    TEST_REPOS_DIR,
+)
 
 
 class LintTestCase(CliTestCase):
@@ -30,3 +33,14 @@ def test_skips(self):
         # Check string splitting and stuff.
         lint_cmd = ["lint", "--skip", "xml_order, citations", fail_citation]
         self._check_exit_code(lint_cmd, exit_code=0)
+
+    def test_recursive(self):
+        nested_dir = os.path.join(TEST_REPOS_DIR, "multi_repos_nested")
+
+        # Fails to find any tools without -r.
+        lint_cmd = ["lint", "--skip", "citations", nested_dir]
+        self._check_exit_code(lint_cmd, exit_code=2)
+
+        # Works with -r.
+        lint_cmd = ["lint", "--skip", "citations", "-r", nested_dir]
+        self._check_exit_code(lint_cmd, exit_code=0)
