Changed the REAME.md and pass charm proof

Changed the REAME.md and pass charm proof



https://code.launchpad.net/~mbruzek/charms/precise/rabbitmq-server/tests/+merge/202573

(do not edit description out of merge proposal)

[mbruzek, 2014-01-21 23:17:31]

Please take a look.

[mbruzek, 2014-01-24 19:05:19]

Please take a look.

[Marco Ceppi, 2014-02-03 09:11:26]

Great start, see my comments below

https://codereview.appspot.com/51230045/diff/20001/tests/10_basic_deploy_test.py
File tests/10_basic_deploy_test.py (right):

https://codereview.appspot.com/51230045/diff/20001/tests/10_basic_deploy_test...
tests/10_basic_deploy_test.py:14: file_path = os.path.join(path,
'rabbit-server-privkey.pem')
You're sending SSL certs but never verify if they're actually working. It'd be
nice if this test also attempted an SSL connection to the rabbitmq server and
verify the certs were installed and setup properly

https://codereview.appspot.com/51230045/diff/20001/tests/20_deploy_relations_...
File tests/20_deploy_relations_test.py (right):

https://codereview.appspot.com/51230045/diff/20001/tests/20_deploy_relations_...
tests/20_deploy_relations_test.py:39:
d.sentry.unit['rabbitmq-server/0'].relation('ceph', 'ceph:client')
While testing the ceph relation is important, you should also test the amqp
relation, as that's the primary purpose of this charm

https://codereview.appspot.com/51230045/diff/20001/tests/30_configuration_tes...
File tests/30_configuration_test.py (right):

https://codereview.appspot.com/51230045/diff/20001/tests/30_configuration_tes...
tests/30_configuration_test.py:54: message = 'The certificate did not match!'
Simply being able to check one of the several configuration options isn't enough
for a complete test. You should get the contents of the configuration file (or
if possible, query rabbitmq directly) to verify the charm is doing the right
thing

[jamespage, 2014-02-09 09:51:43]

https://codereview.appspot.com/51230045/diff/20001/config.yaml
File config.yaml (right):

https://codereview.appspot.com/51230045/diff/20001/config.yaml#newcode16
config.yaml:16: default: ""
I'd rather not see this - "" is not the same as unset and the charm uses
config-get --format=json ssl_key and relies on return values being "None" in
python.

[mbruzek, 2014-03-04 19:35:16]

Please take a look.

[mbruzek, 2014-03-17 19:58:22]

Please take a look.

[jamespage, 2014-03-19 09:53:33]

https://codereview.appspot.com/51230045/diff/40001/hooks/rabbit_utils.py
File hooks/rabbit_utils.py (right):

https://codereview.appspot.com/51230045/diff/40001/hooks/rabbit_utils.py#newc...
hooks/rabbit_utils.py:1: #!/usr/bin/python
Why does rabbit_utils need to be executable? only the relations file should be
directly executed.

https://codereview.appspot.com/51230045/diff/60001/hooks/_pythonpath.py
File hooks/_pythonpath.py (right):

https://codereview.appspot.com/51230045/diff/60001/hooks/_pythonpath.py#newcode2
hooks/_pythonpath.py:2: 
Again - why execute?

https://codereview.appspot.com/51230045/diff/60001/hooks/rabbit_utils.py
File hooks/rabbit_utils.py (right):

https://codereview.appspot.com/51230045/diff/60001/hooks/rabbit_utils.py#newc...
hooks/rabbit_utils.py:2: 
Why does this file need to be executable? typically the only file that does is
the hooks file.

[jamespage, 2014-03-19 10:47:25]

https://codereview.appspot.com/51230045/diff/60001/tests/20_deploy_relations_...
File tests/20_deploy_relations_test.py (right):

https://codereview.appspot.com/51230045/diff/60001/tests/20_deploy_relations_...
tests/20_deploy_relations_test.py:58: d.configure('ceph', ceph_configuration)
I don't think that the ceph charm is actually going to bring up a ceph cluster
with this configuration; by default it requires 3 service units before it will
actually startup; you can either add service units OR you can set the mon-count
config to 1 to fix this.

https://codereview.appspot.com/51230045/diff/60001/tests/20_deploy_relations_...
tests/20_deploy_relations_test.py:113: print(key, value)
I think this needs more rigour; the ceph charm should have provided a key for
the rabbitmq charm to use; also where does print output go?