[Asterisk-code-review] rtp_range: Add tests to verify rtp range settings are followed. (testsuite[16])

Wed Nov 17 16:02:34 CST 2021

Attention is currently required from: Michael Bradeen.
Kevin Harwell has posted comments on this change. ( https://gerrit.asterisk.org/c/testsuite/+/16636 )

Change subject: rtp_range: Add tests to verify rtp range settings are followed.
......................................................................

Patch Set 3: Code-Review-1

(26 comments)

Patchset:

PS3: 
I think for the most part the way you are testing things is fine, and feel free to leave as is.

However, I think another option would be to reverse the calls, and have Asterisk originate them.

For the initial 3 valid cases the SIPp scenario(s) could then be simplified to only accept an invite. You could then use the same scenario file for all scenarios.

For the failure cases the 'Originate' should fail, which hopefully you can then check for the error(s), issue an UserEvent, check for at least that many user events using the 'EventActionModule' found in lib/python/asterisk/pluggable_modules.py, and then end the test.

Doing it this way should make it a little more deterministic (no waits), and also shorten execution time (again no waits).

File tests/rtp/rtp_range/rtp_range_even_even/sipp/call_accept_test_range.xml:

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/dbb9604c_5a62b359 
PS3, Line 87:   </send>    
Remove extra whitespace at eol.

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/221f2f71_a863045e 
PS3, Line 89:   <label id="1"/> 
remove extra space at end

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/ef7dcae1_5f0e7956 
PS3, Line 91:   <send next="2">  
Remove extra whitespace at eol.

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/382d7394_090f4dcc 
PS3, Line 107:    
Remove this whitespace too

File tests/rtp/rtp_range/rtp_range_even_even/test-config.yaml:

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/9011dc8e_db98c00c 
PS3, Line 2: inton
s/inton/into

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/f8249d49_5f5fe8e9 
PS3, Line 4:         This test verifies that the correct RTP Range is used and that Asterisk does not go
           :         in to an infinite loop searching for a free RTP port
I'd add a bit more in order to be more informational and separate it from the other tests. Maybe something like, "Using starting, and ending even values this test ..."

File tests/rtp/rtp_range/rtp_range_even_odd/sipp/call_accept_test_range.xml:

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/6bdd8bb4_427600a7 
PS3, Line 87:   </send>    
remove whitespace

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/605df8be_a471e303 
PS3, Line 89:   <label id="1"/> 
extra space to remove

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/61fc5e4e_127d4b19 
PS3, Line 91:   <send next="2">  
remove whitespace

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/fc60b18d_30583efa 
PS3, Line 107:    
remove whitespace

File tests/rtp/rtp_range/rtp_range_even_odd/test-config.yaml:

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/cb2b83cb_b3cef8cb 
PS3, Line 2: inton
s/inton/into

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/de3d1805_c82f9ca8 
PS3, Line 4:         This test verifies that the correct RTP Range is used and that Asterisk does not go
           :         in to an infinite loop searching for a free RTP port
I'd add a bit more in order to be more informational and separate it from the other tests. Maybe something like, "Using a starting even value, and an odd ending value this test ..."

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/f7210b09_7cfa9a97 
PS3, Line 29:         - RTP        
whitespace

File tests/rtp/rtp_range/rtp_range_odd_even/sipp/call_accept_test_range.xml:

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/e6026d4a_c8d5f0dc 
PS3, Line 87:   </send>    
whitespace

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/4eafd382_1368f48e 
PS3, Line 89:   <label id="1"/> 
whitespace

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/64046bb9_8d692db4 
PS3, Line 91:   <send next="2">  
whitespace

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/6ac1f9b5_354ced26 
PS3, Line 107:    
whitespace

File tests/rtp/rtp_range/rtp_range_odd_even/test-config.yaml:

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/47f87256_426ef633 
PS3, Line 2: inton
s/inton/into

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/7aca99b4_cd860b1a 
PS3, Line 4:         This test verifies that the correct RTP Range is used and that Asterisk does not go
           :         in to an infinite loop searching for a free RTP port
I'd add a bit more in order to be more informational and separate it from the other tests. Maybe something like, "Using a starting odd, and an even ending value this test ..."

File tests/rtp/rtp_range/rtp_range_odd_odd/sipp/call_accept_test_range.xml:

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/bbdaf48d_639dd08b 
PS3, Line 87:   </send>    
whitespace

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/bc0ffe08_cbcadb5d 
PS3, Line 89:   <label id="1"/> 
whitespace

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/f063eeea_cf7a0cac 
PS3, Line 91:   <send next="2">  
whitespace

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/da44d4ed_3893d82c 
PS3, Line 107:    
whitespace

File tests/rtp/rtp_range/rtp_range_odd_odd/test-config.yaml:

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/8fc6175c_f137c6b8 
PS3, Line 2: inton
s/inton/into

https://gerrit.asterisk.org/c/testsuite/+/16636/comment/f6914d15_8a59bcf1 
PS3, Line 4:         This test verifies that the correct RTP Range is used and that Asterisk does not go
           :         in to an infinite loop searching for a free RTP port
I'd add a bit more in order to be more informational and separate it from the other tests. Maybe something like, "Using starting, and ending odd values this test ..."

-- 
To view, visit https://gerrit.asterisk.org/c/testsuite/+/16636
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: testsuite
Gerrit-Branch: 16
Gerrit-Change-Id: Iefa8545cc1785b1e32a42d740f8f567706c73199
Gerrit-Change-Number: 16636
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Bradeen <mbradeen at sangoma.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Attention: Michael Bradeen <mbradeen at sangoma.com>
Gerrit-Comment-Date: Wed, 17 Nov 2021 22:02:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20211117/d377af1f/attachment-0001.html>