[asterisk-dev] [Code Review]: func_jitterbuffer tests

[Matt Jordan]

> On July 23, 2012, 1:15 p.m., Mark Michelson wrote:
> > /asterisk/trunk/tests/funcs/func_jitterbuffer/configs/ast1/extensions.conf, line 13
> > <https://reviewboard.asterisk.org/r/2021/diff/2/?file=30188#file30188line13>
> >
> >     You've got a stray lowercase 'u' in "CuRRENT_TEST_TYPE". This means that default_jbs are never being set up.
> >     
> >     If the test is still passing despite this typo, I think that demonstrates a flaw in the test procedure. Specifically, there is nothing to ensure that you are receiving the test events you expect to be receiving. Receiving correct test events and receiving no test events both result in the test passing. You should modify the test to keep a counter of the test events received to ensure that a jitter buffer actually gets set up and that it sends the expected number of test events.

Shockingly, this actually works as intended:

[Jul 23 13:25:19] VERBOSE[6890][C-00000001] pbx.c:     -- Executing [s at sippeer:1] NoOp("SIP/ast1-00000001", "") in new stack
[Jul 23 13:25:19] VERBOSE[6890][C-00000001] pbx.c:     -- Executing [s at sippeer:2] GosubIf("SIP/ast1-00000001", "1?default_jb:") in new stack
[Jul 23 13:25:19] VERBOSE[6890][C-00000001] pbx.c:     -- Executing [s at sippeer:7] NoOp("SIP/ast1-00000001", "") in new stack
[Jul 23 13:25:19] VERBOSE[6890][C-00000001] pbx.c:     -- Executing [s at sippeer:8] Set("SIP/ast1-00000001", "JITTERBUFFER(adaptive)=default)") in new stack
[Jul 23 13:25:19] VERBOSE[6890][C-00000001] pbx.c:     -- Executing [s at sippeer:9] Return("SIP/ast1-00000001", "") in new stack

This is actually disconcerting, as the information on wiki.asterisk.org implies that variables are case sensitive.  Googling turns this nugget up from voip-info:

"A variable name may be any alphanumeric string beginning with a letter. User-defined variable names are not case sensitive — ${FOO} and ${Foo} refer to the same variable — but Asterisk-defined variables are case-sensitive — ${EXTEN} works, but ${exten} doesn't."

So, uhm... hooray?

When this has all the DTMF cruft in with it, a TestEvent ensured that the JitterBuffer was set up on the channel with the expected parameters.  The refactoring that had been done made it easy to put in a TestEvent with all of the JitterBuffer parameters - with that gone now, that isn't as trivial to do.  Instead, I can listen for a VarSet event to verify that some JitterBuffer has been set on the SIP channel.

This was happening, but an event should ensure that the test fails in case it ever doesn't happen.


> On July 23, 2012, 1:15 p.m., Mark Michelson wrote:
> > /asterisk/trunk/tests/funcs/func_jitterbuffer/run-test, lines 106-111
> > <https://reviewboard.asterisk.org/r/2021/diff/2/?file=30190#file30190line106>
> >
> >     I imagine this may be cruft left over from when this was testing DTMF jitterbuffers, but the TestResult user event will never have a result of "fail". You might as well just go straight into self._launch_talk_detect() when you receive a TestResult event, and you might as well just remove the result field from the event altogether.

Agreed.


- Matt


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


On July 12, 2012, 1:30 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2021/
> -----------------------------------------------------------
> 
> (Updated July 12, 2012, 1:30 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This adds a new test to the Asterisk Test Suite that covers the JITTERBUFFER function.  This includes tests for the following:
> 
> * JITTERBUFFER(type)=default
> * JITTERBUFFER(type)=max_size,resync,target_extra
> 
> The variants in each test are:
> 
> type => adaptive or fixed jitter buffers
> max_size => default, 1000, 2000
> target_extra => default, 10, 50
> resync => default, 2000, 500
> 
> For each test, the following are checked:
> 
> * The JITTERBUFFER function creates a jitter buffer with the specified properties
> * That voice are streamed properly through a jitter buffer.  This uses the Record application, allong with BackgroundDetect for the recorded voice files.
> 
> This patch also groups all function tests under a new folder, "funcs"
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/tests/funcs/func_jitterbuffer/configs/ast1/extensions.conf PRE-CREATION 
>   /asterisk/trunk/tests/funcs/func_jitterbuffer/configs/ast1/sip.conf PRE-CREATION 
>   /asterisk/trunk/tests/funcs/func_jitterbuffer/run-test PRE-CREATION 
>   /asterisk/trunk/tests/funcs/func_jitterbuffer/talking.ulaw UNKNOWN 
>   /asterisk/trunk/tests/funcs/func_jitterbuffer/test-config.yaml PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/2021/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Matt
> 
>

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