[asterisk-dev] [Code Review]: Allow running test suite locally - initial work

[wdoekes]

> On March 21, 2013, 4:52 a.m., wdoekes wrote:
> > /asterisk/trunk/lib/python/asterisk/asterisk.py, lines 166-172
> > <https://reviewboard.asterisk.org/r/2371/diff/2/?file=34068#file34068line166>
> >
> >     This is hard to read and wrong.
> >     (1) AST_TEST_ROOT was always ''
> >     (2) test_suite_root is now always /tmp instead of /tmp/ast_test_XXX/tmp.
> >     
> >     As for the asterisk_etc_directory. I'm not sure why it has to be specified. We're guessing the location below anyway..
> >     
> >     I'd rather remove the ``asterisk_etc_directory`` setting here and set it to os.path.dirname(c) after the guess.

Still hard to read. Still broken. The (non run-local) tests now end up in "/tmp" instead of in "/tmp/asterisk-testsuite".


> On March 21, 2013, 4:52 a.m., wdoekes wrote:
> > /asterisk/trunk/run_local, line 26
> > <https://reviewboard.asterisk.org/r/2371/diff/2/?file=34069#file34069line26>
> >
> >     You're forcing me into naming the testsuite /testsuite/, while it doesn't need to.
> >     
> >     Wouldn't this be nicer?
> >     
> >       ASTROOT=~/astroots/1.8.x ./run_local setup 
> >       ASTROOT=~/astroots/1.8.x ./run_local run
> >     
> >     Then we wouldn't need a random tmpname, but could have /tmp/ast_test_1.8.x instead.
> 
> Tzafrir Cohen wrote:
>     Changing run_local to run-local.
>     
>     As for the other names: I simply decided not to change them. The patch tries to keep everything in tact if run-local is not used. The test suite assumes it is run in the directory under asterisk. I'm not sure why, exactly. I figure I can easily disable that test, but I'm not sure what this breaks.
>     
>     I do find this assumption odd. I suppose the test suite should either be part of the source tree or be completely independent of it. I personally prefer the former.

Well. Instead of the dependency that the tests must be in the source dir, they must now also have a certain name ("testsuite"). That's making things worse, not better.

The only place you're hardcoding /testsuite/ is in run-local. You can easily replace that with the "current" path with a few of basename/dirname calls.


> On March 21, 2013, 4:52 a.m., wdoekes wrote:
> > /asterisk/trunk/run_local, line 50
> > <https://reviewboard.asterisk.org/r/2371/diff/2/?file=34069#file34069line50>
> >
> >     This is empty. mktemp_symlink doesn't echo anything. So the RM below does nothing.
> >     
> >     And it wouldn't be reached if ./runtests.py returned false.
> >     
> >     And we don't like that things are removed. The original tests keep the structure around so we can asterisk -C in a testdir after running it.
> 
> Tzafrir Cohen wrote:
>     I noticed the missing echo and it's now fixed.
>     
>     As for deleting it: I should not flood /tmp with such files. If you need to test anything, you can always recreate the symlink. Real content is not deleted.

You're right.

(a) That means that you're using a tmp-dir in the build (root) dir. That's fine. As long as the caller realises that there are now temp files written to somewhere other than /tmp.

(b) But then the only reason why (a symlink in) /tmp is used at all, is to fix the AF_UNIX length. That should be documented near the symlink action.

And the set -e still causes the symlink to not get removed if runtests.py returns nonzero. Which it does it a test returns failure.


- wdoekes


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


On March 23, 2013, 9:23 a.m., Tzafrir Cohen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2371/
> -----------------------------------------------------------
> 
> (Updated March 23, 2013, 9:23 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Place anything needed for the test suite (except the actual sources of Asterisk) under the test suite directory.
> 
> Fix parts in the python code. Add a wrapper script (run_local) for the required automation.
> 
> Work is still needed on asttest (add a destdir option? Consolidate all the run-test scripts to run it through a common function?) but I'm not exactly sure how the information is passed to Lua.
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/tests/channels/SIP/sip_channel_params/run-test 3678 
>   /asterisk/trunk/tests/channels/SIP/tcpauthlimit/run-test 3678 
>   /asterisk/trunk/tests/channels/SIP/tcpauthtimeout/run-test 3678 
>   /asterisk/trunk/tests/fax/local_channel_t38_queryoption/run-test 3678 
>   /asterisk/trunk/tests/cdr/blind-transfer-accountcode/run-test 3678 
>   /asterisk/trunk/tests/cdr/originate-cdr-disposition/run-test 3678 
>   /asterisk/trunk/tests/channels/SIP/handle_response_refer/run-test 3678 
>   /asterisk/trunk/tests/channels/SIP/rfc2833_dtmf_detect/run-test 3678 
>   /asterisk/trunk/run-local PRE-CREATION 
>   /asterisk/trunk/tests/apps/queues/macro_gosub_test/run-test 3678 
>   /asterisk/trunk/tests/apps/queues/position_priority_maxlen/run-test 3678 
>   /asterisk/trunk/tests/apps/queues/ringinuse_and_pause/run-test 3678 
>   /asterisk/trunk/tests/apps/queues/wrapup_time/run-test 3678 
>   /asterisk/trunk/tests/cdr/app_dial_G_flag/run-test 3678 
>   /asterisk/trunk/tests/cdr/app_queue/run-test 3678 
>   /asterisk/trunk/README.txt 3678 
>   /asterisk/trunk/lib/python/asterisk/TestCase.py 3678 
>   /asterisk/trunk/lib/python/asterisk/asterisk.py 3678 
>   /asterisk/trunk/tests/manager/action-events-response/run-test 3678 
>   /asterisk/trunk/tests/manager/authlimit/run-test 3678 
>   /asterisk/trunk/tests/manager/authtimeout/run-test 3678 
>   /asterisk/trunk/tests/manager/response-time/run-test 3678 
> 
> Diff: https://reviewboard.asterisk.org/r/2371/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Tzafrir
> 
>

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