[Asterisk-code-review] Add options to send crash reports via email (testsuite[master])

[Ashley Sanders]

Ashley Sanders has posted comments on this change.

Change subject: Add options to send crash reports via email
......................................................................


Patch Set 2: Code-Review-1

(4 comments)

https://gerrit.asterisk.org/#/c/1539/2/runtests.py
File runtests.py:

Line 188:         try:
        :             f = open("crash-mail-config.yaml", "r")
        :         except IOError:
        :             print "Failed to open crash-mail-config.yaml"
        :             return -1
        :         except:
        :             print "Unexpected error: %s" % sys.exc_info()[0]
        :             return -1
        : 
        :         email_config = yaml.load(f)
        :         f.close()
I am surprised this pattern hasn't been abstracted into its own method or function already.


Line 203: ""
It seems like to me that this should not default to an empty string. 

It would probably make more sense to default the subject to what you stated in your sample config: Testsuite crash detected.


Line 219: subject = "Subject: {0}\n\n".format(subject)
Is there any easy way to include the name or category of the test that generated the crash? (e.g. "Testsuite crash detected in: pjsip->subscriptions")


Line 224:         try:
        :             import smtplib
        :         except ImportError:
        :             print "Failed to import smtplib"
        :             return -1
        : 
        :         try:
        :             smtpObj = smtplib.SMTP(email_server)
        :             smtpObj.set_debuglevel(email_debug)
        :             smtpObj.sendmail(sender, recipients, email_text)
        :         except smtplib.SMTPServerDisconnected:
        :             print "Failed to connect to SMTP Server"
        :             return -1
        :         except smtplib.SMTPException as smtp_exception:
        :             print "Failed to send crash report due to unhandled SMTPException"
        :             raise(smtp_exception)
This could be abstracted into it's own method (or function). e.g. def send_email(server, debug_level, sender, recipients, body)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I17def41f15294a48754269d9c42aa649389840ad
Gerrit-PatchSet: 2
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Jonathan Rose <jrose at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Jonathan Rose <jrose at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: Yes