Code review comment for lp:~bcsaller/pyjuju/subordinate-ports

Revision history for this message
Benjamin Saller (bcsaller) wrote :

https://codereview.appspot.com/6324045/diff/1/juju/hooks/tests/test_invoker.py
File juju/hooks/tests/test_invoker.py (right):

https://codereview.appspot.com/6324045/diff/1/juju/hooks/tests/test_invoker.py#newcode1519
juju/hooks/tests/test_invoker.py:1519: (yield
unit_state.get_open_ports()),
On 2012/07/01 02:32:22, hazmat wrote:
> This test never verifies the implementation, namely that the container
ports are
> open.

Right you are, thanks

https://codereview.appspot.com/6324045/diff/1/juju/state/tests/test_firewall.py
File juju/state/tests/test_firewall.py (right):

https://codereview.appspot.com/6324045/diff/1/juju/state/tests/test_firewall.py#newcode192
juju/state/tests/test_firewall.py:192: """
fair enough, removed

On 2012/07/01 02:32:22, hazmat wrote:
> this test also seems superfluous. the only impl file being modified by
the
> branch is the invoker, yet we have tests in numerous state test files
verifying
> what has already been verified by other tests in those files

https://codereview.appspot.com/6324045/diff/1/juju/state/tests/test_service.py
File juju/state/tests/test_service.py (right):

https://codereview.appspot.com/6324045/diff/1/juju/state/tests/test_service.py#newcode2810
juju/state/tests/test_service.py:2810: self.assertEqual(
On 2012/07/01 02:32:22, hazmat wrote:
> how is this entire test not entirely superfluous?

removed

https://codereview.appspot.com/6324045/

« Back to merge proposal