Merge pull request #6605 from mvdbeek/enable_and_fix_framework_py3_test

Enable and fix framework py3 tests

.ci/jenkins/framework-py3/run_tests.sh

@@ -0,0 +1,3 @@
+#!/bin/bash
+
+./run_tests.sh --dockerize --python3 --db postgres --clean_pyc --framework_py3 "$@"

.ci/jenkins/selenium-py3/run_tests.sh

@@ -0,0 +1,34 @@
+#!/bin/bash
+
+# Enable retries on tests to reduce chances of transient failures.
+: ${GALAXY_TEST_SELENIUM_RETRIES:=1}
+: ${GALAXY_TEST_CLIENT_BUILD_IMAGE:='node:9.4.0'}
+
+# If in Jenkins environment, use it for artifacts.
+if [ -n "$BUILD_NUMBER" ];
+then
+    : ${GALAXY_TEST_ERRORS_DIRECTORY:=${BUILD_NUMBER}-test-errors}
+    : ${GALAXY_TEST_SCREENSHOTS_DIRECTORY:=${BUILD_NUMBER}-test-screenshots}
+else
+    : ${GALAXY_TEST_ERRORS_DIRECTORY:=database/test-errors}
+    : ${GALAXY_TEST_SCREENSHOTS_DIRECTORY:=database/test-screenshots}
+fi
+
+mkdir -p "$GALAXY_TEST_ERRORS_DIRECTORY"
+mkdir -p "$GALAXY_TEST_SCREENSHOTS_DIRECTORY"
+
+mkdir -p ~/.jenkins-yarn-cache
+YARN_CACHE_FOLDER=~/.jenkins-yarn-cache
+
+# Set git environment variables to enable Git. https://github.com/galaxyproject/galaxy/issues/5912
+# Setup volume and environment variable to cache this users yarn build.
+docker run -e GIT_COMMITTER_NAME=Jenkins -e GIT_COMMITTER_EMAIL=jenkins@galaxyproject.org \
+           -e YARN_CACHE_FOLDER=$YARN_CACHE_FOLDER -v $YARN_CACHE_FOLDER:$YARN_CACHE_FOLDER:rw \
+           -v `pwd`:`pwd`:rw -w `pwd` -u $UID $GALAXY_TEST_CLIENT_BUILD_IMAGE \
+           /bin/bash -c 'make client-production-maps'
+
+# Start Selenium server in the test Docker container.
+DOCKER_RUN_EXTRA_ARGS="-v $YARN_CACHE_FOLDER:$YARN_CACHE_FOLDER -e YARN_CACHE_FOLDER=$YARN_CACHE_FOLDER -e USE_SELENIUM=1 -e GALAXY_TEST_SELENIUM_RETRIES=${GALAXY_TEST_SELENIUM_RETRIES} -e GALAXY_TEST_ERRORS_DIRECTORY=${GALAXY_TEST_ERRORS_DIRECTORY} -e GALAXY_TEST_SCREENSHOTS_DIRECTORY=${GALAXY_TEST_SCREENSHOTS_DIRECTORY} ${DOCKER_RUN_EXTRA_ARGS}"
+export DOCKER_RUN_EXTRA_ARGS
+
+./run_tests.sh --dockerize --python3 --db postgres --clean_pyc --skip_flakey_fails --selenium "$@"

lib/galaxy/datatypes/data.py

@@ -798,10 +798,11 @@ class Text(Data):
         skipping all blank lines and comments.
         """
         data_lines = 0
-        for line in open(dataset.file_name):
-            line = line.strip()
-            if line and not line.startswith('#'):
-                data_lines += 1
+        with compression_utils.get_fileobj(dataset.file_name) as in_file:
+            for line in in_file:
+                line = line.strip()
+                if line and not line.startswith('#'):
+                    data_lines += 1
         return data_lines
 
     def set_peek(self, dataset, line_count=None, is_multi_byte=False, WIDTH=256, skipchars=None, line_wrap=True):

lib/galaxy/datatypes/text.py

@@ -203,9 +203,9 @@ class Biom1(Json):
         is_biom = False
         segment_size = int(load_size / 2)
         try:
-            with open(file_prefix.filename, "r") as fh:
+            with open(file_prefix.filename, "rb") as fh:
                 prev_str = ""
-                segment_str = fh.read(segment_size)
+                segment_str = fh.read(segment_size).decode('utf-8')
                 if segment_str.strip().startswith('{'):
                     while segment_str:
                         current_str = prev_str + segment_str
@@ -215,7 +215,7 @@ class Biom1(Json):
                                 is_biom = True
                                 break
                         prev_str = segment_str
-                        segment_str = fh.read(segment_size)
+                        segment_str = fh.read(segment_size).decode('utf-8')
         except Exception:
             pass
         return is_biom

lib/galaxy/jobs/splitters/multi.py

@@ -83,7 +83,7 @@ def do_split(job_wrapper):
                 raise Exception(log_error)
             input_datasets.append(input.dataset)
 
-    input_type = type_to_input_map.keys()[0]
+    input_type = next(iter(type_to_input_map.keys()))
     # DBTODO execute an external task to do the splitting, this should happen at refactor.
     # If the number of tasks is sufficiently high, we can use it to calculate job completion % and give a running status.
     try:

lib/galaxy/tools/evaluation.py

@@ -32,6 +32,7 @@ from galaxy.tools.wrappers import (
     SelectToolParameterWrapper,
     ToolParameterValueWrapper,
 )
+from galaxy.util import smart_str
 from galaxy.util.bunch import Bunch
 from galaxy.util.none_like import NoneDataset
 from galaxy.util.object_wrapper import wrap_with_safe_string
@@ -574,8 +575,8 @@ class ToolEvaluator(object):
             value = fill_template(content, context=context)
         else:
             value = content
-        with open(config_filename, "w") as f:
-            f.write(value)
+        with open(config_filename, "wb") as f:
+            f.write(smart_str(value))
         # For running jobs as the actual user, ensure the config file is globally readable
         os.chmod(config_filename, 0o644)
 

lib/galaxy/tools/execute.py

@@ -161,7 +161,7 @@ class ExecutionTracker(object):
         self.output_datasets = []
         self.output_collections = []
 
-        self.implicit_collections = {}
+        self.implicit_collections = collections.OrderedDict()
 
     @property
     def param_combinations(self):

lib/galaxy/tools/parameters/output_collect.py

@@ -327,7 +327,7 @@ def collect_dynamic_outputs(
             collection_builder = collections_service.collection_builder_for(
                 collection
             )
-            dataset_collectors = map(dataset_collector, output_collection_def.dataset_collector_descriptions)
+            dataset_collectors = [dataset_collector(description) for description in output_collection_def.dataset_collector_descriptions]
             output_name = output_collection_def.name
             filenames = job_context.find_files(output_name, collection, dataset_collectors)
             job_context.populate_collection_elements(
@@ -549,7 +549,9 @@ def collect_primary_datasets(tool, output, tool_provided_metadata, job_working_d
     new_outdata_name = None
     primary_datasets = {}
     for output_index, (name, outdata) in enumerate(output.items()):
-        dataset_collectors = map(dataset_collector, tool.outputs[name].dataset_collector_descriptions) if name in tool.outputs else [DEFAULT_DATASET_COLLECTOR]
+        dataset_collectors = [DEFAULT_DATASET_COLLECTOR]
+        if name in tool.outputs:
+            dataset_collectors = [dataset_collector(description) for description in tool.outputs[name].dataset_collector_descriptions]
         filenames = odict.odict()
         if 'new_file_path' in app.config.collect_outputs_from:
             if DEFAULT_DATASET_COLLECTOR in dataset_collectors:
@@ -672,7 +674,7 @@ DiscoveredFile = namedtuple('DiscoveredFile', ['path', 'collector', 'match'])
 
 
 def discover_files(output_name, tool_provided_metadata, extra_file_collectors, job_working_directory, matchable):
-    extra_file_collectors = list(extra_file_collectors)
+    extra_file_collectors = extra_file_collectors
     if extra_file_collectors and extra_file_collectors[0].discover_via == "tool_provided_metadata":
         # just load entries from tool provided metadata...
         assert len(extra_file_collectors) == 1

lib/galaxy/tools/wrappers.py

@@ -6,7 +6,10 @@ from six import string_types, text_type
 from six.moves import shlex_quote
 
 from galaxy import exceptions
-from galaxy.util import odict
+from galaxy.util import (
+    odict,
+    smart_str,
+)
 from galaxy.util.none_like import NoneDataset
 from galaxy.util.object_wrapper import wrap_with_safe_string
 
@@ -320,7 +323,7 @@ class HasDatasets(object):
     def paths_as_file(self, sep="\n"):
         handle, filepath = tempfile.mkstemp(prefix="gx_file_list", dir=self.job_working_directory)
         contents = sep.join(map(str, self))
-        os.write(handle, contents)
+        os.write(handle, smart_str(contents))
         os.close(handle)
         return filepath
 

lib/galaxy/util/object_wrapper.py

@@ -223,9 +223,11 @@ class SafeStringWrapper(object):
         # We need to define a __new__ since, we are subclassing from e.g. immutable str, which internally sets data
         # that will be used when other + this (this + other is handled by __add__)
         try:
-            return super(SafeStringWrapper, cls).__new__(cls, sanitize_lists_to_string(arg[0], valid_characters=VALID_CHARACTERS, character_map=CHARACTER_MAP))
-        except Exception as e:
-            log.warning("Could not provide an argument to %s.__new__: %s; will try without arguments.", cls, e)
+            sanitized_value = sanitize_lists_to_string(arg[0], valid_characters=VALID_CHARACTERS, character_map=CHARACTER_MAP)
+            return super(SafeStringWrapper, cls).__new__(cls, sanitized_value)
+        except TypeError:
+            # Class to be wrapped takes no parameters.
+            # This is pefectly normal for mutable types.
             return super(SafeStringWrapper, cls).__new__(cls)
 
     def __init__(self, value, safe_string_wrapper_function=wrap_with_safe_string):

lib/galaxy/web/framework/helpers/grids.py

@@ -144,13 +144,10 @@ class Grid(object):
                     # that we can encode to UTF-8 and thus handle user input to filters.
                     if isinstance(column_filter, list):
                         # Filter is a list; process each item.
-                        column_filter = [text_type(_).encode('utf-8') if not isinstance(_, string_types) else _ for _ in column_filter]
                         extra_url_args["f-" + column.key] = dumps(column_filter)
                     else:
                         # Process singleton filter.
-                        if not isinstance(column_filter, string_types):
-                            column_filter = text_type(column_filter)
-                        extra_url_args["f-" + column.key] = column_filter.encode("utf-8")
+                        extra_url_args["f-" + column.key] = column_filter
         # Process sort arguments.
         sort_key = None
         if 'sort' in kwargs:
@@ -672,7 +669,7 @@ class CommunityTagsColumn(TextColumn):
         if isinstance(column_filter, list):
             # Collapse list of tags into a single string; this is redundant but effective. TODO: fix this by iterating over tags.
             column_filter = ",".join(column_filter)
-        raw_tags = trans.app.tag_handler.parse_tags(column_filter.encode("utf-8"))
+        raw_tags = trans.app.tag_handler.parse_tags(column_filter)
         clause_list = []
         for name, value in raw_tags:
             if name:

run_tests.sh

@@ -429,6 +429,11 @@ do
           framework_test=1;
           shift 1
           ;;
+      -framework_py3|--framework_py3)
+          report_file="run_framework_tests.html"
+          framework_test=1;
+          shift 1
+          ;;
       -d|-data_managers|--data_managers)
           data_managers_test=1;
           shift 1

templates/display_common.mako

@@ -6,6 +6,7 @@
 ## used across webapps, and each webapp has its own model.
 
 <%! from galaxy import model %>
+<% from galaxy import unicodify %>
 
 ## Get display name for a class.
 <%def name="get_class_display_name( a_class )">
@@ -31,9 +32,7 @@
             item_name = item.name
         
         # Encode in unicode.
-        if type( item_name ) is str:
-            item_name = unicode( item_name, 'utf-8' )
-        return item_name    
+        return unicodify(item_name)
     %>
 </%def>
 

templates/legacy/grid_base.mako

@@ -1,4 +1,6 @@
 <%!
+    import six
+    from galaxy.util import unicodify
     from galaxy.webapps.reports.framework.grids import TextColumn
 
     def inherit(context):
@@ -216,8 +218,8 @@
                 value = column.get_value( trans, grid, item )
 
                 # Handle non-ascii chars.
-                if isinstance(value, str):
-                    value = unicode(value, 'utf-8')
+                if isinstance(value, six.binary_type):
+                    value = unicodify(value, 'utf-8')
                     value = value.replace('/', '//')
                 endif
 

templates/tagging_common.mako

@@ -53,7 +53,7 @@
                         tag_value = tag.user_value
 
                     ## Convert tag name, value to unicode.
-                    if isinstance( tag_name, str ):
+                    if isinstance( tag_name, six.binary_type ):
                         tag_name = unicodify( escape( tag_name ) )
                         if tag_value:
                             tag_value = unicodify( escape( tag_value ) )
@@ -201,9 +201,9 @@
                     tag_value = escape( tag.user_value )
 
                 ## Tag names and values may be string or unicode object.
-                if isinstance( tag_name, str ):
+                if isinstance( tag_name, six.binary_type ):
                     tag_names_and_values[unicodify(tag_name, 'utf-8')] = unicodify(tag_value, 'utf-8')
-                else: ## isInstance( tag_name, unicode ):
+                else:
                     tag_names_and_values[tag_name] = tag_value
         %>
         var options =

templates/webapps/tool_shed/repository/tool_form.mako

@@ -114,8 +114,7 @@
                             # Help is Mako template, so render using current static path.
                             tool_help = tool_help.render( static_path=h.url_for( '/static' ) )
                             # Convert to unicode to display non-ascii characters.
-                            if type( tool_help ) is not unicode:
-                                tool_help = unicode( tool_help, 'utf-8')
+                            tool_help = util.unicodify( tool_help, 'utf-8')
                         %>
                         ${tool_help}
                     </div>

test/base/driver_util.py

@@ -553,7 +553,7 @@ def get_ip_address(ifname):
     return socket.inet_ntoa(fcntl.ioctl(
         s.fileno(),
         0x8915,  # SIOCGIFADDR
-        struct.pack('256s', ifname[:15])
+        struct.pack('256s', ifname[:15].encode('utf-8'))
     )[20:24])
 
 
@@ -895,7 +895,7 @@ class GalaxyTestDriver(TestDriver):
             testing_installed_tools
         )
 
-    def build_tool_tests(self, testing_shed_tools=None):
+    def build_tool_tests(self, testing_shed_tools=None, return_test_classes=False):
         if self.app is None:
             return
 
@@ -909,12 +909,14 @@ class GalaxyTestDriver(TestDriver):
         import functional.test_toolbox
         functional.test_toolbox.toolbox = self.app.toolbox
         # When testing data managers, do not test toolbox.
-        functional.test_toolbox.build_tests(
+        test_classes = functional.test_toolbox.build_tests(
             app=self.app,
             testing_shed_tools=testing_shed_tools,
             master_api_key=get_master_api_key(),
             user_api_key=get_user_api_key(),
         )
+        if return_test_classes:
+            return test_classes
         return functional.test_toolbox
 
     def run_tool_test(self, tool_id, index=0, resource_parameters={}):

test/functional/test_toolbox.py

@@ -115,3 +115,4 @@ def build_tests(app=None,
             # from baseclasses (which should be a tuple of classes) and with namespace dict.
             new_class_obj = type(str(name), baseclasses, namespace)
             G[name] = new_class_obj
+    return G

test/functional/test_toolbox_pytest.py

@@ -0,0 +1,45 @@
+import os
+import sys
+
+galaxy_root = os.path.abspath(os.path.join(os.path.dirname(__file__), os.path.pardir + '/' + os.path.pardir))
+sys.path[1:1] = [os.path.join(galaxy_root, "lib"), os.path.join(galaxy_root, "test")]
+
+from base import driver_util
+
+TEST_PREFIX = 'TestForTool_'
+
+
+class DefaultGalaxyTestDriver(driver_util.GalaxyTestDriver):
+    """Default Galaxy-style nose test driver.
+
+    Just populate non-shed tool tests and run tests. Works
+    for tool tests, regular twill tests, and API testing.
+    """
+
+    def build_tests(self):
+        """Build framework tool test methods."""
+        return self.build_tool_tests(return_test_classes=True)
+
+
+def __generate_testcases():
+    driver = DefaultGalaxyTestDriver()
+    driver.setup()
+    tests = driver.build_tests()
+    for test_name, test_class in tests.items():
+        if test_name.startswith(TEST_PREFIX):
+            yield (test_name[len(TEST_PREFIX):], test_class)
+    driver.tear_down()
+
+
+def idfn(val):
+    return val[0]
+
+
+def pytest_generate_tests(metafunc):
+    if 'tool_test' in metafunc.fixturenames:
+        metafunc.parametrize("tool_test", __generate_testcases(), ids=idfn)
+
+
+def test_tool(tool_test):
+    test = tool_test[1]
+    test.do_it(test)

test/functional/tools/metadata_biom1.xml

@@ -22,7 +22,7 @@ table_columns: ${input_biom1.metadata.table_columns}</configfile>
     <tests>
       <test>
         <param name="input_biom1" value="input_taxonomy.biom1" ftype="biom1" />
-        <output name="output_of_input_metadata" ftype="txt" file="biom1_metadata_test.txt"/>
+        <output name="output_of_input_metadata" ftype="txt" file="biom1_metadata_test.txt" lines_diff="4"/>
       </test>
     </tests>
 </tool>

test/functional/tools/parallelism.xml

@@ -1,7 +1,7 @@
 <tool id="parallelism" name="Split file line-by-line and rebuild dataset">
     <parallelism method="multi" split_inputs="input1" split_mode="to_size" split_size="1" merge_outputs="out_file1" />
-    <command>
-        cat $input1 > $out_file1
+    <command detect_errors="exit_code">
+        cat '$input1' > '$out_file1'
     </command>
     <inputs>
         <param name="input1" type="data" label="Dataset"/>

test/functional/tools/parallelism_optional.xml

@@ -1,7 +1,7 @@
 <tool id="parallelism_optional" name="Split file line-by-line and rebuild dataset (with optional dataset)">
     <parallelism method="multi" split_inputs="input1" split_mode="to_size" split_size="1" merge_outputs="out_file1" />
-    <command>
-        cat $input1 > $out_file1
+    <command detect_errors="exit_code">
+        cat '$input1' > '$out_file1'
     </command>
     <inputs>
         <param name="input1" type="data" label="Dataset"/>