AMP callRemoteString ignores requiresAnswer

[ivank]

BoxDispatcher.callRemoteString has a requiresAnswer argument (and documentation), but it is not passed to return self._sendBoxCommand(command, box). There are also no tests for the case where requiresAnswer=False.

[exarkun]

Fixing description markup.

[jesstess]

The patch adds requiresAnswer to the call to _sendBoxCommand and adds a test case in twisted/test/test_amp.py that callRemoteString returns None if requiresAnswer is False.

Something else came up while looking into this ticket:

It looks like callRemoteString creates a new AmpBox that it passes to _sendBoxCommand, and when you make requiresAnswer False, in _sendBoxCommand only a _command tag gets added to this new box before calling box._sendTo(self.boxSender).

However, the AmpBox._sendTo docstring states that exactly one of _ask, _answer, and _error is a required key.

Is the docstring incorrect, or is the path that returnFalse forces bad? Other tests in test_amp.py, including test_requiresNoAnswer and test_requiresNoAnswerFail, produce AmpBoxes without one of the required keys (ostensibly _ask should be set, as _command is required if _ask is present).

print statements from AmpBox._sendBox while running the tests:

test_callRemoteStringRequiresAnswerFalse ... AmpBox({'_command': 'WTF'})

[OK]

test_requiresNoAnswer ... AmpBox({'hello': 'world', '_command': 'hello'})

[OK]

test_requiresNoAnswerFail ... AmpBox({'hello': 'fuck you', '_command': 'hello'})

[OK]

[therve]

Nice patch! Just one comment: please use assertIdentical to compare to None.

Please apply then.

[jesstess]

(In [27346]) Apply amp_callRemoteString_requiresAnwerFalse.patch

Author: jesstess Reviewer: therve Fixes: #3999

Have BoxDispatcher.callRemoteString pass its requiresAnswer argument to its call to _sendBoxCommand. Also add a unit test for requiresAnswer = False.