[Asterisk-code-review] StatsD: Create a mock StatsD server in the Testsuite (testsuite[master])

[Ashley Sanders]

Ashley Sanders has posted comments on this change.

Change subject: StatsD: Create a mock StatsD server in the Testsuite
......................................................................


Patch Set 3: Code-Review-1

(2 comments)

https://gerrit.asterisk.org/#/c/1313/3/tests/apps/statsd/nominal/mockd.py
File tests/apps/statsd/nominal/mockd.py:

Line 68:         self.message = message
> Don't assign message to a member variable.
I agree. You only use self.message within the scope of this method. When you remove self.message, you will also need to adjust line 69 below, like:


    self.packets.append(message)


Line 80:         is_correct = True
I feel like you should put a logger statement here saying something to the effect that you are about to check the state of the test. 

Logger statements at the beginning of a method (especially a method that determines whether or not a test was successful) are incredibly helpful to someone who has to come behind you and piece together the logic, which will probably be when something is broken. When you are writing your tests, keep in mind the things you would want to see if you were looking at this for the first time and needed to figure out the intent of a particular piece of code.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4eaf92465b79569fff4db6ada16e8d215f7d214b
Gerrit-PatchSet: 3
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: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: Yes