----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4442/#review14531 -----------------------------------------------------------
Ship it! The only findings I have are very minor ones that don't actually impact test performance or success. I figure these can be corrected when committing the changes. ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_no_direction.xml <https://reviewboard.asterisk.org/r/4442/#comment25086> I suggest removing these three headers. Session timers aren't a requirement for this test, AFAICT. ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_no_direction.xml <https://reviewboard.asterisk.org/r/4442/#comment25087> For the same reason as before, I suggest removing these headers. ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_no_direction.xml <https://reviewboard.asterisk.org/r/4442/#comment25089> I suggest removing these headers. ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_no_direction.xml <https://reviewboard.asterisk.org/r/4442/#comment25088> Pedantic nitpick: The origin and session lines in previous SDPs being sent from this script had "LiveOps" for the session name and "LiveOps" for the origin username. Asterisk currently apparently doesn't actually care about this change, but it's not a good thing to do mid-session. - Mark Michelson On Feb. 24, 2015, 6:47 p.m., Ashley Sanders wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviewboard.asterisk.org/r/4442/ > ----------------------------------------------------------- > > (Updated Feb. 24, 2015, 6:47 p.m.) > > > Review request for Asterisk Developers. > > > Bugs: ASTERISK-24824 > https://issues.asterisk.org/jira/browse/ASTERISK-24824 > > > Repository: testsuite > > > Description > ------- > > This test is to ensure that Asterisk correctly applies the direction of the > media stream when a=<sendonly|recvonly|inactive|sendrecv> is missing from the > offer's SDP. The expected behavior is for Asterisk to apply "sendrecv" as the > direction of the media stream when no direction attribute is present in an > offer's SDP. According to RFC 4566 (Section 6. SDP Attributes): "If none of > the attributes "sendonly", "recvonly", "inactive", and "sendrecv" is present, > "sendrecv" SHOULD be assumed as the default for sessions that are not of the > conference type "broadcast" or "H332" [...]" > > The test scenario: > > 1. From Phone A, send an offer to Phone B to establish a call > 2. From Phone B, send an offer to Phone A to put the call on hold. > 3. Observe that the MOH start event occurs. > 4. From Phone B, send an offer to Phone A to 'un-hold' the call (ensure that > the direction attribute from the offer's SDP is omitted) > 5. Observe that the MOH stop event occurs. > > Presently, this test fails for certain versions of Asterisk. From what I can > tell, it is present from (at least) 1.8.21 up to the 11 branch. > > ***Note*** This is the test. It is only the test. The update to the Asterisk > source is coming soon to a review board near you (well, this review board). > > > Diffs > ----- > > ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_no_direction.xml > PRE-CREATION > > ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_media_restrict.xml > 6458 > ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_IP_restrict.xml > 6458 > > ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_B_IP_media_restrict.xml > 6458 > ./asterisk/trunk/tests/channels/SIP/sip_hold/sipp/phone_A_no_direction.xml > PRE-CREATION > ./asterisk/trunk/tests/channels/SIP/sip_hold/run-test 6458 > > Diff: https://reviewboard.asterisk.org/r/4442/diff/ > > > Testing > ------- > > > Thanks, > > Ashley Sanders > >
-- _____________________________________________________________________ -- 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
