[asterisk-dev] [Code Review]: Initial tagging of tests in Asterisk Test Suite

Matt Jordan reviewboard at asterisk.org
Tue Mar 6 08:50:07 CST 2012



> On March 5, 2012, 2:58 a.m., wdoekes wrote:
> > /asterisk/trunk/runtests.py, lines 203-213
> > <https://reviewboard.asterisk.org/r/1791/diff/1/?file=25446#file25446line203>
> >
> >     I'd prefer this for output, but that's only because I had time to spare ;)
> >     
> >     len3 = int((len(tags) + 2) / 3)
> >     l1, l2, l3 = tags[:len3], tags[len3:len3*2], tags[len3*2:]
> >     for i in range(len3):
> >         print '\t%-*s      %-*s      %-*s' % (
> >             maxwidth, ''.join(l1[i:i+1]),
> >             maxwidth, ''.join(l2[i:i+1]),
> >             maxwidth, ''.join(l3[i:i+1])
> >         )
> >
> 
> Matt Jordan wrote:
>     I like it as well, although it doesn't work well if the length of tags is less than 3.  Granted, that's not very likely to happen - but nothing says it can't. :-)
>     
>     That being said, I'm not a big fan of having to use a counter inside an iterator.  Modified this slightly to use part of yours and an iterator to split the lists up into chunks.
> 
> wdoekes wrote:
>     What do you mean, not work well with 2 or less items?
>     
>     
>     $ python nix.py 
>     Mine:
>     	aap      noot         
>     Matt's:
>     	aap      noot         
>     
>     
>     And your version is back to LTR sorting instead of top-down:
>     
>     
>     $ python nix.py 
>     Mine:
>     	aap        noot       steel 
>     	C          Perl       vork  
>     	C++        PHP              
>     	mies       Python           
>     Matt:
>     	aap        C          C++   
>     	mies       noot       Perl  
>     	PHP        Python     steel 
>     	vork                        
>     
>     
>     Of course this is a non-issue, except when you say that my version doesn't work well ;)
>     
>     P.S.1. You don't need the ''.join().. You're doing ''.join('abc') which is already 'abc'. My version needed it to handle empty slices, you're handling those with an if.
>     P.S.2. list()-ifying your generator (diff 2, line 205) is unnecessary; leftover debug code?
>     P.S.3. So, for your enjoyment -- or mostly mine -- I bring you:
>     
>     def chunks(l, n):
>         l = list(l) # allow generators
>         for i in xrange(0, len(l), n):
>             yield l[i:i+n]
>     
>     def columnify(in_list, columns):
>         items = list(in_list) # allow generators
>         len_per_column = int((len(items) + columns - 1) / columns)
>         for column in range(len_per_column):
>             for i in range(0, len(items), len_per_column):
>                 try:
>                     yield items[i + column]
>                 except IndexError:
>                     yield ''
>     
>     columns = 3
>     for tag in chunks(columnify(tags, columns), columns):
>         print '\t' + '     '.join('%-*s' % (maxwidth, i) for i in tag)
>     
>

Interesting - I didn't know that when slicing a sequence, i.e., s[i:j], if j is greater then the length of s, len(s) is used automatically.  I'm certain I have length checking elsewhere in the Test Suite that's no longer needed as well... Whoops.  Objection removed!

Anyway, as you pointed out, mine does result in printing the items LTR, as opposed to top down - which is what I intended, actually.  I read left to right better than top to bottom :-)

P.S.1 - fixed
P.S.2 - yup, removed
P.S.3 - very nice


> On March 5, 2012, 2:58 a.m., wdoekes wrote:
> > /asterisk/trunk/runtests.py, lines 193-200
> > <https://reviewboard.asterisk.org/r/1791/diff/1/?file=25446#file25446line193>
> >
> >     If you're nesting that deep, you're not doing the python way ;)
> >     
> >     tags = []
> >     for test in tests:
> >         tags.extend(test.test_config.tags)
> >     tags = list(set(tags))
> >     tags.sort(key=str.lower)
> >     maxwidth = max(len(i) for i in tags)
> 
> Matt Jordan wrote:
>     That's much cleaner.  My only dislike is that before we build a set from the tags, we extend all tags onto the list.  With enough tests, that might get a little unwieldy... but premature optimization and all that.
> 
> wdoekes wrote:
>     You're right, both about the dislike and the premature optimization ;)
>     But in that case, set() comes to the rescue again.
>     
>     tags = set()
>     for test in tests:
>         tags = tags.union(test.test_config.tags)
>     tags = list(tags)

I like this better as well


- Matt


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1791/#review5733
-----------------------------------------------------------


On March 5, 2012, 9:55 a.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1791/
> -----------------------------------------------------------
> 
> (Updated March 5, 2012, 9:55 a.m.)
> 
> 
> Review request for Asterisk Developers, Paul Belanger, wdoekes, and opticron.
> 
> 
> Summary
> -------
> 
> In review 1771 (https://reviewboard.asterisk.org/r/1771), wdoekes modified the -t option in the Test Suite to only run a single test, as opposed to all tests that matched the parameter value.  While this was the correct behavior for that option, it did prevent running sets of tests (for example, by specifying "tests/channels/SIP").  Fortunately, the Test Suite has the ability to run sets of tests using the "tags" option, although most of the tests (with the exception of the fax tests) were not taking advantage of this capability.  
> 
> The tags feature allows each test to have zero or more tags associated with it, where if tags are associated with a test and specified as parameters to runtests.py, determine if it can run.  All tags specified on the command line must be matched in order for the test to run.
> 
> As an example, if Test A specifies:
> tags:
>   - SIP
>   - fax
> Then executing runtests.py with "-g SIP" will run the test, as will "-g SIP -g fax"; however, "-g SIP -g CDR" would not.
> 
> This patch does the following:
> 1) It tags the vast majority of tests with a default set of tags.  Tests that didn't really belong in any group with any other test were left alone (func_srv, for example), as having tests that can be run standalone using the -t option exist in a group with one member didn't make a lot of sense.
> 2) It adds an option to runtests.py to list all available tags (-L).  The current output of this option is:
> 
> Available test tags:
> 	AGI                 AMI                 apps                
> 	CDR                 chan_local          chanspy             
> 	confbridge          dial                dialplan            
> 	dialplan_lua        directory           DTMF                
> 	fax                 fax_gateway         fax_passthrough     
> 	features            IAX                 incomplete          
> 	mixmonitor          parking             pickup              
> 	queues              SIP                 transfer            
> 	voicemail         
> 3) It displays the value of a test configuration's tags field in a number of places, including the -l (list tests) option, as well as when a test is skipped
> 
> Additional tags could certainly be specified and added for many of the tests - this patch is merely a first effort at applying basic, useful tags across all tests.  It is expected that when new tests are added, part of the criteria for reviewing that test should be to check whether or not they are tagged appropriately.
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/tests/apps/voicemail/authenticate_invalid_mailbox/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/authenticate_invalid_password/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/directory_operator_exit/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/incomplete/sip_incomplete/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/authenticate_extensions/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/directory_context_operator_exit/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/directory_attendant_exit/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/confbridge/confbridge_nominal/test-config.yaml 3064 
>   /asterisk/trunk/tests/agi/exit_status/test-config.yaml 3064 
>   /asterisk/trunk/runtests.py 3076 
>   /asterisk/trunk/lib/python/asterisk/TestConfig.py 3064 
>   /asterisk/trunk/tests/fastagi/database/test-config.yaml 3064 
>   /asterisk/trunk/tests/fastagi/connect/test-config.yaml 3064 
>   /asterisk/trunk/tests/fastagi/control-stream-file/test-config.yaml 3064 
>   /asterisk/trunk/tests/fastagi/channel-status/test-config.yaml 3064 
>   /asterisk/trunk/tests/dialplan/test-config.yaml 3064 
>   /asterisk/trunk/tests/directed_pickup/test-config.yaml 3064 
>   /asterisk/trunk/tests/chanspy/chanspy_w_mixmonitor/test-config.yaml 3076 
>   /asterisk/trunk/tests/chanspy/chanspy_barge/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/use_contact_from_200/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/tcpauthtimeout/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/tcpauthlimit/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/sip_tls_register/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/sip_tls_call/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/sip_srtp/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/sip_register_domain_acl/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/sip_register/test-config.yaml 3070 
>   /asterisk/trunk/tests/channels/SIP/sip_outbound_address/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/sip_one_legged_transfer_v6/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/sip_one_legged_transfer/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/sip_hold/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/sip_channel_params/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/sip_blind_transfer/caller_with_reinvite/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/sip_blind_transfer/caller_refer_only/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/sip_blind_transfer/callee_with_reinvite/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/sip_blind_transfer/callee_refer_only/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/sip_attended_transfer_v6/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/sip_attended_transfer_tcp/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/sip_attended_transfer/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/secure_bridge_media/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/refer_replaces_to_self/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/realtime_sipregs/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/pcap_demo/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/realtime_nosipregs/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/options/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/noload_res_srtp_attempt_srtp/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/noload_res_srtp/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/nat_supertest/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/message_unauth_from/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/message_unauth/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/message_from_call/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/message_disabled/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/message_auth_cust_hdr/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/message_auth/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/info_dtmf/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/header_parsing/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/handle_response_refer/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/handle_response_address_incomplete/test-config.yaml 3064 
>   /asterisk/trunk/tests/channels/SIP/codec_negotiation/test-config.yaml 3064 
>   /asterisk/trunk/tests/cdr/originate-cdr-disposition/test-config.yaml 3064 
>   /asterisk/trunk/tests/cdr/nocdr/test-config.yaml 3064 
>   /asterisk/trunk/tests/cdr/console_fork_before_dial/test-config.yaml 3064 
>   /asterisk/trunk/tests/cdr/console_fork_after_busy_forward/test-config.yaml 3064 
>   /asterisk/trunk/tests/cdr/console_dial_sip_transfer/test-config.yaml 3064 
>   /asterisk/trunk/tests/cdr/console_dial_sip_congestion/test-config.yaml 3064 
>   /asterisk/trunk/tests/cdr/console_dial_sip_busy/test-config.yaml 3064 
>   /asterisk/trunk/tests/cdr/console_dial_sip_answer/test-config.yaml 3064 
>   /asterisk/trunk/tests/cdr/cdr_userfield/test-config.yaml 3064 
>   /asterisk/trunk/tests/cdr/cdr_unanswered_yes/test-config.yaml 3064 
>   /asterisk/trunk/tests/cdr/cdr_originate_sip_congestion_log/test-config.yaml 3064 
>   /asterisk/trunk/tests/cdr/blind-transfer-accountcode/test-config.yaml 3064 
>   /asterisk/trunk/tests/cdr/app_queue/test-config.yaml 3064 
>   /asterisk/trunk/tests/cdr/app_dial_G_flag/test-config.yaml 3064 
>   /asterisk/trunk/tests/callparking/test-config.yaml 3064 
>   /asterisk/trunk/tests/callparking_retrieval/test-config.yaml 3064 
>   /asterisk/trunk/tests/blind-transfer-parkingtimeout/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/leave_voicemail_priority/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/leave_voicemail_nominal/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/leave_voicemail_forwarding_auto_urgent/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/leave_voicemail_forwarding/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/leave_voicemail_external_notification/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/leave_voicemail_contexts/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/func_vmcount/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/check_voicemail_while_leaving_msg/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/check_voicemail_reply/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/check_voicemail_options_record_unavail/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/check_voicemail_options_record_temp/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/check_voicemail_options_record_name/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/check_voicemail_options_record_busy/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/check_voicemail_options_change_password/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/check_voicemail_nominal/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/check_voicemail_new_user_hangup/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/check_voicemail_new_user/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/check_voicemail_forward_with_prepend/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/check_voicemail_forward_hangup/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/check_voicemail_forward/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/check_voicemail_envelope/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/check_voicemail_dialout/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/check_voicemail_delete/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/check_voicemail_callback/test-config.yaml 3064 
>   /asterisk/trunk/tests/apps/voicemail/authenticate_nominal/test-config.yaml 3064 
>   /asterisk/trunk/tests/fastagi/execute/test-config.yaml 3064 
>   /asterisk/trunk/tests/fastagi/get-data/test-config.yaml 3064 
>   /asterisk/trunk/tests/fastagi/hangup/test-config.yaml 3064 
>   /asterisk/trunk/tests/fastagi/record-file/test-config.yaml 3064 
>   /asterisk/trunk/tests/fastagi/say-alpha/test-config.yaml 3064 
>   /asterisk/trunk/tests/fastagi/say-date/test-config.yaml 3064 
>   /asterisk/trunk/tests/fastagi/say-datetime/test-config.yaml 3064 
>   /asterisk/trunk/tests/fastagi/say-digits/test-config.yaml 3064 
>   /asterisk/trunk/tests/fastagi/say-number/test-config.yaml 3064 
>   /asterisk/trunk/tests/fastagi/say-phonetic/test-config.yaml 3064 
>   /asterisk/trunk/tests/fastagi/say-time/test-config.yaml 3064 
>   /asterisk/trunk/tests/fastagi/stream-file/test-config.yaml 3064 
>   /asterisk/trunk/tests/fastagi/wait-for-digit/test-config.yaml 3064 
>   /asterisk/trunk/tests/feature_attended_transfer/test-config.yaml 3064 
>   /asterisk/trunk/tests/feature_blonde_transfer/test-config.yaml 3064 
>   /asterisk/trunk/tests/iax2/basic-call/test-config.yaml 3064 
>   /asterisk/trunk/tests/manager/action-events-response/test-config.yaml 3064 
>   /asterisk/trunk/tests/manager/authlimit/test-config.yaml 3064 
>   /asterisk/trunk/tests/manager/authtimeout/test-config.yaml 3064 
>   /asterisk/trunk/tests/manager/login/test-config.yaml 3064 
>   /asterisk/trunk/tests/manager/response-time/test-config.yaml 3064 
>   /asterisk/trunk/tests/mixmonitor/test-config.yaml 3064 
>   /asterisk/trunk/tests/mixmonitor_audiohook_inherit/test-config.yaml 3064 
>   /asterisk/trunk/tests/one-step-parking/test-config.yaml 3064 
>   /asterisk/trunk/tests/pbx/call-files/test-config.yaml 3064 
>   /asterisk/trunk/tests/pbx/call-files2/test-config.yaml 3064 
>   /asterisk/trunk/tests/pbx/merge_contexts/test-config.yaml 3064 
>   /asterisk/trunk/tests/pbx/pbx_lua_background/test-config.yaml 3064 
>   /asterisk/trunk/tests/pbx/pbx_lua_goto/test-config.yaml 3064 
>   /asterisk/trunk/tests/queues/macro_gosub_test/test-config.yaml 3064 
>   /asterisk/trunk/tests/queues/position_priority_maxlen/test-config.yaml 3064 
>   /asterisk/trunk/tests/queues/queue_baseline/test-config.yaml 3064 
>   /asterisk/trunk/tests/queues/ringinuse_and_pause/test-config.yaml 3064 
>   /asterisk/trunk/tests/queues/set_penalty/test-config.yaml 3064 
>   /asterisk/trunk/tests/queues/wrapup_time/test-config.yaml 3064 
>   /asterisk/trunk/tests/regressions/M18882/test-config.yaml 3064 
>   /asterisk/trunk/tests/rfc2833_dtmf_detect/test-config.yaml 3064 
>   /asterisk/trunk/tests/udptl/test-config.yaml 3064 
>   /asterisk/trunk/tests/udptl_v6/test-config.yaml 3064 
> 
> Diff: https://reviewboard.asterisk.org/r/1791/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Matt
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120306/1d0b1898/attachment-0001.htm>


More information about the asterisk-dev mailing list