wordpress-charmers team mailing list archive

[Stuart Bishop <stuart.bishop@xxxxxxxxxxxxx>]

Review: Approve



Diff comments:

> diff --git a/tests/unit/test_wordpress.py b/tests/unit/test_wordpress.py
> index 2d4fe25..3558d38 100644
> --- a/tests/unit/test_wordpress.py
> +++ b/tests/unit/test_wordpress.py
> @@ -28,6 +30,19 @@ TEST_MODEL_CONFIG = {
>  }
>  
>  
> +TEST_GENERATED_PASSWORD = "realsecure"
> +
> +
> +def dummy_password_generator():
> +    return TEST_GENERATED_PASSWORD
> +
> +
> +class RequestsResult:
> +
> +    status_code = 0
> +    headers = {}

You should initialize headers inside an __init__, so each instance gets a unique dict. Otherwise you risk a test mutating the class global headers dict.

> +
> +
>  class HelperTest(unittest.TestCase):
>  
>      test_model_config = TEST_MODEL_CONFIG
> @@ -62,3 +77,109 @@ class HelperTest(unittest.TestCase):
>          result = charm.generate_pod_config(self.test_model_config)
>          test_container_config = yaml.safe_load(self.test_model_config["container_config"])
>          self.assertEqual(test_container_config["test-key"], result["test-key"])
> +
> +
> +class WordpressTest(unittest.TestCase):
> +
> +    test_model_config = TEST_MODEL_CONFIG
> +
> +    def setUp(self):
> +        self.test_wordpress = wordpress.Wordpress(copy.deepcopy(self.test_model_config))
> +        self.test_service_ip = "1.1.1.1"
> +
> +    def test__init__(self):
> +        self.assertEqual(self.test_wordpress.model_config, self.test_model_config)
> +
> +    @mock.patch("wordpress.password_generator", side_effect=dummy_password_generator)
> +    def test_first_install(self, password_generator_func):
> +        mocked_call_wordpress = mock.MagicMock(name="call_wordpress", return_value=True)
> +        mocked__write_initial_password = mock.MagicMock(name="_write_initial_password", return_value=None)
> +        mocked_wordpress_configured = mock.MagicMock(name="wordpress_configured", return_value=True)
> +        self.test_wordpress.call_wordpress = mocked_call_wordpress
> +        self.test_wordpress._write_initial_password = mocked__write_initial_password
> +        self.test_wordpress.wordpress_configured = mocked_wordpress_configured
> +
> +        test_payload = {
> +            'admin_password': TEST_GENERATED_PASSWORD,
> +            'admin_password2': TEST_GENERATED_PASSWORD,
> +            'blog_public': 'unchecked',
> +            'Submit': 'submit',
> +            'user_name': 'admin',
> +            'admin_email': 'root@xxxxxxxxxxxxxxxxxxx',
> +            'weblog_title': 'Test Blog',
> +        }
> +        self.test_wordpress.first_install(self.test_service_ip)
> +
> +        # Test that we wrote initial admin credentials inside the operator pod.
> +        self.test_wordpress._write_initial_password.assert_called_with(TEST_GENERATED_PASSWORD, "/root/initial.passwd")
> +
> +        # Test that we POST'd our initial configuration options to the wordpress API.
> +        self.test_wordpress.call_wordpress.assert_called_with(
> +            self.test_service_ip, "/wp-admin/install.php?step=2", redirects=True, payload=test_payload
> +        )
> +
> +        # Test that we don't call the Wordpress API with missing (admin_email) initial settings.
> +        mocked_call_wordpress = mock.MagicMock(name="call_wordpress", return_value=True)
> +        self.test_wordpress.call_wordpress = mocked_call_wordpress
> +        self.test_wordpress.model_config["initial_settings"] = (
> +            "user_name: admin\n" "weblog_title: Test Blog\n" "blog_public: False"
> +        )
> +        rv = self.test_wordpress.first_install(self.test_service_ip)
> +        self.test_wordpress.call_wordpress.assert_not_called()
> +        self.assertIsNone(rv)

If you think it worth ensuring first_install returns None, sure. Normally people don't bother, because normally it isn't important and not important things are not worth testing.

> +
> +    def test_wordpress_configured(self):
> +        # Test install successful.
> +        success = RequestsResult()
> +        success.status_code = 200
> +        mocked_call_wordpress = mock.MagicMock(name="call_wordpress", return_value=success)
> +        self.test_wordpress.call_wordpress = mocked_call_wordpress
> +        self.test_wordpress.wordpress_configured(self.test_service_ip)
> +        self.test_wordpress.call_wordpress.assert_called_with(self.test_service_ip, "/", redirects=False)
> +
> +        # Test install failed.
> +        for uri in ("/wp-admin/install.php", "/wp-admin/setup-config.php"):
> +            failure = RequestsResult()
> +            failure.status_code = 302
> +            failure.headers["location"] = uri
> +            mocked_call_wordpress = mock.MagicMock(name="call_wordpress", return_value=failure)
> +            self.test_wordpress.call_wordpress = mocked_call_wordpress
> +            rv = self.test_wordpress.wordpress_configured(self.test_service_ip)
> +            self.assertFalse(rv)
> +
> +        # Test unexpected status code from webserver.
> +        for sc in (500, 403, 404):
> +            failure = RequestsResult()
> +            failure.status_code = sc
> +            mocked_call_wordpress = mock.MagicMock(name="call_wordpress", return_value=failure)
> +            self.test_wordpress.call_wordpress = mocked_call_wordpress
> +            with self.assertRaises(RuntimeError, msg="unexpected status_code returned from Wordpress") as _:

Drop the trailing 'as _', since it does nothing.

> +                self.test_wordpress.wordpress_configured(self.test_service_ip)
> +
> +    def test_is_vhost_ready(self):
> +        # Test vhost not ready yet and called with expected args.
> +        mocked_call_wordpress = mock.MagicMock(name="call_wordpress", return_value=None)
> +        self.test_wordpress.call_wordpress = mocked_call_wordpress
> +        rv = self.test_wordpress.is_vhost_ready(self.test_service_ip)
> +        self.assertFalse(rv)
> +        self.test_wordpress.call_wordpress.assert_called_with(self.test_service_ip, "/wp-login.php", redirects=False)
> +
> +        # Test vhost ready and has unexpected status_code
> +        failure = RequestsResult()
> +        failure.status_code = 403
> +        mocked_call_wordpress = mock.MagicMock(name="call_wordpress", return_value=failure)
> +        self.test_wordpress.call_wordpress = mocked_call_wordpress
> +        rv = self.test_wordpress.is_vhost_ready(self.test_service_ip)
> +        self.assertFalse(rv)
> +
> +        # Test vhost isn't up yet.
> +        mocked_call_wordpress = mock.MagicMock(name="call_wordpress", side_effect=requests.exceptions.ConnectionError)
> +        self.test_wordpress.call_wordpress = mocked_call_wordpress
> +        rv = self.test_wordpress.is_vhost_ready(self.test_service_ip)
> +        self.assertFalse(rv)
> +
> +        # Test vhost is ready.
> +        mocked_call_wordpress = mock.MagicMock(name="call_wordpress", return_value=True)
> +        self.test_wordpress.call_wordpress = mocked_call_wordpress
> +        rv = self.test_wordpress.is_vhost_ready(self.test_service_ip)
> +        self.assertTrue(rv)


-- 
https://code.launchpad.net/~tcuthbert/charm-k8s-wordpress/+git/charm-k8s-wordpress/+merge/381805
Your team Wordpress Charmers is requested to review the proposed merge of ~tcuthbert/charm-k8s-wordpress/+git/charm-k8s-wordpress:operator_charm_tests into charm-k8s-wordpress:master.