Hello, The current tool testing options provided by Galaxy are excellent for verifying that Galaxy is functioning properly and that tools provide reproducible results.
However, I believe there are two related shortcomings to the current Galaxy approach, and I would like to propose a solution for these shortcomings. The first is that the workflow for the tool developer is very clunky, especially if the developer is building up a tool incrementally with a test-driven development (TDD) approach. For each parameter addition a whole new output file must be created externally, manually verified by the developer, possibly converted to a regular expression, and placed in the test data directory. Put another way, it is easy to iteratively build up a tool, but not to iteratively build a test case. The second point is a bit pedantic. The current approach only verifies that the output is the same as the supplied output, not that the output is actually "correct". The typical workflow I described above relies on manual inspection of the expected output file to verify it is "correct". I believe it is better to programmatically state assertions about what makes an output correct than to rely on manual verification, this serves both to reduce human error and act as documentation about what makes an output correct. To address these two points, I propose an extensible addition to the Galaxy tool syntax (and provide an implementation) to declaratively make assertions about output files. The syntax change is to the output child element of test elements. With this change, the output element may have a child element named assert_contents, which in turn may have any number of child elements each of which describes an assertion about the contents of the referenced output file. The file attribute on the output element is still required if an assert_contents element is not found, but it is optional if an assert_contents element is found. The whole file check described by the file attribute will be executed in addition to the listed assertions if both are present. As an example, the following fragment assserts that an output file contains the text 'chr7', does not contain the text 'chr8', and has a line matching the regular expression '.*\s+127489808\s+127494553'. <test> <param name="input" value="maf_stats_interval_in.dat" /> <output name="out_file1"> <assert_contents> <has_text text="chr7" /> <not_has_text text="chr8" /> <has_line_matching expression=".*\s+127489808\s+127494553" /> </assert_contents> </output> </test> Each potential child element of assert_contents corresponds to a simple python function. These functions are broken out into modules which are dynamically (mostly) loaded at test time. The extensibility of this approach comes from how trivial it is to add new assertion functions to a module and whole new modules of such functions. I have started work on three modules of assertion functions, these are for text, tabular, and XML output files respectively. has_text, not_has_text, and has_line_matching above are examples of three such assertion functions from the text module. To see how it works, here is a function from the file test/base/asserts/text.py defining the has_line_matching element: def assert_has_line_matching(output, expression): """ Asserts the specified output contains a line matching the regular expression specified by the argument expression.""" match = re.search("^%s$" % expression, output, flags = re.MULTILINE) assert match != None, "No line matching expression '%s' was found in output file." % expression As demonstrated, the function name corresponding to the element element_name is just assert_element_name. The code that calls these assertion functions, automatically matches XML attributes with function arguments by names, and matches an argument named output with a string containing the contents of the output file resulting from the test run. Matching function arguments this way gracefully allows for multiple arguments and optional arguments. There is additional information about the implementation at the end of this e-mail. This approach should really aide iterative development of tools. Each new parameter you add to a tool is going to change the output in some way, hopefully you will be able to describe how it affects the output as an assertion. As you add new parameters, the previous parameters will hopefully affect the output in the same way and the old assertion will not need to change, you will just need to add new ones. Obviously this won't always be the case, but hopefully changes to previous assertions will be minimal over time. I believe this process will be faster over time than repeatedly producing output files or interactive GUI based testing, and the final product will be a richer test case. I have attached two patches. The first patch (implementation.patch) is the patch that I propose merging into galaxy-central. It modifies the tool parser to parse these new elements, modifies twilltestcase.py to perform the assertions, and includes the three modules of assertions described above. The second patch (examples.patch) adds a data files to the test-data directory and modifies tools/filters/headWrapper.xml to demonstrate each of the initial assertions elements I have defined. This patch merely proves it works and provides a sandbox to quickly play around with these assertions from working examples, this is not meant to be merged into galaxy-central. The first patch can be imported by executing the following command from an up-to-date galaxy-central pull: hg patch /path/to/implementation.patch To try it out, apply the second patch (examples.patch) and run the functional test Show_beginning1 hg patch /path/to/examples.patch ./run_functional_tests.sh -id Show_beginning1 To view the examples ran with the test see tools/filters/headWrapper.xml after applying the second patch. Let me know if you have any questions, concerns, or if there are any changes I can make to get this work included in galaxy-central. Thanks for your time and consideration, -John ------------------------------------------------ John Chilton Software Developer University of Minnesota Supercomputing Institute Office: 612-625-0917 Cell: 612-226-9223 E-Mail: chil...@msi.umn.edu Advanced Usage: In addition to the argument output defined above, there are two other argument names that when used have a special meaning - children and verify_assertions_function. children is a parsed dictionary-based python description of the child element of the assertion element. verify_assertions_function is a function that takes in a string and the same parsed dictionary-based python description of assertion XML elements and checks them. Used in conjunction these can be used to by assertion function authors to allow for the expression recursively defining assertion over some subset of the output. Here is an example: <output name="out_file1"> <assert_contents> <element_text path="BlastOutput_iterations/Iteration/Iteration_hits/Hit/Hit_def"> <not_has_text text="EDK72998.1" /> <has_text_matching expression="ABK[\d\.]+" /> </element_text> </assert_contents> </output> With corresponding Python function definition: def assert_element_text(output, path, verify_assertions_function, children): """ Recursively checks the specified assertions against the text of the first element matching the specified path.""" text = xml_find_text(output, path) verify_assertions_function(text, children) The children argument could also be used to define other assertion specific syntaxes not dependent on verify_assertions_function. A note on this example: This example is admittedly convoluted, but it is working and included in the patch described above. My real desire for this functionality is for some tools I am developing that produce zip files. Stock Galaxy doesn't really play well with zip file base datatypes so this code is not included in the implementation, but you can imagine why this would be useful. I hope to be able to define tests like: <zip_has_file name="subfile"> <has_text text="subfile text" /> </zip_has_file>
# HG changeset patch # User John Chilton <jmchil...@gmail.com> # Date 1299729367 21600 # Node ID 8736e425a6dc68de1364471ca16c710d1feb3526 # Parent c65d7871da8867ae89c29eb789811920c399551d Patch adding an extensible syntax for declaring assertions that must hold for output files during functional testing. diff -r c65d7871da88 -r 8736e425a6dc lib/galaxy/tools/__init__.py --- a/lib/galaxy/tools/__init__.py Wed Mar 09 21:53:02 2011 -0600 +++ b/lib/galaxy/tools/__init__.py Wed Mar 09 21:56:07 2011 -0600 @@ -630,8 +630,31 @@ name = attrib.pop( 'name', None ) if name is None: raise Exception( "Test output does not have a 'name'" ) + + assert_elem = output_elem.find("assert_contents") + assert_list = None + # Trying to keep testing patch as localized as + # possible, this function should be relocated + # somewhere more conventional. + def convert_elem(elem): + """ Converts and XML element to a dictionary format, used by assertion checking code. """ + tag = elem.tag + attributes = dict( elem.attrib ) + child_elems = list( elem.getchildren() ) + converted_children = [] + for child_elem in child_elems: + converted_children.append( convert_elem(child_elem) ) + return {"tag" : tag, "attributes" : attributes, "children" : converted_children} + + if assert_elem is not None: + assert_list = [] + for assert_child in list(assert_elem): + assert_list.append(convert_elem(assert_child)) + + file = attrib.pop( 'file', None ) - if file is None: + # File no longer required if an list of assertions was present. + if assert_list is None and file is None: raise Exception( "Test output does not have a 'file'") attributes = {} # Method of comparison @@ -642,6 +665,9 @@ attributes['delta'] = int( attrib.pop( 'delta', '10000' ) ) attributes['sort'] = util.string_as_bool( attrib.pop( 'sort', False ) ) attributes['extra_files'] = [] + attributes['assert_list'] = assert_list + + if 'ftype' in attrib: attributes['ftype'] = attrib['ftype'] for extra in output_elem.findall( 'extra_files' ): diff -r c65d7871da88 -r 8736e425a6dc test/base/asserts/__init__.py --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/base/asserts/__init__.py Wed Mar 09 21:56:07 2011 -0600 @@ -0,0 +1,76 @@ +import inspect +import logging +import sys +log = logging.getLogger( __name__ ) + +assertion_module_names = ['text', 'tabular', 'xml'] + +# Code for loading modules containing assertion checking functions, to +# create a new module of assertion functions, create the needed python +# source file "test/base/asserts/<MODULE_NAME>.py" and add +# <MODULE_NAME> to the list of assertion module names defined above. +assertion_modules = [] +for assertion_module_name in assertion_module_names: + full_assertion_module_name = 'base.asserts.' + assertion_module_name + log.debug(full_assertion_module_name) + try: + #Dynamically import module + __import__(full_assertion_module_name) + assertion_module = sys.modules[full_assertion_module_name] + assertion_modules.append(assertion_module) + except Exception, e: + log.exception( 'Failed to load assertion module: %s %s' % (assertion_module_name, str(e))) + +def verify_assertions(data, assertion_description_list): + """ This function takes a list of assertions and a string to check + these assertions against. """ + for assertion_description in assertion_description_list: + verify_assertion(data, assertion_description) + +def verify_assertion(data, assertion_description): + tag = assertion_description["tag"] + assert_function_name = "assert_" + tag + assert_function = None + for assertion_module in assertion_modules: + if hasattr(assertion_module, assert_function_name): + assert_function = getattr(assertion_module, assert_function_name) + + if assert_function is None: + errmsg = "Unable to find test function associated with XML tag '%s'. Check your tool file syntax." % tag + raise AssertionError(errmsg) + + assert_function_args = inspect.getargspec(assert_function).args + args = {} + for attribute, value in assertion_description["attributes"].iteritems(): + if attribute in assert_function_args: + args[attribute] = value + + # Three special arguments automatically populated independently of + # tool XML attributes. output is passed in as the contents of the + # output file. verify_assertions_function is passed in as the + # verify_assertions function defined above, this allows + # recursively checking assertions on subsections of + # output. children is the parsed version of the child elements of + # the XML element describing this assertion. See + # assert_element_text in test/base/asserts/xml.py as an example of + # how to use verify_assertions_function and children in conjuction + # to apply assertion checking to a subset of the input. The parsed + # version of an elements child elements do not need to just define + # assertions, developers of assertion functions can also use the + # child elements in novel ways to define inputs the assertion + # checking function (for instance consider the following fictional + # assertion function for checking column titles of tabular output + # - <has_column_titles><with_name name="sequence"><with_name + # name="probability"></has_column_titles>.) + if "output" in assert_function_args: + args["output"] = data + + if "verify_assertions_function" in assert_function_args: + args["verify_assertions_function"] = verify_assertions + + if "children" in assert_function_args: + args["children"] = assertion_description["children"] + + # TODO: Verify all needed function arguments are specified. + assert_function(**args) + diff -r c65d7871da88 -r 8736e425a6dc test/base/asserts/tabular.py --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/base/asserts/tabular.py Wed Mar 09 21:56:07 2011 -0600 @@ -0,0 +1,17 @@ +import re + +def get_first_line(output): + match = re.search("^(.*)$", output, flags = re.MULTILINE) + if match is None: + return None + else: + return match.group(1) + +def assert_has_n_columns(output, n, sep = '\t'): + """ Asserts the tabular output contains n columns. The optional + sep argument specifies the column seperator used to determine the + number of columns.""" + n = int(n) + first_line = get_first_line(output) + assert first_line is not None, "Was expecting output with %d columns, but output was empty." % n + assert len(first_line.split(sep)) == n, "Output does not have %d columns." % n diff -r c65d7871da88 -r 8736e425a6dc test/base/asserts/text.py --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/base/asserts/text.py Wed Mar 09 21:56:07 2011 -0600 @@ -0,0 +1,30 @@ +import re + +def assert_has_text(output, text): + """ Asserts specified output contains the substring specified by + the argument text.""" + assert output.find(text) >= 0, "Output file did not contain expected text '%s' (ouptut '%s')" % (text, output) + +def assert_not_has_text(output, text): + """ Asserts specified output does not contain the substring + specified the argument text.""" + assert output.find(text) < 0, "Output file contains unexpected text '%s'" % text + +def assert_has_line(output, line): + """ Asserts the specified output contains the line specified the + argument line.""" + match = re.search("^%s$" % re.escape(line), output, flags = re.MULTILINE) + assert match != None, "No line of output file was '%s' (output was '%s') " % (line, output) + +def assert_has_text_matching(output, expression): + """ Asserts the specified output contains text matching the + regular expression specified by the argument expression.""" + match = re.search(expression, output) + assert match != None, "No text matching expression '%s' was found in output file." % expression + +def assert_has_line_matching(output, expression): + """ Asserts the specified output contains a line matching the + regular expression specified by the argument expression.""" + match = re.search("^%s$" % expression, output, flags = re.MULTILINE) + assert match != None, "No line matching expression '%s' was found in output file." % expression + diff -r c65d7871da88 -r 8736e425a6dc test/base/asserts/xml.py --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/base/asserts/xml.py Wed Mar 09 21:56:07 2011 -0600 @@ -0,0 +1,77 @@ +import elementtree.ElementTree +import re + +# Helper functions used to work with XML output. +def to_xml(output): + return elementtree.ElementTree.fromstring(output) + +def xml_find_text(output, path): + xml = to_xml(output) + text = xml.findtext(path) + return text + +def xml_find(output, path): + xml = to_xml(output) + return xml.find(path) + +def assert_is_valid_xml(output): + """ Simple assertion that just verifies the specified output + is valid XML.""" + try: + to_xml(output) + except Exception, e: + # TODO: Narrow caught exception to just parsing failure + raise AssertionError("Expected valid XML, but could not parse output. %s" % str(e)) + +def assert_has_element_with_path(output, path): + """ Asserts the specified output has at least one XML element with a + path matching the specified path argument. Valid paths are the + simplified subsets of XPath implemented by elementtree (currently + Galaxy makes use of elementtree 1.2). See + http://effbot.org/zone/element-xpath.htm for more information.""" + if xml_find(output, path) is None: + errmsg = "Expected to find XML element matching expression %s, not such match was found." % path + raise AssertionError(errmsg) + +def assert_has_n_elements_with_path(output, path, n): + """ Asserts the specified output has exactly n elements matching the + path specified.""" + xml = to_xml(output) + n = int(n) + num_elements = len(xml.findall(path)) + if num_elements != n: + errmsg = "Expected to find %d elements with path %s, but %d were found." % (n, path, num_elements) + raise AssertionError(errmsg) + +def assert_element_text_matches(output, path, expression): + """ Asserts the text of the first element matching the specified + path matches the specified regular expression.""" + text = xml_find_text(output, path) + if re.match(expression, text) is None: + errmsg = "Expected element with path '%s' to contain text matching '%s', instead text '%s' was found." % (path, text, actual_text) + raise AssertionError(errmsg) + +def assert_element_text_is(output, path, text): + """ Asserts the text of the first element matching the specified + path matches exactly the specified text. """ + assert_element_text_matches(output, path, re.escape(text)) + +def assert_attribute_matches(output, path, attribute, expression): + """ Asserts the specified attribute of the first element matching + the specified path matches the specified regular expression.""" + xml = xml_find(output, path) + attribute_value = xml.attrib[attribute] + if re.match(expression, attribute_value) is None: + errmsg = "Expected attribute '%s' on element with path '%s' to match '%s', instead attribute value was '%s'." % (attribute, path, expression, attribute_value) + raise AssertionError(errmsg) + +def assert_attribute_is(output, path, attribute, text): + """ Asserts the specified attribute of the first element matching + the specified path matches exactly the specified text.""" + assert_attribute_matches(output, path, attribute, re.escape(text)) + +def assert_element_text(output, path, verify_assertions_function, children): + """ Recursively checks the specified assertions against the text of + the first element matching the specified path.""" + text = xml_find_text(output, path) + verify_assertions_function(text, children) diff -r c65d7871da88 -r 8736e425a6dc test/base/twilltestcase.py --- a/test/base/twilltestcase.py Wed Mar 09 21:53:02 2011 -0600 +++ b/test/base/twilltestcase.py Wed Mar 09 21:56:07 2011 -0600 @@ -11,6 +11,7 @@ from elementtree import ElementTree from galaxy.web import security from galaxy.web.framework.helpers import iff +from base.asserts import verify_assertions buffer = StringIO.StringIO() @@ -614,7 +615,7 @@ elem = elems[0] self.assertTrue( hid ) self._assert_dataset_state( elem, 'ok' ) - if self.is_zipped( filename ): + if filename is not None and self.is_zipped( filename ): errmsg = 'History item %s is a zip archive which includes invalid files:\n' % hid zip_file = zipfile.ZipFile( filename, "r" ) name = zip_file.namelist()[0] @@ -626,44 +627,55 @@ if ext != test_ext: raise AssertionError( errmsg ) else: - local_name = self.get_filename( filename ) - temp_name = self.makeTfname(fname = filename) self.home() self.visit_page( "display?hid=" + hid ) data = self.last_page() - file( temp_name, 'wb' ).write(data) - try: - # have to nest try-except in try-finally to handle 2.4 + + assert_list = attributes["assert_list"] + if assert_list is not None: try: - if attributes is None: - attributes = {} - if attributes.get( 'ftype', None ) == 'bam': - local_fh, temp_name = self._bam_to_sam( local_name, temp_name ) - local_name = local_fh.name - compare = attributes.get( 'compare', 'diff' ) - extra_files = attributes.get( 'extra_files', None ) - if compare == 'diff': - self.files_diff( local_name, temp_name, attributes=attributes ) - elif compare == 're_match': - self.files_re_match( local_name, temp_name, attributes=attributes ) - elif compare == 're_match_multiline': - self.files_re_match_multiline( local_name, temp_name, attributes=attributes ) - elif compare == 'sim_size': - delta = attributes.get('delta','100') - s1 = len(data) - s2 = os.path.getsize(local_name) - if abs(s1-s2) > int(delta): - raise Exception, 'Files %s=%db but %s=%db - compare (delta=%s) failed' % (temp_name,s1,local_name,s2,delta) - else: - raise Exception, 'Unimplemented Compare type: %s' % compare - if extra_files: - self.verify_extra_files_content( extra_files, elem.get( 'id' ) ) + verify_assertions(data, assert_list) except AssertionError, err: - errmsg = 'History item %s different than expected, difference (using %s):\n' % ( hid, compare ) + errmsg = 'History item %s different than expected\n' % (hid) errmsg += str( err ) - raise AssertionError( errmsg ) - finally: - os.remove( temp_name ) + raise AssertionError( errmsg ) + + if filename is not None: + local_name = self.get_filename( filename ) + temp_name = self.makeTfname(fname = filename) + file( temp_name, 'wb' ).write(data) + try: + # have to nest try-except in try-finally to handle 2.4 + try: + if attributes is None: + attributes = {} + if attributes.get( 'ftype', None ) == 'bam': + local_fh, temp_name = self._bam_to_sam( local_name, temp_name ) + local_name = local_fh.name + compare = attributes.get( 'compare', 'diff' ) + extra_files = attributes.get( 'extra_files', None ) + if compare == 'diff': + self.files_diff( local_name, temp_name, attributes=attributes ) + elif compare == 're_match': + self.files_re_match( local_name, temp_name, attributes=attributes ) + elif compare == 're_match_multiline': + self.files_re_match_multiline( local_name, temp_name, attributes=attributes ) + elif compare == 'sim_size': + delta = attributes.get('delta','100') + s1 = len(data) + s2 = os.path.getsize(local_name) + if abs(s1-s2) > int(delta): + raise Exception, 'Files %s=%db but %s=%db - compare (delta=%s) failed' % (temp_name,s1,local_name,s2,delta) + else: + raise Exception, 'Unimplemented Compare type: %s' % compare + if extra_files: + self.verify_extra_files_content( extra_files, elem.get( 'id' ) ) + except AssertionError, err: + errmsg = 'History item %s different than expected, difference (using %s):\n' % ( hid, compare ) + errmsg += str( err ) + raise AssertionError( errmsg ) + finally: + os.remove( temp_name ) def _bam_to_sam( self, local_name, temp_name ): temp_local = tempfile.NamedTemporaryFile( suffix='.sam', prefix='local_bam_converted_to_sam_' )
# HG changeset patch # User John Chilton <jmchil...@gmail.com> # Date 1299729182 21600 # Node ID c65d7871da8867ae89c29eb789811920c399551d # Parent b001ba3b7b3a2eabc3c6d90744286db7b4f9bdcd Changes to headerWrapper.xml to demonstrate all assertion functions. diff -r b001ba3b7b3a -r c65d7871da88 test-data/xml_for_testing.xml --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test-data/xml_for_testing.xml Wed Mar 09 21:53:02 2011 -0600 @@ -0,0 +1,7 @@ +<?xml version="1.0"?> +<root> + <outerElement> + <innerElement1 foo="bar" /> + <innerElement2 foo2="bar2345" /> + </outerElement> +</root> diff -r b001ba3b7b3a -r c65d7871da88 tools/filters/headWrapper.xml --- a/tools/filters/headWrapper.xml Wed Mar 09 15:33:04 2011 -0500 +++ b/tools/filters/headWrapper.xml Wed Mar 09 21:53:02 2011 -0600 @@ -1,4 +1,4 @@ -<tool id="Show beginning1" name="Select first"> +<tool id="Show_beginning1" name="Select first"> <description>lines from a dataset</description> <command interpreter="perl">headWrapper.pl $input $lineNum $out_file1</command> <inputs> @@ -14,6 +14,48 @@ <param name="input" value="1.bed"/> <output name="out_file1" file="eq-showbeginning.dat"/> </test> + <test> + <param name="input" value="maf_stats_interval_in.dat" /> + <param name="lineNum" value="99999"/> + <output name="out_file1"> + <assert_contents> + <has_text text="chr7" /> + <not_has_text text="chr8" /> + <has_text_matching expression="1274\d+53" /> + <has_line_matching expression=".*\s+127489808\s+127494553" /> + <!-- 	 is XML escape code for tab --> + <has_line line="chr7	127471195	127489808" /> + <has_n_columns n="3" /> + </assert_contents> + </output> + </test> + <test> + <param name="input" value="blastp_sample.xml" /> + <param name="lineNum" value="99999"/> + <output name="out_file1"> + <assert_contents> + <is_valid_xml /> + <has_element_with_path path="BlastOutput_param/Parameters/Parameters_matrix" /> + <has_n_elements_with_path n="9" path="BlastOutput_iterations/Iteration/Iteration_hits/Hit/Hit_num" /> + <element_text_matches path="BlastOutput_version" expression="BLASTP\s+2\.2.*" /> + <element_text_is path="BlastOutput_program" text="blastp" /> + <element_text path="BlastOutput_iterations/Iteration/Iteration_hits/Hit/Hit_def"> + <not_has_text text="EDK72998.1" /> + <has_text_matching expression="ABK[\d\.]+" /> + </element_text> + </assert_contents> + </output> + </test> + <test> + <param name="input" value="xml_for_testing.xml" /> + <param name="lineNum" value="99999"/> + <output name="out_file1"> + <assert_contents> + <attribute_is path="outerElement/innerElement1" attribute="foo" text="bar" /> + <attribute_matches path="outerElement/innerElement2" attribute="foo2" expression="bar\d+" /> + </assert_contents> + </output> + </test> </tests> <help>
___________________________________________________________ Please keep all replies on the list by using "reply all" in your mail client. To manage your subscriptions to this and other Galaxy lists, please use the interface at: http://lists.bx.psu.edu/