[Asterisk-code-review] Fix failing tcpauthlimit TCP scenario test. (testsuite[master])

Mark Michelson asteriskteam at digium.com
Tue Feb 16 09:38:50 CST 2016


Hello Anonymous Coward #1000019, Joshua Colp,

I'd like you to reexamine a change.  Please visit

    https://gerrit.asterisk.org/2253

to look at the new patch set (#3).

Change subject: Fix failing tcpauthlimit TCP scenario test.
......................................................................

Fix failing tcpauthlimit TCP scenario test.

The existing tcpauthlimit test was bouncing in CI test runs. The test
created TCP connections to Asterisk. It would then attempt to write
data to the socket and read data from the socket. If the read attempts
returned the EWOULDBLOCK error, then the connection was marked as closed
by the remote peer. If the read attempts succeeded, then the connection
was marked as connected.

The problem with this is that it does not accurately reflect what is
happening under the hood. Getting EWOULDBLOCK does not indicate that the
connection is closed; it just means that the connection never had data
to read when we checked. Further, not getting EWOULDBLOCK did not
necessarily indicate that the connection was up, since a common way that
TCP indicates that the remote peer closed the connection is to have the
socket ready for reading but return 0 bytes of data when attempting to
read. As such, I had local test runs where the test run assumed that
certain connections to Asterisk were up, but I could see from some added
debug in Asterisk that those connections were ones that Asterisk had
closed. I saw the reverse scenario as well.

The existing test was a bit complex for what it was trying to do, so
rather than trying to patch the existing test, I have opted to rewrite
it. The new approach is to use the twisted.protocol's connectionLost()
method to detect when the connection is lost. The scenario is also
simplified to use fewer connections, so it is easy to track what is
happening during a test.

Change-Id: Ie776e62288bde454754a35840aa870e07a452555
---
D tests/channels/SIP/tcpauthlimit/tcp_client_scenario/configs/ast1/extensions.conf
M tests/channels/SIP/tcpauthlimit/tcp_client_scenario/configs/ast1/sip.conf
A tests/channels/SIP/tcpauthlimit/tcp_client_scenario/tcp_scenario.py
M tests/channels/SIP/tcpauthlimit/tcp_client_scenario/test-config.yaml
D tests/channels/SIP/tcpauthlimit/tcp_scenario.py
5 files changed, 73 insertions(+), 773 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/testsuite refs/changes/53/2253/3
-- 
To view, visit https://gerrit.asterisk.org/2253
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie776e62288bde454754a35840aa870e07a452555
Gerrit-PatchSet: 3
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>



More information about the asterisk-code-review mailing list