Code review comment for ~barryprice/postgresql-charm/+git/postgresql-charm:master

Unit tests look fine.

The best way of testing stuff under reactive/ is to move the code elsewhere. This needs to improve, but isn't your problem. Best way for now is to move testable chunks of code elsewhere.

The main() can be testable, but needs a mock for the print function. Mocking print and dealing with sys.exit are not obvious, so it might be better to avoid using them (using 'return' instead of sys.exit, leaving it to the __main__ section to sys.exit(main(sys.argv)) instead of just main(sys.argv), and defining an easily mocked wrapper around print and using it instead.

Start by changing the main() signature into main(args=sys.argv), which allows you to pass in alternative arguments giving you control.

Next, move the 'max_age_filename' constant definition from inside main() to module scope (and in CAPS per PEP8 naming conventions). This allows your test to override it (using a try: finally: block to put back the original value, or setUp/tearDown methods to do the same). Alternatively, make it a function def max_age_filename(): return '/var/whatsit'. This function can then be mocked by the test to return a different result. A third alternative is pass it in as yet another parameter to main(), but that trick gets old fast.

Now your test can look something like:

import check_latest_ready_wal

@patch('check_latest_ready_wal.print')
def test_main(self, mock_print):
    with tempfile.TemporaryFile() as max_age_f:
        max_age_f.write(b'42\n')
        max_age_f.flush()
        org_max_age_path = check_latest_ready_wal.MAX_AGE_FILENAME
        try:
            check_latest_ready_wal.MAX_AGE_FILENAME = max_age_f.name
            check_latest_ready_wal([]) # overriding sys.argv
            with self.assertRaises(SystemExit) as ex:
                main([])
            self.assertEqual(ex.code, 0)
            mock_print.assert_called_once_with('OK: Expected message')
        finally:
            check_latest_ready_wal.MAX_AGE_FILENAME = org_max_age_path