bind-charmers team mailing list archive

[Tom Haddon <tom.haddon@xxxxxxxxxxxxx>]

Can you add docstrings for all tests please? And one question inline about whether we need the make_pod_resources method

Diff comments:

> diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
> index 4b3ae7e..cadfb1e 100644
> --- a/tests/unit/test_charm.py
> +++ b/tests/unit/test_charm.py
> @@ -27,18 +28,131 @@ CONFIG_IMAGE_PASSWORD_MISSING = {
>      'https_proxy': '',
>  }
>  
> +CONFIG_VALID = {
> +    'bind_image_path': 'example.com/bind:v1',
> +    'bind_image_username': '',
> +    'bind_image_password': '',
> +    'container_config': '',
> +    'container_secrets': '',
> +    'custom_config_repo': '',
> +    'https_proxy': '',
> +}
> +
> +CONFIG_VALID_WITH_CONTAINER_CONFIG = {
> +    'bind_image_path': 'example.com/bind:v1',
> +    'bind_image_username': '',
> +    'bind_image_password': '',
> +    'container_config': '"magic_number": 123',
> +    'container_secrets': '',
> +    'custom_config_repo': '',
> +    'https_proxy': '',
> +}
> +
> +CONFIG_VALID_WITH_CONTAINER_CONFIG_AND_SECRETS = {
> +    'bind_image_path': 'example.com/bind:v1',
> +    'bind_image_username': '',
> +    'bind_image_password': '',
> +    'container_config': '"magic_number": 123',
> +    'container_secrets': '"secret_password": "xyzzy"',
> +    'custom_config_repo': '',
> +    'https_proxy': '',
> +}
> +
> +
> +class TestBindK8s(unittest.TestCase):
> +    maxDiff = None
>  
> -class TestBindK8sCharmHooksDisabled(unittest.TestCase):
>      def setUp(self):
>          self.harness = testing.Harness(BindK8sCharm)
>          self.harness.begin()
>          self.harness.disable_hooks()
>  
> -    def test_check_for_config_problems(self):
> +    def test_check_for_config_problems_empty_image_path(self):
>          self.harness.update_config(CONFIG_EMPTY)
>          expected = 'required setting(s) empty: bind_image_path'
>          self.assertEqual(self.harness.charm._check_for_config_problems(), expected)
>  
> +    def test_check_for_config_problems_empty_image_password(self):
>          self.harness.update_config(CONFIG_IMAGE_PASSWORD_MISSING)
>          expected = 'required setting(s) empty: bind_image_password'
>          self.assertEqual(self.harness.charm._check_for_config_problems(), expected)
> +
> +    def test_check_for_config_problems_none(self):
> +        self.harness.update_config(CONFIG_VALID)
> +        expected = ''
> +        self.assertEqual(self.harness.charm._check_for_config_problems(), expected)
> +
> +    def test_make_pod_resources(self):
> +        expected = {}
> +        self.assertEqual(self.harness.charm.make_pod_resources(), expected)

This doesn't exposes the fact that the make_pod_resources method doesn't really seem to do anything useful. Do we really need it or can we just remove it for now?

> +
> +    def test_make_pod_spec_basic(self):
> +        self.harness.update_config(CONFIG_VALID)
> +        expected = {
> +            'version': 2,
> +            'containers': [
> +                {
> +                    'name': 'bind',
> +                    'imageDetails': {'imagePath': 'example.com/bind:v1'},
> +                    'ports': [
> +                        {'containerPort': 53, 'name': 'domain-tcp', 'protocol': 'TCP'},
> +                        {'containerPort': 53, 'name': 'domain-udp', 'protocol': 'UDP'},
> +                    ],
> +                    'config': {},
> +                    'kubernetes': {'readinessProbe': {'exec': {'command': ['/usr/local/bin/dns-check.sh']}}},
> +                }
> +            ],
> +        }
> +        self.assertEqual(self.harness.charm.make_pod_spec(), expected)
> +
> +    def test_make_pod_spec_with_extra_config(self):
> +        self.harness.update_config(CONFIG_VALID_WITH_CONTAINER_CONFIG)
> +        expected = {
> +            'version': 2,
> +            'containers': [
> +                {
> +                    'name': 'bind',
> +                    'imageDetails': {'imagePath': 'example.com/bind:v1'},
> +                    'ports': [
> +                        {'containerPort': 53, 'name': 'domain-tcp', 'protocol': 'TCP'},
> +                        {'containerPort': 53, 'name': 'domain-udp', 'protocol': 'UDP'},
> +                    ],
> +                    'config': {'magic_number': 123},
> +                    'kubernetes': {'readinessProbe': {'exec': {'command': ['/usr/local/bin/dns-check.sh']}}},
> +                }
> +            ],
> +        }
> +        self.assertEqual(self.harness.charm.make_pod_spec(), expected)
> +
> +    def test_make_pod_spec_with_extra_config_and_secrets(self):
> +        self.harness.update_config(CONFIG_VALID_WITH_CONTAINER_CONFIG_AND_SECRETS)
> +        expected = {
> +            'version': 2,
> +            'containers': [
> +                {
> +                    'name': 'bind',
> +                    'imageDetails': {'imagePath': 'example.com/bind:v1'},
> +                    'ports': [
> +                        {'containerPort': 53, 'name': 'domain-tcp', 'protocol': 'TCP'},
> +                        {'containerPort': 53, 'name': 'domain-udp', 'protocol': 'UDP'},
> +                    ],
> +                    'config': {'magic_number': 123, 'secret_password': 'xyzzy'},
> +                    'kubernetes': {'readinessProbe': {'exec': {'command': ['/usr/local/bin/dns-check.sh']}}},
> +                }
> +            ],
> +        }
> +        self.assertEqual(self.harness.charm.make_pod_spec(), expected)
> +
> +    def test_configure_pod_as_leader(self):
> +        self.harness.enable_hooks()
> +        self.harness.set_leader(True)
> +        self.harness.update_config(CONFIG_VALID)
> +        expected = ActiveStatus('Pod configured')
> +        self.assertEqual(self.harness.model.unit.status, expected)
> +
> +    def test_configure_pod_as_non_leader(self):
> +        self.harness.enable_hooks()
> +        self.harness.set_leader(False)
> +        self.harness.update_config(CONFIG_VALID)
> +        expected = ActiveStatus()
> +        self.assertEqual(self.harness.model.unit.status, expected)


-- 
https://code.launchpad.net/~barryprice/charm-k8s-bind/+git/charm-k8s-bind/+merge/388452
Your team Bind Charmers is requested to review the proposed merge of ~barryprice/charm-k8s-bind/+git/charm-k8s-bind:unit-tests into charm-k8s-bind:master.