Fix error messages, add tests, miscellaneous fix

A few error messages have been altered to be a bit clearer, and a
bug was fixed that stopped an error from being fired.

Tests were also added to test some of the new functionality of
dynamic_tool_destination. The added tests:

 - test for an error being thrown when a priority in priority_list
   does not appear in a tool's default_destination section (this
   may not be final behaviour)

 - test to ensure that errors are fired if a destination is
   referenced that is not in the destinations section of the
   job_conf.xml

lib/galaxy/jobs/dynamic_tool_destination.py

@@ -751,7 +751,7 @@ def validate_config(obj, return_bool=False):
     global destination_list
 
     priority_list = set()
-    destination_list = get_destination_list_from_config()
+    destination_list = get_destination_list_from_job_config()
 
     def infinite_defaultdict():
         return collections.defaultdict(infinite_defaultdict)
@@ -917,8 +917,8 @@ def infinite_defaultdict():
                                     ### May not be necessary check if something has all priorities specified as default
                                     for priority in priority_list:
                                         if priority not in curr['default_destination']['priority']:
-                                            error = ("No default for destination for priority  " +
-                                                     str(priority) + " in tool " + str(tool))
+                                            error = ("no default for destination for priority '" +
+                                                     str(priority) + "' in '" + str(tool)) + "'."
                                             if verbose:
                                                 log.debug(error)
                                             valid_config = False
@@ -936,6 +936,9 @@ def infinite_defaultdict():
                                                              str(tool) + "': '" +
                                                              str(destination) +
                                                              "' does not exist.")
+                                                    if verbose:
+                                                        log.debug(error)
+                                                    valid_config = False
                                             else:
                                                 error = ("No default '" + str(priority) +
                                                          "' priority destination  for tool " +
@@ -1309,8 +1312,10 @@ def map_tool_to_destination(
         priority = default_priority  ###FIXME: this is sometimes a value that isn't in the tool's default destinations field.
     else:
         fail_message = ("No priorities declared in config file!" +
-                        "cannot map " + str(tool.old_id) + " to destination")
-        error = "No priorities found so no default priorty set!"
+                        "cannot map " + str(tool.old_id) + " to destination" +
+                        "using priorities")
+        error = ("No priorities found so no default priorty set! " +
+                 "Things may behave unexpectedly.")
         if verbose:
             log.debug(error)
         valid_config = False
@@ -1441,7 +1446,7 @@ def map_tool_to_destination(
                 else:
                     if priority in matched_rule["destination"]["priority"]:
                         destination = matched_rule["destination"]["priority"][priority]
-                    elif default_priority in matched_rule["destination"]["priority"]:  ###TODO: There's likely a better way to do this, but we'd need to get a tool's priorities before setting default_priority
+                    elif default_priority in matched_rule["destination"]["priority"]:
                         destination = (matched_rule["destination"]["priority"][default_priority])
                     # else global default destination is used
 
@@ -1474,12 +1479,12 @@ def map_tool_to_destination(
     return destination
 
 
-def get_destination_list_from_config(config_location='/config/job_conf.xml'):
+def get_destination_list_from_job_config(job_config_location='/config/job_conf.xml'):
     """
     returns A list of all destination IDs declared in the job configuration
 
-    @type config_location: str
-    @param config_location: The location of the job config file relative
+    @type job_config_location: str
+    @param job_config_location: The location of the job config file relative
                 to the galaxy root directory. Defaults to
                 galaxy/config/job_conf.xml
 
@@ -1494,7 +1499,7 @@ def get_destination_list_from_config(config_location='/config/job_conf.xml'):
     config_directory = os.path.join(
         os.path.dirname(os.path.realpath(__file__)), '../../..')
 
-    job_conf_file = config_directory + config_location
+    job_conf_file = config_directory + job_config_location
     job_conf = ET.parse(job_conf_file)
 
     for destination in job_conf.getroot().iter("destination"):

test/unit/jobs/dynamic_tool_destination/test_dynamic_tool_destination.py

@@ -103,7 +103,7 @@ def test_brokenDestYML(self, l):
             ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', 'Finished config validation.'),
             ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', 'Loading file: input1' + script_dir + '/data/test3.full'),
             ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', 'Total size: 3.23 KB'),
-            ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', 'No priorities found so no default priorty set!')
+            ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', 'No priorities found so no default priorty set! Things may behave unexpectedly.')
         )
 
     @log_capture()
@@ -628,7 +628,7 @@ def test_return_rule_for_worse_num_input_datasets_bound(self, l):
             ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', "Error: lower_bound is set to Infinity, but must be lower than upper_bound! Setting lower_bound to 0!"),
             ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', 'Finished config validation.')
         )
-
+    """
     @log_capture()
     def test_priority_default_destination_without_med_priority_destination(self, l):
         dt.parse_yaml(path=yt.ivYMLTest144, test=True)
@@ -644,9 +644,10 @@ def test_priority_default_destination_with_invalid_priority_destination(self, l)
             ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', 'Running config validation...'),
             ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', 'Finished config validation.')
         )
+      """
 
     @log_capture()
-    def test_tool_without_med_priority_destination(self, l):
+    def test_tool_without_low_default_destination(self, l):
         dt.parse_yaml(path=yt.ivYMLTest146, test=True)
         l.check(
             ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', 'Running config validation...'),
@@ -663,12 +664,42 @@ def test_tool_with_invalid_priority_destination(self, l):
             ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', 'Finished config validation.')
         )
 
+      #### New Tests
+
+    @log_capture()
+    def test_not_all_priorities_in_tool(self, l):
+        dt.parse_yaml(path=yt.ivYMLTest149, test=True)
+        l.check(
+            ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', 'Running config validation...'),
+            ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', "no default for destination for priority 'lowish' in 'yuck'."),
+            ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', "no default for destination for priority 'higher' in 'yuck'."),
+            ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', 'Finished config validation.')
+        )
+
+    @log_capture()
+    def test_rule_destination_not_in_job_conf(self, l):  ### Also test when desination is just a string, not a priority?
+        dt.parse_yaml(path=yt.ivYMLTest150, test=True)
+        l.check(
+            ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', 'Running config validation...'),
+            ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', "destination for 'blegh', rule 1: 'fake_destination' does not exist in job configuration. Ignoring..."),
+            ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', 'Finished config validation.')
+        )
+
+    @log_capture()
+    def test_tool_default_destination_not_in_job_conf(self, l):
+        dt.parse_yaml(path=yt.ivYMLTest151, test=True)
+        l.check(
+            ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', 'Running config validation...'),
+            ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', "default destination for 'blah': 'not_true_destination' does not exist."),
+            ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', 'Finished config validation.')
+        )
+
     @log_capture()
-    def test_users_with_invalid_priority(self, l):
-        dt.parse_yaml(path=yt.ivYMLTest148, test=True)
+    def test_default_destination_not_in_job_conf(self, l):
+        dt.parse_yaml(path=yt.ivYMLTest152, test=True)
         l.check(
             ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', 'Running config validation...'),
-            ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', "User 'user@email.com', priority 'mine' is not defined in the global default_destination section"),
+            ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', "destination 'no_such_dest' does not appear in the job configuration."),
             ('galaxy.jobs.dynamic_tool_destination', 'DEBUG', 'Finished config validation.')
         )
 

test/unit/jobs/dynamic_tool_destination/ymltests.py

@@ -945,3 +945,77 @@
         priority: mine
     verbose: True
 '''
+
+#### New Tests
+
+# not all priorities in tool destinations
+ivYMLTest149 = '''
+    default_destination:
+      priority:
+        med: waffles_low
+        lowish: waffles_low
+        high: waffles_default
+        higher: waffles_high
+    tools:
+      yuck:
+        default_destination:
+          priority:
+            med: waffles_default
+            high: waffles_high
+    verbose: True
+'''
+
+# rule destination not in job config
+ivYMLTest150 = '''
+    default_destination:
+      priority:
+        med: waffles_low
+        high: waffles_default
+    tools:
+      blegh:
+        rules:
+          - rule_type: num_input_datasets
+            nice_value: 0
+            lower_bound: 0
+            upper_bound: Infinity
+            destination:
+              priority:
+                med: fake_destination
+                high: waffles_default
+    verbose: True
+'''
+
+# tool default destination not in job config
+ivYMLTest151 = '''
+    default_destination:
+      priority:
+        med: waffles_low
+        high: waffles_default
+    tools:
+      blah:
+        rules:
+          - rule_type: num_input_datasets
+            nice_value: 0
+            lower_bound: 0
+            upper_bound: Infinity
+            destination:
+              priority:
+                med: waffles_default
+                high: waffles_default
+        default_destination:
+          priority:
+            med: not_true_destination
+            high: waffles_default
+    verbose: True
+'''
+
+# default destination not in job config
+ivYMLTest152 = '''
+    default_destination:
+      priority:
+        med: no_such_dest
+    users:
+      user@email.com:
+        priority: med
+    verbose: True
+'''