[asterisk-dev] [Code Review] 3132: Test for allow=all sdp issue

[Scott Griepentrog]

> On Jan. 20, 2014, 9:08 p.m., Matt Jordan wrote:
> > /asterisk/trunk/tests/channels/pjsip/allow_all_sdp/test-config.yaml, lines 16-29
> > <https://reviewboard.asterisk.org/r/3132/diff/1/?file=52955#file52955line16>
> >
> >     I'd prefer not to have all of these tests.
> >     
> >     1. Most of what these tests cover - early media, deferred SDP, various transports - is already covered. What we need to test here is that configuring and endpoint with "allow=all" results in the correct outbound offer - the rest of this behaviour is already tested.
> >     
> >     2. This test already has lots of problems running under Bamboo. It fails probably 25% of the time. We shouldn't have put this many SIPp scenarios under a single test; I'm guessing that this test will suffer from the same problem.

I would have agreed, except that I caught an issue with deferred sdp that I wouldn't have otherwise.  Maybe it would be better to break them out into smaller groups of tests, such as deferred/ipv4, deferred/ipv6, etc - which would also help identify which ones are being problematic.


> On Jan. 20, 2014, 9:08 p.m., Matt Jordan wrote:
> > /asterisk/trunk/tests/channels/pjsip/tests.yaml, line 16
> > <https://reviewboard.asterisk.org/r/3132/diff/1/?file=52956#file52956line16>
> >
> >     Thinking for the future:
> >     
> >     This is really a test for Offer/Answer - if we configure an endpoint with all, do we offer the correct thing?
> >     
> >     I'd put this into something like the following:
> >     
> >     sdp_offer_answer/nominal/outbound/offer_all
> >     
> >     We will, at some point, have off nominal offers sent to Asterisk, and have Asterisk deal with offers that contain all (or a lot) of formats. With Josh's format work, this is coming up sooner rather than later.

Actually, this particular test is checking inbound, not outbound.  But I do think that outbound testing should be done as well, and I agree it should be broken out into separate categories as you show.

Do you want this set up as a new subdirectory pjsip/sdp_offer_answer instead of merging it into pjsip/basic_calls?


- Scott


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


On Jan. 16, 2014, 4:13 p.m., Scott Griepentrog wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3132/
> -----------------------------------------------------------
> 
> (Updated Jan. 16, 2014, 4:13 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23018
>     https://issues.asterisk.org/jira/browse/ASTERISK-23018
> 
> 
> Repository: testsuite
> 
> 
> Description
> -------
> 
> This is just a copy of tests/channels/pjsip/basic_calls/incoming/nominal/unauthed/ident_by_host with allow=all instead of allow=ulaw.
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/tests/channels/pjsip/tests.yaml 4563 
>   /asterisk/trunk/tests/channels/pjsip/allow_all_sdp/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/allow_all_sdp/sipp/playback_with_initial_sdp.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/allow_all_sdp/sipp/playback_with_deferred_sdp.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/allow_all_sdp/sipp/echo_with_initial_sdp.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/allow_all_sdp/sipp/echo_with_deferred_sdp.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/allow_all_sdp/configs/ast1/pjsip.conf PRE-CREATION 
>   /asterisk/trunk/tests/channels/pjsip/allow_all_sdp/configs/ast1/extensions.conf PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/3132/diff/
> 
> 
> Testing
> -------
> 
> It correctly fails without the 3131 code implemented.
> 
> 
> Thanks,
> 
> Scott Griepentrog
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140121/6640f7fb/attachment.html>