Code review comment for lp:~joeborg/charm-helpers/apache-config

Hey Joe

I admit that the current test coverage is not great. This isn't a detectable change at present, but the infrastructure is there in tests/contrib/hardening/apache/checks/test_config.py at line 52:

Existing function:

    @patch.object(config.utils, 'get_settings', lambda x: {
        'common': {'apache_dir': TEST_TMPDIR,
                   'traceenable': 'Off'},
        'hardening': {
            'allowed_http_methods': {'GOGETEM'},
            'modules_to_disable': {'modfoo'}
        }
    })
    @patch.object(config.subprocess, 'call', lambda *args, **kwargs: 0)
    def test_get_audits_apache_is_installed(self):
        audits = config.get_audits()
        self.assertEqual(7, len(audits))

Essentially, the 1st element of audits is being changed here, so you could check after the self.assertEqual(7, len(audits)) something like this:

    self.assertEqual(audits[1].paths, ["{}/apache.conf".format(TEST_TMPDIR)])

This would at least verify that your change is what you think it should be. I agree that it's tricky with these config/declaration type changes. Perhaps also add a comment to indicate that it's a change of behaviour from previously?

However, I'm open to the idea of not testing this change, and just accepting it as is.