[Asterisk-code-review] StatsD: Write Testsuite tests (testsuite[master])

[Kevin Harwell]

Kevin Harwell has posted comments on this change.

Change subject: StatsD: Write Testsuite tests
......................................................................


Patch Set 8: Code-Review-1

(3 comments)

13.6.0 has been released, so these tests will need to be bumped to 13.7.0.

Also an overall suggestion - the reactor timeouts for each test have been set to '10'. That seems a bit low (for instance testing on a slower machine might trip this). I'd suggest bumping that up a bit.

https://gerrit.asterisk.org/#/c/1392/8/tests/apps/statsd/mockd.py
File tests/apps/statsd/mockd.py:

Line 85:         if self.config[0] == 'ReceiveNothing':
       :             self.test_object.set_passed(False)
       :             return
If this is a failure does it make sense to set a fail token and go ahead and stop the test here?


Line 93:     def _stop_handler(self, result):
Given a chain of callbacks in a deferred, twisted will pass the result of one callback to the next callback in the sequence. Since some callbacks may expect a result object to be available it is a good idea to return it (or something that is guaranteed to not crash the next callback in the chain) from your callback.


Line 101:         if (self.config[0] == 'ReceiveNothing') and (len(self.packets) == 0):
        :             LOGGER.info('Server correctly received nothing')
        :         else:
Just a suggestion, but I would add a return to the "if", remove the else, thus decreasing indention by one.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id93dbeea53cf978461151e9002af2afe1029ce8b
Gerrit-PatchSet: 8
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Tyler Cambron <tcambron at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Tyler Cambron <tcambron at digium.com>
Gerrit-HasComments: Yes