[asterisk-dev] Change in testsuite[master]: Add a test for PJSIP t38 with authentication based on normal...

[Ashley Sanders (Code Review)]

Ashley Sanders has posted comments on this change.

Change subject: Add a test for PJSIP t38 with authentication based on normal t38 test
......................................................................


Patch Set 1: Code-Review-1

(5 comments)

Very close. I think just fix the pylint errors and the little tweaks that Matt suggested and it will be good.

https://gerrit.asterisk.org/#/c/22/1/tests/fax/pjsip/t38_with_auth/configs/ast1/extensions.conf
File tests/fax/pjsip/t38_with_auth/configs/ast1/extensions.conf:

Line 3: exten => 1234,n,SendFax(${ASTDATADIR}/send.tiff)
> Use same =>
@Matt Jordan, is SendFax similar to Stasis in that once the channel is handed off to the application, it only comes back on a hangup?


https://gerrit.asterisk.org/#/c/22/1/tests/fax/pjsip/t38_with_auth/run-test
File tests/fax/pjsip/t38_with_auth/run-test:

I think you need to run pylint on this file. Right now, it is scoring a -6.86/10. 

On the plus side, it seems like it is mostly indentation errors and missing docstrings, though :)


Line 6: 
Should you add your name to the authors list?


Line 26:    event_count = 0
If you aren't using this class as a singleton, you don't need to have these variables at the class level. They would be safe to be instance variables.


Line 30:       TestCase.__init__(self)
> Use the new style mechanism for invoking the base class init method:
Interesting. When I run pylint on my files using super(FooTestCase, self).__init__(), pylint returns, "Use of super on an old-style class." 

But, after thinking about it now, TestCase is not an old-style class. I am going to have to go back and revisit my test and fix those references, too (and ignore pylint's barks).


-- 
To view, visit https://gerrit.asterisk.org/22
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id8fd9683dc1b61e7b1afd2b6ede857921beebb88
Gerrit-PatchSet: 1
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Jonathan Rose <jrose at digium.com>
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: Yes