Merge pull request #774 from jmchilton/py3_full

jmchilton · web-flow · commit da6b48d5003a · 2018-02-27T11:31:29.000-05:00
Enable full test suite with Python 3.
diff --git a/.travis.yml b/.travis.yml
@@ -20,7 +20,6 @@ install:
 
 matrix:
   allow_failures:
-    - env: TOX_ENV=py34
     - env: TOX_ENV=py27-lint-docstrings
 
 script: PLANEMO_ENABLE_POSTGRES_TESTS=1 PLANEMO_SKIP_CWLTOOL_TESTS=1 PLANEMO_TEST_WORKFLOW_RUN_PROFILE=travisworkflowtests tox -e $TOX_ENV
diff --git a/planemo/commands/cmd_project_init.py b/planemo/commands/cmd_project_init.py
@@ -8,7 +8,6 @@
 from planemo import options
 from planemo.cli import command_function
 from planemo.io import (
-    shell,
     untar_to,
     warn,
 )
@@ -43,11 +42,7 @@ def cli(ctx, path, template=None, **kwds):
         untar_args = UNTAR_ARGS % (tempdir)
         untar_to(DOWNLOAD_URL, tempdir, untar_args)
         template_dir = os.path.join(tempdir, template)
-        shell("ls '%s'" % (template_dir))
-        shell("mv '%s'/* '%s'" % (template_dir, path))
-        dot_files = [os.path.join(template_dir, f) for f in os.listdir(template_dir) if f.startswith(".")]
-        if len(dot_files) > 0:
-            dot_files_quoted = "'" + "' '".join(dot_files) + "'"
-            shell("mv %s '%s'" % (dot_files_quoted, path))
+        for entry in os.listdir(template_dir):
+            shutil.move(os.path.join(template_dir, entry), path)
     finally:
         shutil.rmtree(tempdir)
diff --git a/planemo/commands/cmd_travis_before_install.py b/planemo/commands/cmd_travis_before_install.py
@@ -8,17 +8,8 @@
 from planemo.io import shell
 
 SETUP_FILE_NAME = "setup_custom_dependencies.bash"
-SAMTOOLS_URL = (
-    "http://archive.ubuntu.com/ubuntu/pool/universe/"
-    "s/samtools/samtools_0.1.19-1_amd64.deb"
-)
-
-FIX_EGGS_DIR = 'mkdir -p "$HOME/.python-eggs"; chmod 700 "$HOME/.python-eggs"'
-# samtools essentially required by Galaxy
-INSTALL_SAMTOOLS = (
-    "wget %s; "
-    "sudo dpkg -i samtools_0.1.19-1_amd64.deb"
-) % SAMTOOLS_URL
+SAMTOOLS_DEB = 'samtools_0.1.19-1_amd64.deb'
+SAMTOOLS_URL = "http://archive.ubuntu.com/ubuntu/pool/universe/s/samtools/%s" % SAMTOOLS_DEB
 
 BUILD_ENVIRONMENT_TEMPLATE = """
 export PATH=$PATH:${BUILD_BIN_DIR}
@@ -56,8 +47,14 @@ def cli(ctx):
     )
     open(build_env_path, "a").write(build_env)
 
-    shell(FIX_EGGS_DIR)
-    shell(INSTALL_SAMTOOLS)
+    eggs_dir = os.path.join(os.getenv('HOME'), '.python-eggs')
+    if not os.path.exists(eggs_dir):
+        os.makedirs(eggs_dir, 0o700)
+    else:
+        os.chmod(eggs_dir, 0o700)
+    # samtools essentially required by Galaxy
+    shell(['wget', SAMTOOLS_URL])
+    shell(['sudo', 'dpkg', '-i', SAMTOOLS_DEB])
     setup_file = os.path.join(build_travis_dir, SETUP_FILE_NAME)
     if os.path.exists(setup_file):
         shell(
diff --git a/planemo/commands/cmd_travis_init.py b/planemo/commands/cmd_travis_init.py
@@ -2,7 +2,6 @@
 import os
 
 import click
-from galaxy.tools.deps.commands import shell
 
 from planemo import options
 from planemo import RAW_CONTENT_URL
@@ -71,15 +70,17 @@ def cli(ctx, path):
     # TODO: Option --verbose_travis_yaml to unroll travis_test.sh line by line
     # and place all but last in 'install' section and last in 'script'. Would
     # require a yaml dependency though.
-    shell("mkdir -p '%s/.travis'" % path)
+    dot_travis_dir = os.path.join(path, '.travis')
+    if not os.path.exists(dot_travis_dir):
+        os.makedirs(dot_travis_dir)
     travis_yml = os.path.join(path, ".travis.yml")
-    setup_sh = os.path.join(path, ".travis", "setup_custom_dependencies.bash")
+    setup_sh = os.path.join(dot_travis_dir, "setup_custom_dependencies.bash")
     if not os.path.exists(travis_yml):
         open(travis_yml, "w").write(TRAVIS_YML)
     else:
         warn(".travis.yml file already exists, not overwriting.")
     if not os.path.exists(setup_sh):
         open(setup_sh, "w").write(TRAVIS_SETUP)
     else:
-        warn(".travis/setup_custom_dependencies.bash already exists, not overwriting.")
+        warn("%s already exists, not overwriting." % setup_sh)
     info(PREPARE_MESSAGE)
diff --git a/planemo/cwl/run.py b/planemo/cwl/run.py
@@ -61,8 +61,8 @@ def run_cwltool(ctx, path, job_path, **kwds):
 
     args.extend([path, job_path])
     ctx.vlog("Calling cwltool with arguments %s" % args)
-    with tempfile.NamedTemporaryFile() as tmp_stdout, \
-            tempfile.NamedTemporaryFile() as tmp_stderr:
+    with tempfile.NamedTemporaryFile("w") as tmp_stdout, \
+            tempfile.NamedTemporaryFile("w") as tmp_stderr:
         # cwltool passes sys.stderr to subprocess.Popen - ensure it has
         # and actual fileno.
         with real_io():
diff --git a/planemo/galaxy/config.py b/planemo/galaxy/config.py
@@ -13,6 +13,7 @@
 import click
 from galaxy.tools.deps import docker_util
 from galaxy.tools.deps.commands import argv_to_str
+from galaxy.util import unicodify
 from six import (
     add_metaclass,
     iteritems
@@ -571,11 +572,11 @@ def _user_email(kwds):
 @contextlib.contextmanager
 def _config_directory(ctx, **kwds):
     config_directory = kwds.get("config_directory", None)
-    ctx.vlog("Created directory for Galaxy configuration [%s]" % config_directory)
     created_config_directory = False
     if not config_directory:
         created_config_directory = True
         config_directory = os.path.realpath(mkdtemp())
+        ctx.vlog("Created directory for Galaxy configuration [%s]" % config_directory)
     try:
         yield config_directory
     finally:
@@ -967,7 +968,8 @@ def _download_database_template(
         return True
 
     if latest or not galaxy_root:
-        template_url = DOWNLOADS_URL + urlopen(LATEST_URL).read()
+        symlink_target = unicodify(urlopen(LATEST_URL).read())
+        template_url = DOWNLOADS_URL + symlink_target
         urlretrieve(template_url, database_location)
         return True
 
diff --git a/planemo/io.py b/planemo/io.py
@@ -28,8 +28,10 @@
 
 def communicate(cmds, **kwds):
     if isinstance(cmds, list):
-        cmds = commands.argv_to_str(cmds)
-    info(cmds)
+        cmd_string = commands.argv_to_str(cmds)
+    else:
+        cmd_string = cmds
+    info(cmd_string)
     p = commands.shell_process(cmds, **kwds)
     if kwds.get("stdout", None) is None and commands.redirecting_io(sys=sys):
         output = commands.redirect_aware_commmunicate(p)
@@ -38,13 +40,17 @@ def communicate(cmds, **kwds):
 
     if p.returncode != 0:
         template = "Problem executing commands {0} - ({1}, {2})"
-        msg = template.format(cmds, output[0], output[1])
+        msg = template.format(cmd_string, output[0], output[1])
         raise RuntimeError(msg)
     return output
 
 
 def shell(cmds, **kwds):
-    info(cmds)
+    if isinstance(cmds, list):
+        cmd_string = commands.argv_to_str(cmds)
+    else:
+        cmd_string = cmds
+    info(cmd_string)
     return commands.shell(cmds, **kwds)
 
 
diff --git a/planemo/linters/biocontainer_registered.py b/planemo/linters/biocontainer_registered.py
@@ -20,7 +20,7 @@ def lint_biocontainer_registered(tool_source, lint_ctx):
         lint_ctx.warn(MESSAGE_WARN_NO_REQUIREMENTS)
         return
 
-    mulled_targets = map(lambda c: build_target(c.package, c.version), conda_targets)
+    mulled_targets = [build_target(c.package, c.version) for c in conda_targets]
     name = mulled_container_name("biocontainers", mulled_targets)
     if name:
         lint_ctx.info(MESSAGE_INFO_FOUND_BIOCONTAINER % name)
diff --git a/planemo/mulled.py b/planemo/mulled.py
@@ -18,11 +18,11 @@
 
 
 def conda_to_mulled_targets(conda_targets):
-    return map(lambda c: build_target(c.package, c.version), conda_targets)
+    return list(map(lambda c: build_target(c.package, c.version), conda_targets))
 
 
 def collect_mulled_target_lists(ctx, paths, recursive=False):
-    return map(conda_to_mulled_targets, collect_conda_target_lists(ctx, paths, recursive=recursive))
+    return list(map(conda_to_mulled_targets, collect_conda_target_lists(ctx, paths, recursive=recursive)))
 
 
 def build_involucro_context(ctx, **kwds):
diff --git a/planemo/runnable.py b/planemo/runnable.py
@@ -93,7 +93,7 @@ def for_path(path):
 
 def for_paths(paths):
     """Return a specialized list of Runnable objects for paths."""
-    return map(for_path, paths)
+    return list(map(for_path, paths))
 
 
 def cases(runnable):
diff --git a/planemo/shed/__init__.py b/planemo/shed/__init__.py
@@ -287,7 +287,7 @@ def upload_repository(ctx, realized_repository, **kwds):
         tar_path = build_tarball(path, **kwds)
     if kwds.get("tar_only", False):
         name = realized_repository.pattern_to_file_name("shed_upload.tar.gz")
-        shell("cp '%s' '%s'" % (tar_path, name))
+        shutil.copy(tar_path, name)
         return 0
     shed_context = get_shed_context(ctx, **kwds)
     update_kwds = {}
@@ -393,8 +393,9 @@ def _diff_in(ctx, working, realized_repository, **kwds):
         )
     else:
         tar_path = build_tarball(path)
-        cmd_template = 'mkdir "%s"; tar -xzf "%s" -C "%s"; rm -rf %s'
-        shell(cmd_template % (mine, tar_path, mine, tar_path))
+        os.mkdir(mine)
+        shell(['tar', '-xzf', tar_path, '-C', mine])
+        shutil.rmtree(tar_path, ignore_errors=True)
 
     output = kwds.get("output", None)
     raw = kwds.get("raw", False)
diff --git a/planemo/shed2tap/base.py b/planemo/shed2tap/base.py
@@ -8,6 +8,7 @@
 from ftplib import all_errors as FTPErrors  # tuple of exceptions
 from xml.etree import ElementTree
 
+from galaxy.util import unicodify
 from six import iteritems
 from six import string_types
 from six.moves import map as imap
@@ -426,6 +427,7 @@ def _cache_download(url, filename, sha256sum=None):
             # TODO - log this nicely...
             sys.stderr.write("Verifying checksum for %s\n" % filename)
             filehash = subprocess.check_output(['shasum', '-a', '256', local])[0:64].strip()
+            filehash = unicodify(filehash)
             if filehash != sha256sum:
                 raise RuntimeError("Checksum failure for %s, got %r but wanted %r" % (local, filehash, sha256sum))
 
diff --git a/planemo/tool_builder.py b/planemo/tool_builder.py
@@ -7,6 +7,7 @@
 import os
 import re
 import shlex
+import shutil
 import subprocess
 from collections import namedtuple
 
@@ -847,10 +848,13 @@ def write_tool_description(ctx, tool_description, **kwds):
     if tool_description.test_files:
         if not os.path.exists("test-data"):
             io.info("No test-data directory, creating one.")
-            io.shell("mkdir -p 'test-data'")
+            os.makedirs('test-data')
         for test_file in tool_description.test_files:
             io.info("Copying test-file %s" % test_file)
-            io.shell("cp '%s' 'test-data'" % test_file)
+            try:
+                shutil.copy(test_file, 'test-data')
+            except Exception as e:
+                io.info("Copy of %s failed: %s" % (test_file, e))
 
 
 __all__ = (
diff --git a/requirements.txt b/requirements.txt
@@ -13,6 +13,6 @@ virtualenv
 lxml
 gxformat2>=0.2.0
 ephemeris>=0.8
-galaxy-lib>=17.9.12
+galaxy-lib>=18.5.1
 html5lib>=0.9999999,!=0.99999999,!=0.999999999,!=1.0b10,!=1.0b09
 cwltool==1.0.20170828135420
diff --git a/tests/test_cmd_test.py b/tests/test_cmd_test.py
@@ -115,7 +115,7 @@ def append_profile_argument_if_needed(self, command):
         # while running tests.
         profile_name = os.getenv("PLANEMO_TEST_WORKFLOW_RUN_PROFILE", None)
 
-        if not profile_name:
+        if profile_name:
             command += ["--profile", profile_name]
 
             database_type = os.getenv("PLANEMO_TEST_WORKFLOW_RUN_PROFILE_DATABASE_TYPE", None)
diff --git a/tests/test_run.py b/tests/test_run.py
@@ -50,7 +50,6 @@ def test_run_cat_cwltool_more_options(self):
             ]
             self._check_exit_code(test_cmd)
 
-    @skip_unless_python_2_7()
     @skip_if_environ("PLANEMO_SKIP_GALAXY_TESTS")
     def test_run_gxtool_randomlines(self):
         with self._isolate():
diff --git a/tests/test_utils.py b/tests/test_utils.py
@@ -108,7 +108,7 @@ def _isolate_repo(self, name):
 
     def _copy_repo(self, name, dest):
         repo = os.path.join(TEST_REPOS_DIR, name)
-        io.shell("cp -r '%s/.' '%s'" % (repo, dest))
+        io.shell(['cp', '-r', "%s/." % repo, dest])
 
     @property
     def test_context(self):
