[asterisk-dev] [Code Review] SDP Media Attribute Test

[Mark Michelson]

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


My main beef with this test is that with the changes you made in response to Paul's review, the test appears as though it is impossible to fail. You set the passed variable to True in SDPPassthrough.__init__() and then there is never a point afterwards where it can be set to False.

Even if you were to change the failure conditions back to checking whether sipp_a.passed and sipp_b.passed are true, this is not enough. The reason is that you could run, say, the H.263 test and encounter a failure, but then you could run the H.264 test and its passing would overwrite the failure of the previous test. The test would end up passing even though one of the test calls failed.

Of course, I could be wrong about this statement if the test immediately bombs out when a SIPp scenario fails.

- Mark


On July 10, 2012, 2:30 p.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2029/
> -----------------------------------------------------------
> 
> (Updated July 10, 2012, 2:30 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This test ensures that attribute information is transported through Asterisk.
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/tests/channels/SIP/SDP_attribute_passthrough/configs/ast1/extensions.conf PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/SDP_attribute_passthrough/configs/ast1/sip.conf PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/SDP_attribute_passthrough/run-test PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/SDP_attribute_passthrough/sipp/phone_A_h263.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/SDP_attribute_passthrough/sipp/phone_A_h264.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/SDP_attribute_passthrough/sipp/phone_A_speex.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/SDP_attribute_passthrough/sipp/phone_B_h263.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/SDP_attribute_passthrough/sipp/phone_B_h264.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/SDP_attribute_passthrough/sipp/phone_B_speex.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/SDP_attribute_passthrough/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/tests.yaml 3307 
> 
> Diff: https://reviewboard.asterisk.org/r/2029/diff
> 
> 
> Testing
> -------
> 
> Ran it, checked to make sure it worked okay. Tweaked test to fail and made sure it failed as expected.
> 
> 
> Thanks,
> 
> Joshua
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120710/856ba1f0/attachment.htm>