details: http://www.bx.psu.edu/hg/galaxy/rev/d995557d383b changeset: 3416:d995557d383b user: Greg Von Kuster <greg@bx.psu.edu> date: Fri Feb 19 10:01:33 2010 -0500 description: Re-fix new functional test support for setting check boxes and boolean tool params to false, and fix a broken functional test in the security and libraries suite. diffstat: lib/galaxy/web/controllers/library_common.py | 24 ++++++++++---------- templates/library/common/browse_library.mako | 4 +- templates/library/common/common.mako | 2 +- test-data/gops_concat_out2.bed | 10 ++++---- test/base/twilltestcase.py | 31 ++++++++++++++++--------- test/functional/test_security_and_libraries.py | 13 ++++------ 6 files changed, 45 insertions(+), 39 deletions(-) diffs (228 lines): diff -r 52edc83ed81c -r d995557d383b lib/galaxy/web/controllers/library_common.py --- a/lib/galaxy/web/controllers/library_common.py Thu Feb 18 21:38:49 2010 -0500 +++ b/lib/galaxy/web/controllers/library_common.py Fri Feb 19 10:01:33 2010 -0500 @@ -1067,7 +1067,6 @@ # use act_on_multiple_datasets( self, trans, cntrller, library_id, ldda_ids='', **kwd ) since it does what we need kwd['do_action'] = 'zip' return self.act_on_multiple_datasets( trans, cntrller, library_id, ldda_ids=id, **kwd ) - else: mime = trans.app.datatypes_registry.get_mimetype_by_extension( ldda.extension.lower() ) trans.response.set_content_type( mime ) @@ -1198,15 +1197,16 @@ msg = util.restore_text( params.get( 'msg', '' ) ) messagetype = params.get( 'messagetype', 'done' ) show_deleted = util.string_as_bool( params.get( 'show_deleted', False ) ) + action = params.get( 'do_action', None ) if not ldda_ids: msg = "You must select at least one dataset" messagetype = 'error' - elif not params.do_action: + elif not action: msg = "You must select an action to perform on selected datasets" messagetype = 'error' else: ldda_ids = util.listify( ldda_ids ) - if params.do_action == 'add': + if action == 'add': history = trans.get_history() total_imported_lddas = 0 msg = '' @@ -1223,7 +1223,7 @@ trans.sa_session.add( history ) trans.sa_session.flush() msg += "%i dataset(s) have been imported into your history. " % total_imported_lddas - elif params.do_action == 'manage_permissions': + elif action == 'manage_permissions': # We need the folder containing the LibraryDatasetDatasetAssociation(s) ldda = trans.sa_session.query( trans.app.model.LibraryDatasetDatasetAssociation ).get( trans.security.decode_id( ldda_ids[0] ) ) trans.response.send_redirect( web.url_for( controller='library_common', @@ -1235,7 +1235,7 @@ show_deleted=show_deleted, msg=util.sanitize_text( msg ), messagetype=messagetype ) ) - elif params.do_action == 'delete': + elif action == 'delete': for ldda_id in ldda_ids: ldda = trans.sa_session.query( trans.app.model.LibraryDatasetDatasetAssociation ).get( trans.security.decode_id( ldda_id ) ) ldda.deleted = True @@ -1245,18 +1245,18 @@ else: error = False try: - if params.do_action == 'zip': + if action == 'zip': # Can't use mkstemp - the file must not exist first tmpd = tempfile.mkdtemp() - tmpf = os.path.join( tmpd, 'library_download.' + params.do_action ) + tmpf = os.path.join( tmpd, 'library_download.' + action ) if ziptype == '64': archive = zipfile.ZipFile( tmpf, 'w', zipfile.ZIP_DEFLATED, True ) else: archive = zipfile.ZipFile( tmpf, 'w', zipfile.ZIP_DEFLATED ) archive.add = lambda x, y: archive.write( x, y.encode('CP437') ) - elif params.do_action == 'tgz': + elif action == 'tgz': archive = util.streamball.StreamBall( 'w|gz' ) - elif params.do_action == 'tbz': + elif action == 'tbz': archive = util.streamball.StreamBall( 'w|bz2' ) except (OSError, zipfile.BadZipFile): error = True @@ -1321,7 +1321,7 @@ msg = "Unable to create archive for download, please report this error" messagetype = 'error' if not error: - if params.do_action == 'zip': + if action == 'zip': archive.close() tmpfh = open( tmpf ) # clean up now @@ -1335,11 +1335,11 @@ messagetype = 'error' if not error: trans.response.set_content_type( "application/x-zip-compressed" ) - trans.response.headers[ "Content-Disposition" ] = "attachment; filename=GalaxyLibraryFiles.%s" % params.do_action + trans.response.headers[ "Content-Disposition" ] = "attachment; filename=GalaxyLibraryFiles.%s" % action return tmpfh else: trans.response.set_content_type( "application/x-tar" ) - trans.response.headers[ "Content-Disposition" ] = "attachment; filename=GalaxyLibraryFiles.%s" % params.do_action + trans.response.headers[ "Content-Disposition" ] = "attachment; filename=GalaxyLibraryFiles.%s" % action archive.wsgi_status = trans.response.wsgi_status() archive.wsgi_headeritems = trans.response.wsgi_headeritems() return archive.stream diff -r 52edc83ed81c -r d995557d383b templates/library/common/browse_library.mako --- a/templates/library/common/browse_library.mako Thu Feb 18 21:38:49 2010 -0500 +++ b/templates/library/common/browse_library.mako Fri Feb 19 10:01:33 2010 -0500 @@ -472,12 +472,12 @@ %if cntrller in [ 'library', 'requests' ]: ${self.render_folder( 'library', library.root_folder, 0, created_ldda_ids, trans.security.encode_id( library.id ), hidden_folder_ids, tracked_datasets, show_deleted=show_deleted, parent=None, row_counter=row_counter, root_folder=True )} %if not library.deleted: - ${render_actions_on_multiple_items( 'library', default_action=default_action )} + ${render_actions_on_multiple_items()} %endif %elif cntrller in [ 'library_admin', 'requests_admin' ]: ${self.render_folder( 'library_admin', library.root_folder, 0, created_ldda_ids, trans.security.encode_id( library.id ), [], tracked_datasets, show_deleted=show_deleted, parent=None, row_counter=row_counter, root_folder=True )} %if not library.deleted and not show_deleted: - ${render_actions_on_multiple_items( 'library_admin' )} + ${render_actions_on_multiple_items()} %endif %endif </table> diff -r 52edc83ed81c -r d995557d383b templates/library/common/common.mako --- a/templates/library/common/common.mako Thu Feb 18 21:38:49 2010 -0500 +++ b/templates/library/common/common.mako Fri Feb 19 10:01:33 2010 -0500 @@ -356,7 +356,7 @@ %endif </%def> -<%def name="render_actions_on_multiple_items( cntrller, default_action=None )"> +<%def name="render_actions_on_multiple_items()"> <tfoot> <tr> <td colspan="4" style="padding-left: 42px;"> diff -r 52edc83ed81c -r d995557d383b test-data/gops_concat_out2.bed --- a/test-data/gops_concat_out2.bed Thu Feb 18 21:38:49 2010 -0500 +++ b/test-data/gops_concat_out2.bed Fri Feb 19 10:01:33 2010 -0500 @@ -63,8 +63,8 @@ chrX 152648964 152649196 CCDS14733.1_cds_0_0_chrX_152648965_r 0 - chrX 152691446 152691471 CCDS14735.1_cds_0_0_chrX_152691447_f 0 + chrX 152694029 152694263 CCDS14736.1_cds_0_0_chrX_152694030_r 0 - -chr1 4348187 4348589 3.70 4.90 2.55 -chr1 4488177 4488442 4.03 5.77 1.92 -chr1 4774091 4774440 8.07 8.33 7.82 -chr1 4800122 4800409 6.40 7.35 5.44 -chr1 4878925 4879277 2.18 0.28 4.93 +chr1 4348187 4348589 . . + +chr1 4488177 4488442 . . + +chr1 4774091 4774440 . . + +chr1 4800122 4800409 . . + +chr1 4878925 4879277 . . + diff -r 52edc83ed81c -r d995557d383b test/base/twilltestcase.py --- a/test/base/twilltestcase.py Thu Feb 18 21:38:49 2010 -0500 +++ b/test/base/twilltestcase.py Fri Feb 19 10:01:33 2010 -0500 @@ -862,17 +862,14 @@ # Copied from form_builder.CheckboxField if value == True: return True - if isinstance( value, basestring ) and value.lower() in ( "yes", "true", "on" ): - return True - # 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 + if isinstance( value, list ): + value = value[0] + return isinstance( value, basestring ) and value.lower() in ( "yes", "true", "on" ) try: checkbox = control.get() checkbox.selected = is_checked( control_value ) except Exception, e1: + print "Attempting to set checkbox selected value threw exception: ", e1 # if there's more than one checkbox, probably should use the behaviour for # ClientForm.ListControl ( see twill code ), but this works for now... for elem in control_value: @@ -885,6 +882,7 @@ try: tc.fv( f.name, control.name, str( elem ) ) except Exception, e2: + print "Attempting to set control '", control.name, "' to value '", elem, "' threw exception: ", e2 # Galaxy truncates long file names in the dataset_collector in ~/parameters/basic.py if len( elem ) > 30: elem_name = '%s..%s' % ( elem[:17], elem[-11:] ) @@ -1654,11 +1652,22 @@ self.home() def download_archive_of_library_files( self, cntrller, library_id, ldda_ids, format ): self.home() - self.visit_url( "%s/library_common/browse_library?cntrller=%s&id=%s" % ( self.url, cntrller, library_id ) ) + # Here it would be ideal to have twill set form values and submit the form, but + # twill barfs on that due to the recently introduced page wrappers around the contents + # of the browse_library.mako template which enable panel layout when visiting the + # page from an external URL. By "barfs", I mean that twill somehow loses hod on the + # cntrller param. We'll just simulate the form submission by building the URL manually. + # Here's the old, better approach... + #self.visit_url( "%s/library_common/browse_library?cntrller=%s&id=%s" % ( self.url, cntrller, library_id ) ) + #for ldda_id in ldda_ids: + # tc.fv( "1", "ldda_ids", ldda_id ) + #tc.fv( "1", "do_action", format ) + #tc.submit( "action_on_datasets_button" ) + # Here's the new approach... + url = "%s/library_common/act_on_multiple_datasets?cntrller=%s&library_id=%s&do_action=%s" % ( self.url, cntrller, library_id, format ) for ldda_id in ldda_ids: - tc.fv( "1", "ldda_ids", ldda_id ) - tc.fv( "1", "do_action", format ) - tc.submit( "action_on_datasets_button" ) + url += "&ldda_ids=%s" % ldda_id + self.visit_url( url ) tc.code( 200 ) archive = self.write_temp_file( self.last_page(), suffix=format ) self.home() diff -r 52edc83ed81c -r d995557d383b test/functional/test_security_and_libraries.py --- a/test/functional/test_security_and_libraries.py Thu Feb 18 21:38:49 2010 -0500 +++ b/test/functional/test_security_and_libraries.py Fri Feb 19 10:01:33 2010 -0500 @@ -1596,18 +1596,15 @@ self.home() self.logout() self.login( email=admin_user.email ) - """ - TODO: debug this, somebody recently broke it def test_167_download_archive_of_library_files( self ): - Testing downloading an archive of files from the library + """Testing downloading an archive of files from the library""" for format in ( 'tbz', 'tgz', 'zip' ): - archive = self.download_archive_of_library_files( 'library', - self.security.encode_id( library_one.id ), - ( self.security.encode_id( ldda_one.id ), self.security.encode_id( ldda_two.id ) ), - format ) + archive = self.download_archive_of_library_files( cntrller='library', + library_id=self.security.encode_id( library_one.id ), + ldda_ids=[ self.security.encode_id( ldda_one.id ), self.security.encode_id( ldda_two.id ) ], + format=format ) self.check_archive_contents( archive, ( ldda_one, ldda_two ) ) os.remove( archive ) - """ def test_170_mark_group_deleted( self ): """Testing marking a group as deleted""" # Logged in as admin_user