Code review comment for lp:~hloeung/charms/precise/apache2/ssl-security-options

Revision history for this message
Adam Israel (aisrael) wrote :

Hi Haw,

Thanks for your work addressing the requested changes! I had the opportunity to retest this merge proposal today. The updates to the README and Makefile look great. I did run into one issue running the tests, unfortunately.

Both test_is_selfsigned_cert_stale_private_address_changed and test_is_selfsigned_cert_stale_public_address_changed failed, running under the local environment:

======================================================================
FAIL: tests.test_cert.CertTests.test_is_selfsigned_cert_stale_private_address_changed
----------------------------------------------------------------------
_StringException: Traceback (most recent call last):
  File "/home/stone/charms/precise/apache2/.venv/local/lib/python2.7/site-packages/mock.py", line 1201, in patched
    return func(*args, **keywargs)
  File "/home/stone/charms/precise/apache2/hooks/tests/test_cert.py", line 95, in test_is_selfsigned_cert_stale_private_address_changed
    hooks.is_selfsigned_cert_stale(config, cert_file, key_file))
  File "/home/stone/charms/precise/apache2/.venv/local/lib/python2.7/site-packages/unittest2/case.py", line 678, in assertTrue
    raise self.failureException(msg)
AssertionError: False is not true

======================================================================
FAIL: tests.test_cert.CertTests.test_is_selfsigned_cert_stale_public_address_changed
----------------------------------------------------------------------
_StringException: Traceback (most recent call last):
  File "/home/stone/charms/precise/apache2/.venv/local/lib/python2.7/site-packages/mock.py", line 1201, in patched
    return func(*args, **keywargs)
  File "/home/stone/charms/precise/apache2/hooks/tests/test_cert.py", line 82, in test_is_selfsigned_cert_stale_public_address_changed
    hooks.is_selfsigned_cert_stale(config, cert_file, key_file))
  File "/home/stone/charms/precise/apache2/.venv/local/lib/python2.7/site-packages/unittest2/case.py", line 678, in assertTrue
    raise self.failureException(msg)
AssertionError: False is not true

Thanks again for all your hard work on this! I'm looking forward to seeing these changes promulgated.

review: Needs Fixing

« Back to merge proposal