[Asterisk-code-review] Add local attended transfer tests with direct media, hold, a... (testsuite[master])
Samuel Galarneau
asteriskteam at digium.com
Fri May 22 11:22:23 CDT 2015
Samuel Galarneau has posted comments on this change.
Change subject: Add local attended transfer tests with direct media, hold, and REFER/Replaces.
......................................................................
Patch Set 3: Code-Review-1
(4 comments)
https://gerrit.asterisk.org/#/c/483/3/tests/channels/pjsip/transfers/attended_transfer/nominal/packet_sniffer.py
File tests/channels/pjsip/transfers/attended_transfer/nominal/packet_sniffer.py:
Line 76: pcap_defaults = {'device': 'lo', 'snaplen': 2000,
: 'bpf-filter': 'udp port 5060', 'debug-packets': True,
: 'buffer-size': 4194304, 'register-observer': True}
: for name, value in pcap_defaults.items():
: module_config[name] = module_config.get(name, value)
Very nice!
Line 195: location = self.find_call(callid)
: if location is not None:
: return self.calls[location[0]][location[1]][callid]
Although this will in fact return None in the event that location is None, I would add a variable result = None and return that at the end of the method for clarity. You can set result = self.calls... if location is not None.
Line 259: self.get_call_state(callid) == "CONNECTING"):
Since this starts a block, self.get_call_state... should be indented by 4 more spaces to make it easier to see that the next line starts the block and isn't part of the conditional.
Line 284: .format(reinvited_endpoints))
PEP8 wants this aligned with "Endpoints...
--
To view, visit https://gerrit.asterisk.org/483
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd1f9a3ea8bf6b80fe95efe3fcd880a55d5a7cae
Gerrit-PatchSet: 3
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: John Bigelow <jbigelow at digium.com>
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: John Bigelow <jbigelow at digium.com>
Gerrit-Reviewer: Samuel Galarneau <sgalarneau at digium.com>
Gerrit-HasComments: Yes
More information about the asterisk-code-review
mailing list