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

Ship it!


Good changes.

A few minor nits below, but good to go.


/asterisk/trunk/run-local
<https://reviewboard.asterisk.org/r/3397/#comment22090>

    /usr/local/bin and /usr/sbin?
    
    What kind of combination is that?
    
    Should this just be all permutations of
    
      /usr(/local)?/s?bin
    
    instead?



/asterisk/trunk/tests/apps/chanspy/chanspy_barge/run-test
<https://reviewboard.asterisk.org/r/3397/#comment22091>

      os.getcwd() is always a bad idea
    
    Please replace with path relative to this file:
    
      os.path.join(os.path.dirname(__file__),
                   "sounds/talking")
    
    Unless someone is actively os.chdir()ing around to make things work,
    but my grep did not detect any such foolishness.



/asterisk/trunk/tests/apps/chanspy/chanspy_w_mixmonitor/run-test
<https://reviewboard.asterisk.org/r/3397/#comment22092>

    Another os.getcwd() issue.



/asterisk/trunk/tests/channels/SIP/sip_tls_call/run-test
<https://reviewboard.asterisk.org/r/3397/#comment22093>

    These rely on an implicit CWD which is less bad, so I won't make you change 
these too ;)
    
    



/asterisk/trunk/tests/channels/SIP/sip_tls_register/run-test
<https://reviewboard.asterisk.org/r/3397/#comment22094>

    I prefer outdenting this brace, but I don't think there is any concensus 
about that.


- wdoekes


On June 6, 2014, 1:43 p.m., Corey Farrell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3397/
> -----------------------------------------------------------
> 
> (Updated June 6, 2014, 1:43 p.m.)
> 
> 
> Review request for Asterisk Developers and wdoekes.
> 
> 
> Bugs: ASTERISK-23538
>     https://issues.asterisk.org/jira/browse/ASTERISK-23538
> 
> 
> Repository: testsuite
> 
> 
> Description
> -------
> 
> run-local: install asttest within astroot directory structure, fix the path 
> statement (local install of asttest was never in $PATH before).
> lib/python/asterisk.py: add get_path
> chanspy_barge, chanspy_w_mixmonitor: use astspooldir/tmp to write the 
> testaudio1
> sip_tls_call & sip_tls_register: use key files from original location instead 
> of copying them
> fax/sip/local_channel_t38_queryoption, udptl, udptl_v6: store file from 
> ReceiveFax in ${ASTDATADIR}
> 
> asterisk.get_path is designed to be used in place of manual path construction 
> involving asterisk._base, and to encourage use of os.path.join style 
> construction.
> 
> Though reviewboard isn't showing it, this patch adds 'astroot' to svn:ignore 
> of the testsuite root.
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/tests/udptl_v6/configs/ast2/extensions.conf 5094 
>   /asterisk/trunk/tests/udptl/configs/ast2/extensions.conf 5094 
>   
> /asterisk/trunk/tests/fax/sip/local_channel_t38_queryoption/configs/ast2/extensions.conf
>  5094 
>   /asterisk/trunk/tests/channels/SIP/sip_tls_register/run-test 5094 
>   /asterisk/trunk/tests/channels/SIP/sip_tls_call/run-test 5094 
>   /asterisk/trunk/tests/channels/SIP/sip_tls_call/configs/helper2 5094 
>   /asterisk/trunk/tests/channels/SIP/sip_tls_call/configs/helper1 5094 
>   /asterisk/trunk/tests/apps/chanspy/chanspy_w_mixmonitor/run-test 5094 
>   /asterisk/trunk/tests/apps/chanspy/chanspy_barge/run-test 5094 
>   /asterisk/trunk/run-local 5094 
>   /asterisk/trunk/lib/python/asterisk/asterisk.py 5094 
> 
> Diff: https://reviewboard.asterisk.org/r/3397/diff/
> 
> 
> Testing
> -------
> 
> Verified all modified tests still using work with run-local, and no longer 
> create any untracked files in svn.  I believe these changes to be correct for 
> use without run-local, but can't test that.
> 
> 
> Thanks,
> 
> Corey Farrell
> 
>

-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Reply via email to