From 48b7a50eb204fce54baedaca432ff28792b562cc Mon Sep 17 00:00:00 2001
From: John Chilton <jmchilton@gmail.com>
Date: Thu, 21 Nov 2013 23:37:22 -0600
Subject: [PATCH] Fixes related to implicit defaults of param values. Logic
 errors related to them being contained in repeat blocks and to picking the
 top value in a select by default when no other value is marked as default.

Other small adjustments - add another sample tool demonstrating the problem and clean up error messages.
---
 lib/galaxy/tools/test.py                      | 26 +++++++---
 .../tools/implicit_default_conds.xml          | 49 +++++++++++++++++++
 test/functional/tools/samples_tool_conf.xml   |  1 +
 3 files changed, 70 insertions(+), 6 deletions(-)
 create mode 100644 test/functional/tools/implicit_default_conds.xml

diff --git a/lib/galaxy/tools/test.py b/lib/galaxy/tools/test.py
index 7015eaa3c8a..73939003932 100644
--- a/lib/galaxy/tools/test.py
+++ b/lib/galaxy/tools/test.py
@@ -78,13 +78,15 @@ class ToolTestBuilder( object ):
                     if selected:
                         default_option = name
                 else:
-                    default_option = test_param.static_options[0]
+                    first_option = test_param.static_options[0]
+                    first_option_value = first_option[1]
+                    default_option = first_option_value
                 matches_declared_value = lambda case_value: case_value == default_option
             else:
                 # No explicit value for this param and cannot determine a
                 # default - give up. Previously this would just result in a key
                 # error exception.
-                msg = "Failed to find test parameter specification required for conditional %s" % cond
+                msg = "Failed to find test parameter value specification required for conditional %s" % cond.name
                 raise Exception( msg )
 
         # Check the tool's defined cases against predicate to determine
@@ -93,7 +95,9 @@ class ToolTestBuilder( object ):
             if matches_declared_value( case.value ):
                 return case
         else:
-            log.info("Failed to find case matching test parameter specification for cond %s. Remainder of test behavior is unspecified." % cond)
+            msg_template = "%s - Failed to find case matching value (%s) for test parameter specification for conditional %s. Remainder of test behavior is unspecified."
+            msg = msg_template % ( self.tool.id, declared_value, cond.name )
+            log.info( msg )
 
     def __split_if_str( self, value ):
         split = isinstance(value, str)
@@ -143,10 +147,20 @@ class ToolTestBuilder( object ):
                 case_value = raw_input[ 1 ] if raw_input else None
                 case = self.__matching_case_for_value( value, case_value )
                 if case:
-                    expanded_value = self.__split_if_str( case.value )
-                    expanded_inputs[ case_context.for_state() ] = expanded_value
                     for input_name, input_value in case.inputs.items():
-                        expanded_inputs.update( self.__process_raw_inputs( { input_name: input_value }, raw_inputs, parent_context=cond_context ) )
+                        case_inputs = self.__process_raw_inputs( { input_name: input_value }, raw_inputs, parent_context=cond_context )
+                        expanded_inputs.update( case_inputs )
+                    expanded_case_value = self.__split_if_str( case.value )
+                    if case_value is not None:
+                        # A bit tricky here - we are growing inputs with value
+                        # that may be implicit (i.e. not defined by user just
+                        # a default defined in tool). So we do not want to grow
+                        # expanded_inputs and risk repeat block viewing this
+                        # as a new instance with value defined and hence enter
+                        # an infinite loop - hence the "case_value is not None"
+                        # check.
+                        expanded_inputs[ case_context.for_state() ] = expanded_case_value
+
             elif isinstance( value, grouping.Repeat ):
                 repeat_index = 0
                 while True:
diff --git a/test/functional/tools/implicit_default_conds.xml b/test/functional/tools/implicit_default_conds.xml
new file mode 100644
index 00000000000..0a5fe3b7d77
--- /dev/null
+++ b/test/functional/tools/implicit_default_conds.xml
@@ -0,0 +1,49 @@
+<tool id="implicit_default_conds" name="implicit_default_conds">
+    <command>
+        echo "$param_group[0].p1.val"  >> $out_file1;
+        echo "$param_group[0].p2.val"  >> $out_file1;
+    </command>
+    <inputs>
+        <repeat name="param_group" title="Param Group" min="1">
+            <conditional name="p1">
+                <param name="type" type="select">
+                    <option value="default">THE DEFAULT</option>
+                    <option value="different">A different value</option>
+                </param>
+                <when value="default">
+                    <param name="val" value="p1default" type="text" />
+                </when>
+                <when value="different">
+                    <param name="val" value="p1different" type="text" />
+                </when>
+            </conditional>
+            <conditional name="p2">
+                <param name="type" type="select">
+                    <option value="default">THE DEFAULT</option>
+                    <option value="different" selected="true">A different value</option>
+                </param>
+                <when value="default">
+                    <param name="val" value="p2default" type="text" />
+                </when>
+                <when value="different">
+                    <param name="val" value="p2different" type="text" />
+                </when>
+            </conditional>
+            <param name="int_param" type="integer" value="8" />
+        </repeat>
+    </inputs>
+    <outputs>
+        <data name="out_file1" format="txt" />
+    </outputs>
+    <tests>
+        <test>
+            <param name="int_param" value="7" /> <!-- Specify at least one value in repeat to force one instance. -->
+            <output name="out_file1">
+                <assert_contents>
+                    <has_line line="p1default" />
+                    <has_line line="p2different" />
+                </assert_contents>
+            </output>
+        </test>
+    </tests>
+</tool>
diff --git a/test/functional/tools/samples_tool_conf.xml b/test/functional/tools/samples_tool_conf.xml
index 7181a42c5ad..bc008c17468 100644
--- a/test/functional/tools/samples_tool_conf.xml
+++ b/test/functional/tools/samples_tool_conf.xml
@@ -13,4 +13,5 @@
   <tool file="output_order.xml" />
   <tool file="disambiguate_repeats.xml" />
   <tool file="min_repeat.xml" />
+  <tool file="implicit_default_conds.xml" />
 </toolbox>
\ No newline at end of file