Matt Jordan has posted comments on this change. Change subject: Add a test for PJSIP t38 with authentication based on normal t38 test ......................................................................
Patch Set 1: (15 comments) https://gerrit.asterisk.org/#/c/22/1//COMMIT_MSG Commit Message: Line 7: Add a test for PJSIP t38 with authentication based on normal t38 test I'd capitalize "T38" Line 8: You need to add: 1. A full description of what the test does 2. The standard commit message tags, e.g., the ASTERISK issue related to the change 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 => Line 6: exten => h,n,UserEvent(FaxStatus,operation: send,status: ${FAXOPT(status)},statusstr: ${FAXOPT(statusstr)},error: ${FAXOPT(error)}) Use same => https://gerrit.asterisk.org/#/c/22/1/tests/fax/pjsip/t38_with_auth/configs/ast2/extensions.conf File tests/fax/pjsip/t38_with_auth/configs/ast2/extensions.conf: Line 3: exten => 1234,n,ReceiveFax(${ASTDATADIR}/receive.tiff) Use same => Line 6: exten => h,n,UserEvent(FaxStatus,operation: receive,status: ${FAXOPT(status)},statusstr: ${FAXOPT(statusstr)},error: ${FAXOPT(error)}) Use same => https://gerrit.asterisk.org/#/c/22/1/tests/fax/pjsip/t38_with_auth/run-test File tests/fax/pjsip/t38_with_auth/run-test: Line 23: logger = logging.getLogger(__name__) Our typical "practice" capitalizes LOGGER Line 25: class T38Test(TestCase): Add a pydoc comment for that this class is Line 30: TestCase.__init__(self) Use the new style mechanism for invoking the base class init method: super(T38Test, self).__init__() Line 34: # copy the tiff file we are going to send to a good known location : shutil.copy("%s/send.tiff" % (os.path.dirname(os.path.realpath(__file__)),), "%s%s" % (self.ast[0].base, self.ast[0].directories['astdatadir'])) I'm fairly sure this will break the recommended line length :-) Line 37: def ami_connect(self, ami): Add a pydoc comment Line 43: def handle_failure(reason): : logging.error("error sending originate:") : logging.error(reason.getTraceback()) : self.stop_reactor() : : return reason : : df.addErrback(handle_failure) There's a standard handler for failed Originate calls in TestCase. Use that instead of providing your own. Line 52: def fax_result(self, ami, event): pydoc comment please https://gerrit.asterisk.org/#/c/22/1/tests/fax/pjsip/t38_with_auth/test-config.yaml File tests/fax/pjsip/t38_with_auth/test-config.yaml: Line 3: description: | : "This test starts two Asterisk instances and sends a fax between them : using the PJSIP channel driver - The endpoints use userpass : authentication." I think it is worth mentioning why we needed this test. Referencing the ASTERISK issue (which there is a standard YAML tag for) would also be nice: issue: ASTERISK-xxxxx Line 9: minversion: '13.3.1' minversion should be '13.4.0'. The patch that fixes this is not going into a point release. -- 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: 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
