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 <[email protected]> Gerrit-Reviewer: Ashley Sanders <[email protected]> Gerrit-Reviewer: Matt Jordan <[email protected]> Gerrit-HasComments: Yes -- _____________________________________________________________________ -- 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
