details: http://www.bx.psu.edu/hg/galaxy/rev/da557305232f changeset: 3326:da557305232f user: Greg Von Kuster <greg@bx.psu.edu> date: Thu Feb 04 09:18:17 2010 -0500 description: Miscellaneous code cleanup: clean up old code in form_builder, adding important comments for the CheckboxField class; make the functional test diff checker for PDF files more flexible; allow tab chars to be mapped so they can be properly stored in the db. diffstat: lib/galaxy/util/__init__.py | 3 ++- lib/galaxy/web/form_builder.py | 36 ++++++++++++++++++++---------------- test/base/twilltestcase.py | 42 ++++++++++++++++++++++++++++-------------- 3 files changed, 50 insertions(+), 31 deletions(-) diffs (134 lines): diff -r a213a1d8ac03 -r da557305232f lib/galaxy/util/__init__.py --- a/lib/galaxy/util/__init__.py Wed Feb 03 19:44:01 2010 -0500 +++ b/lib/galaxy/util/__init__.py Thu Feb 04 09:18:17 2010 -0500 @@ -122,7 +122,8 @@ '}' :'__cc__', '@' : '__at__', '\n' : '__cn__', - '\r' : '__cr__' + '\r' : '__cr__', + '\t' : '__tc__' } def restore_text(text): diff -r a213a1d8ac03 -r da557305232f lib/galaxy/web/form_builder.py --- a/lib/galaxy/web/form_builder.py Wed Feb 03 19:44:01 2010 -0500 +++ b/lib/galaxy/web/form_builder.py Thu Feb 04 09:18:17 2010 -0500 @@ -88,7 +88,7 @@ self.value = value or "" def get_html( self, prefix="" ): return '<textarea name="%s%s" rows="%d" cols="%d">%s</textarea>' \ - % ( prefix, self.name, self.rows, self.cols, escape(str(self.value), quote=True) ) + % ( prefix, self.name, self.rows, self.cols, escape( str( self.value ), quote=True ) ) def set_size(self, rows, cols): self.rows = rows self.cols = cols @@ -104,27 +104,31 @@ """ def __init__( self, name, checked=None ): self.name = name - self.checked = ( checked == True ) or ( type( checked ) == type( 'a' ) and ( checked.lower() in ( "yes", "true", "on" ) ) ) + self.checked = ( checked == True ) or ( isinstance( checked, basestring ) and ( checked.lower() in ( "yes", "true", "on" ) ) ) def get_html( self, prefix="" ): if self.checked: checked_text = "checked" - else: checked_text = "" + else: + checked_text = "" + # The hidden field is necessary because if the check box is not checked on the form, it will + # not be included in the request params. The hidden field ensure that this will happen. When + # parsing the request, the value 'true' in the hidden field actually means it is NOT checked. + # See the is_checked() method below. The prefix is necessary in each case to ensure functional + # correctness when the param is inside a conditional. return '<input type="checkbox" name="%s%s" value="true" %s><input type="hidden" name="%s%s" value="true">' \ % ( prefix, self.name, checked_text, prefix, self.name ) @staticmethod def is_checked( value ): - if value == True: # wierd behaviour caused by following check for 2 valued list - wtf? ross august 22 - return value - if type( value ) == list and len( value ) == 2: - return True - else: - return False + if value == True: + return value + # This may look strange upon initial inspection, but see the comments in the get_html() method + # above for clarification. Basically, if value is not True, then it will always be a list with + # 2 input fields ( a checkbox and a hidden field ) if the checkbox is checked. If it is not + # checked, then value will be only the hidden field. + return isinstance( value, list ) and len( value ) == 2 def set_checked(self, value): - if type(value) == type('a'): - if value.lower() in [ "yes", "true", "on" ]: - self.checked = True - else: - self.checked = False + if isinstance( value, basestring ): + self.checked = value.lower() in [ "yes", "true", "on" ] else: self.checked = value @@ -335,8 +339,8 @@ self.name = name self.multiple = multiple or False self.options = options - if value is not None: - if not isinstance( value, list ): value = [ value ] + if value and not isinstance( value, list ): + value = [ value ] else: value = [] self.value = value diff -r a213a1d8ac03 -r da557305232f test/base/twilltestcase.py --- a/test/base/twilltestcase.py Wed Feb 03 19:44:01 2010 -0500 +++ b/test/base/twilltestcase.py Thu Feb 04 09:18:17 2010 -0500 @@ -53,21 +53,35 @@ files_differ = True if files_differ: diff = difflib.unified_diff( local_file, history_data, "local_file", "history_data" ) - diff_slice = list( islice( diff, 40 ) ) + diff_slice = list( islice( diff, 40 ) ) if file1.endswith( '.pdf' ) or file2.endswith( '.pdf' ): - # PDF files contain both a creation and modification date, so we need to - # handle these differences. As long as the rest of the PDF file does not differ, - # we're ok. - if len( diff_slice ) == 13 and \ - diff_slice[6].startswith( '-/CreationDate' ) and diff_slice[7].startswith( '-/ModDate' ) \ - and diff_slice[8].startswith( '+/CreationDate' ) and diff_slice[9].startswith( '+/ModDate' ): - return True - for line in diff_slice: - for char in line: - if ord( char ) > 128: - raise AssertionError( "Binary data detected, not displaying diff" ) - raise AssertionError( "".join( diff_slice ) ) - return True + # PDF files contain creation dates, modification dates, ids and descriptions that change with each + # new file, so we need to handle these differences. As long as the rest of the PDF file does + # not differ we're ok. + valid_diff_strs = [ 'description', 'createdate', 'creationdate', 'moddate', 'id' ] + valid_diff = False + for line in diff_slice: + # Make sure to lower case strings before checking. + line = line.lower() + # Diff lines will always start with a + or - character, but handle special cases: '--- local_file \n', '+++ history_data \n' + if ( line.startswith( '+' ) or line.startswith( '-' ) ) and line.find( 'local_file' ) < 0 and line.find( 'history_data' ) < 0: + for vdf in valid_diff_strs: + if line.find( vdf ) < 0: + valid_diff = False + else: + valid_diff = True + # Stop checking as soon as we know we have a valid difference + break + if not valid_diff: + # Print out diff_slice so we can see what failed + print "###### diff_slice ######" + raise AssertionError( "".join( diff_slice ) ) + break + else: + for line in diff_slice: + for char in line: + if ord( char ) > 128: + raise AssertionError( "Binary data detected, not displaying diff" ) def get_filename( self, filename ): full = os.path.join( self.file_dir, filename)