From cf8f53df25b936ab7bc37e68079cac414386392c Mon Sep 17 00:00:00 2001 From: "Karl O. Pinc" Date: Thu, 12 Dec 2019 04:48:46 -0600 Subject: [PATCH] Report all configuration errors, not just the first --- src/pgwui_server/__init__.py | 72 +++++++---- tests/test___init__.py | 237 +++++++++++++++++++++++++---------- 2 files changed, 214 insertions(+), 95 deletions(-) diff --git a/src/pgwui_server/__init__.py b/src/pgwui_server/__init__.py index a695958..73ca789 100644 --- a/src/pgwui_server/__init__.py +++ b/src/pgwui_server/__init__.py @@ -56,6 +56,11 @@ class Error(Exception): pass +class BadSettingsAbort(Error): + def __init__(self): + super().__init__('Aborting due to bad setting(s)') + + class UnknownSettingKeyError(Error): def __init__(self, key): super().__init__('Unknown PGWUI setting: {}'.format(key)) @@ -98,28 +103,28 @@ class HMACLengthError(BadHMACError): # Functions -def abort_on_bad_setting(key): +def abort_on_bad_setting(errors, key): '''Abort on a bad pgwui setting ''' if key[:6] == 'pgwui.': if key[6:] not in SETTINGS: - raise UnknownSettingKeyError(key) + errors.append(UnknownSettingKeyError(key)) -def require_setting(setting, settings): +def require_setting(errors, setting, settings): if setting not in settings: - raise MissingSettingError(setting) + errors.append(MissingSettingError(setting)) -def boolean_setting(setting, settings): +def boolean_setting(errors, setting, settings): if setting in settings: val = literal_eval(settings[setting]) if (val is not True and val is not False): - raise NotBooleanSettingError(setting, settings[setting]) + errors.append(NotBooleanSettingError(setting, settings[setting])) -def validate_setting_values(settings): +def validate_setting_values(errors, settings): '''Check each settings value for validity ''' # pg_host can be missing, it defaults to the Unix socket (in psycopg2) @@ -129,18 +134,18 @@ def validate_setting_values(settings): # default_db can be missing, then the user sees no default # dry_run - require_setting('pgwui.dry_run', settings) - boolean_setting('pgwui.dry_run', settings) + require_setting(errors, 'pgwui.dry_run', settings) + boolean_setting(errors, 'pgwui.dry_run', settings) # route_prefix can be missing, defaults to no route prefix which is fine. # routes can be missing, sensible defaults are built-in. # validate_hmac - boolean_setting('pgwui.validate_hmac', settings) + boolean_setting(errors, 'pgwui.validate_hmac', settings) # literal_column_headings - validate_literal_column_headings(settings) + validate_literal_column_headings(errors, settings) def do_validate_hmac(settings): @@ -150,46 +155,61 @@ def do_validate_hmac(settings): or literal_eval(settings['pgwui.validate_hmac'])) -def validate_hmac(settings): +def validate_hmac(errors, settings): '''Unless otherwise requested, validate the session.secret setting''' if not do_validate_hmac(settings): return if 'session.secret' not in settings: - raise NoHMACError() + errors.append(NoHMACError()) + return if len(settings['session.secret']) != HMAC_LEN: - raise HMACLengthError() + errors.append(HMACLengthError()) + return -def validate_literal_column_headings(settings): +def validate_literal_column_headings(errors, settings): '''Make sure the values are those allowed ''' value = settings.get('pgwui.literal_column_headings') if value is None: return if value not in ('on', 'off', 'ask'): - raise BadLiteralColumnHeadingsError(value) + errors.append(BadLiteralColumnHeadingsError(value)) -def validate_settings(settings): +def validate_settings(errors, settings): '''Be sure all settings validate ''' for key in settings.keys(): - abort_on_bad_setting(key) - validate_setting_values(settings) - validate_hmac(settings) + abort_on_bad_setting(errors, key) + validate_setting_values(errors, settings) + validate_hmac(errors, settings) + + +def exit_reporting_errors(errors): + '''Report errors and exit + ''' + tagged = [(logging.ERROR, error) for error in errors] + tagged.append((logging.CRITICAL, BadSettingsAbort())) + + for (level, error) in tagged: + log.log(level, error) + + for (level, error) in (tagged[0], tagged[-1]): + print(error, file=sys.stderr) # in case logging is broken + + sys.exit(1) def exit_on_invalid_settings(settings): '''Exit when settings don't validate ''' - try: - validate_settings(settings) - except Error as ex: - log.critical(ex) - print(ex, file=sys.stderr) # in case logging is broken - sys.exit(1) + errors = [] + validate_settings(errors, settings) + if errors: + exit_reporting_errors(errors) def parse_assignments(lines): diff --git a/tests/test___init__.py b/tests/test___init__.py index 743145a..2ea26a9 100644 --- a/tests/test___init__.py +++ b/tests/test___init__.py @@ -66,58 +66,81 @@ class MockConfig(): def test_abort_on_bad_setting_unknown(): '''Nothing bad happens when there's a non-pgwui setting''' - pgwui_server_init.abort_on_bad_setting('foo') + errors = [] + pgwui_server_init.abort_on_bad_setting(errors, 'foo') + + assert errors == [] def test_abort_on_bad_setting_bad(): - '''Raises an error on a bad pgwui setting''' - with pytest.raises(pgwui_server_init.UnknownSettingKeyError): - pgwui_server_init.abort_on_bad_setting('pgwui.foo') + '''Delivers an error on a bad pgwui setting''' + errors = [] + pgwui_server_init.abort_on_bad_setting(errors, 'pgwui.foo') + + assert errors + assert isinstance(errors[0], pgwui_server_init.UnknownSettingKeyError) def test_abort_on_bad_setting_good(): '''Does nothing when a known pgwui setting is supplied''' - pgwui_server_init.abort_on_bad_setting('pgwui.pg_host') + errors = [] + pgwui_server_init.abort_on_bad_setting(errors, 'pgwui.pg_host') + + assert errors == [] # require_setting() def test_require_setting_missing(): - '''Raise an exception when a required setting is missing''' - with pytest.raises(pgwui_server_init.MissingSettingError): - pgwui_server_init.require_setting('key', {}) + '''Deliver exception when a required setting is missing''' + errors = [] + pgwui_server_init.require_setting(errors, 'key', {}) + + assert errors + assert isinstance(errors[0], pgwui_server_init.MissingSettingError) def test_require_setting_present(): '''Does nothing when a required setting is present''' - pgwui_server_init.require_setting('key', - {'key': 'value'}) + errors = [] + pgwui_server_init.require_setting(errors, 'key', {'key': 'value'}) + + assert errors == [] # boolean_setting() def test_boolean_setting_missing(): '''Does nothing when the setting is not in the settings''' - pgwui_server_init.boolean_setting('key', {}) + errors = [] + pgwui_server_init.boolean_setting(errors, 'key', {}) + + assert errors == [] def test_boolean_setting_true(): '''Does nothing when the setting is "True"''' - pgwui_server_init.boolean_setting('key', - {'key': 'True'}) + errors = [] + pgwui_server_init.boolean_setting(errors, 'key', {'key': 'True'}) + + assert errors == [] def test_boolean_setting_false(): '''Does nothing when the setting is "False"''' - pgwui_server_init.boolean_setting('key', - {'key': 'False'}) + errors = [] + pgwui_server_init.boolean_setting(errors, 'key', {'key': 'False'}) + + assert errors == [] def test_boolean_setting_notboolean(): - '''Raise an exception when the setting does not evaluate to a boolean''' - with pytest.raises(pgwui_server_init.NotBooleanSettingError): - pgwui_server_init.boolean_setting('key', - {'key': '0'}) + '''Deliver an exception when the setting does not evaluate to a boolean''' + errors = [] + pgwui_server_init.boolean_setting(errors, 'key', {'key': '0'}) + + assert errors + assert isinstance(errors[0], pgwui_server_init.NotBooleanSettingError) # validate_setting_values() @@ -140,7 +163,7 @@ def test_validate_setting_values(monkeypatch): monkeypatch.setattr(pgwui_server_init, 'boolean_setting', mock_boolean_setting) - pgwui_server_init.validate_setting_values({}) + pgwui_server_init.validate_setting_values([], {}) assert require_setting_called assert boolean_setting_called @@ -170,66 +193,94 @@ def test_do_validate_hmac_False(): # validate_hmac() def test_validate_hmac_unvalidated(monkeypatch): - '''No error is raised when hmac validation is off''' + '''No error is returned when hmac validation is off''' monkeypatch.setattr(pgwui_server_init, 'do_validate_hmac', lambda *args: False) - pgwui_server_init.validate_hmac({}) + errors = [] + pgwui_server_init.validate_hmac(errors, {}) + + assert errors == [] def test_validate_hmac_success(monkeypatch): - '''No error is raised when hmac is validated an the right length''' + '''No error is returned when hmac is validated an the right length''' monkeypatch.setattr(pgwui_server_init, 'do_validate_hmac', lambda *args: True) + errors = [] pgwui_server_init.validate_hmac( - {'session.secret': 'x' * pgwui_server_init.HMAC_LEN}) + errors, {'session.secret': 'x' * pgwui_server_init.HMAC_LEN}) + + assert errors == [] def test_validate_hmac_missing(monkeypatch): - '''Raise error when hmac is validated and missing''' + '''Deliver error when hmac is validated and missing''' monkeypatch.setattr(pgwui_server_init, 'do_validate_hmac', lambda *args: True) - with pytest.raises(pgwui_server_init.NoHMACError): - pgwui_server_init.validate_hmac({}) + errors = [] + pgwui_server_init.validate_hmac(errors, {}) + + assert errors + assert isinstance(errors[0], pgwui_server_init.NoHMACError) def test_validate_hmac_length(monkeypatch): - '''Raise error when hmac is validated and the wrong length''' + '''Deliver error when hmac is validated and the wrong length''' monkeypatch.setattr(pgwui_server_init, 'do_validate_hmac', lambda *args: True) - with pytest.raises(pgwui_server_init.HMACLengthError): - pgwui_server_init.validate_hmac({'session.secret': ''}) + errors = [] + pgwui_server_init.validate_hmac(errors, {'session.secret': ''}) + + assert errors + assert isinstance(errors[0], pgwui_server_init.HMACLengthError) # validate_literal_column_headings() def test_validate_literal_column_headings_nosetting(): - '''No error is raised when there's no setting''' - pgwui_server_init.validate_literal_column_headings({}) + '''No error is delivered when there's no setting''' + errors = [] + pgwui_server_init.validate_literal_column_headings(errors, {}) + + assert errors == [] def test_validate_literal_column_headings_on(): - '''No error is raised when the setting is "on"''' + '''No error is delivered when the setting is "on"''' + errors = [] pgwui_server_init.validate_literal_column_headings( - {'pgwui.literal_column_headings': 'on'}) + errors, {'pgwui.literal_column_headings': 'on'}) + + assert errors == [] def test_validate_literal_column_headings_off(): - '''No error is raised when the setting is "off"''' + '''No error is delivered when the setting is "off"''' + errors = [] pgwui_server_init.validate_literal_column_headings( - {'pgwui.literal_column_headings': 'off'}) + errors, {'pgwui.literal_column_headings': 'off'}) + + assert errors == [] def test_validate_literal_column_headings_ask(): - '''No error is raised when the setting is "ask"''' + '''No error is delivered when the setting is "ask"''' + errors = [] pgwui_server_init.validate_literal_column_headings( - {'pgwui.literal_column_headings': 'ask'}) + errors, {'pgwui.literal_column_headings': 'ask'}) + + assert errors == [] def test_validate_literal_column_headings_bad(): - '''Raises an error when given a bad value''' - with pytest.raises(pgwui_server_init.BadLiteralColumnHeadingsError): - pgwui_server_init.validate_literal_column_headings( - {'pgwui.literal_column_headings': 'bad'}) + '''delivers an error when given a bad value''' + errors = [] + pgwui_server_init.validate_literal_column_headings( + errors, {'pgwui.literal_column_headings': 'bad'}) + + assert errors + assert isinstance( + errors[0], pgwui_server_init.BadLiteralColumnHeadingsError) # validate_settings() @@ -239,7 +290,7 @@ def test_validate_settings(monkeypatch): ''' count = 0 - def mock_abort_on_bad_setting(key): + def mock_abort_on_bad_setting(*args): nonlocal count count += 1 @@ -252,45 +303,96 @@ def test_validate_settings(monkeypatch): settings = {'key1': 'value1', 'key2': 'value2'} - pgwui_server_init.validate_settings(settings) + errors = [] + pgwui_server_init.validate_settings(errors, settings) assert count == len(settings) +# exit_reporting_errors() + +@pytest.fixture +def assert_exit1(): + def run(): + + exit1_called = False + + def mock_exit(status): + nonlocal exit1_called + exit1_called = status == 1 + + return mock_exit + + assert exit1_called + + return run + + +def test_exit_reporting_errors_logged( + assert_exit1, monkeypatch, caplog, capsys): + '''All errors are logged at ERROR, and a extra one at CRITICAL + ''' + monkeypatch.setattr(sys, 'exit', assert_exit1()) + caplog.set_level(logging.INFO) + errors = ['one', 'two', 'three'] + pgwui_server_init.exit_reporting_errors(errors) + + logs = caplog.record_tuples + + assert len(logs) == 4 + + levels = [log[1] for log in logs] + for level in levels[:-1]: + assert level == logging.ERROR + assert levels[-1] == logging.CRITICAL + + +def test_exit_reporting_errors_printed( + assert_exit1, monkeypatch, caplog, capsys): + '''First and last (the extra) errors are printed on stderr + ''' + monkeypatch.setattr(sys, 'exit', assert_exit1()) + errors = ['one', 'two', 'three'] + pgwui_server_init.exit_reporting_errors(errors) + + (out, err) = capsys.readouterr() + errlines = err.split('\n')[:-1] + + assert out == '' + assert len(errlines) == 2 + assert errlines[0] == 'one' + assert errlines[1] != 'two' + assert errlines[1] != 'three' + + # exit_on_invalid_settings() -def test_exit_on_invalid_settings_exits(monkeypatch, capsys, caplog): - '''Logs critical, prints on stderr, and exits with 1 when a +def test_exit_on_invalid_settings_invalid(monkeypatch): + '''Calls validate_settings and exit_reporting_errors() when setting is invalid ''' - caplog.set_level(logging.CRITICAL) - - exit1_called = False + def mock_validate_settings(errors, settings): + errors.append('error1') - def mock_exit(status): - nonlocal exit1_called - exit1_called = status == 1 + exit_reporting_errors_called = False - def mock_validate_settings(settings): - raise pgwui_server_init.Error() + def mock_exit_reporting_errors(errors): + nonlocal exit_reporting_errors_called + exit_reporting_errors_called = True monkeypatch.setattr(pgwui_server_init, 'validate_settings', mock_validate_settings) - monkeypatch.setattr(sys, 'exit', mock_exit) + monkeypatch.setattr(pgwui_server_init, 'exit_reporting_errors', + mock_exit_reporting_errors) pgwui_server_init.exit_on_invalid_settings({}) - assert exit1_called - assert len(caplog.record_tuples) == 1 - (out, err) = capsys.readouterr() - assert out == '' - assert err != '' - + assert exit_reporting_errors_called -def test_exit_on_invalid_settings_valid(monkeypatch, capsys, caplog): - '''Returns, without logging or printing, when all settings are valid''' - caplog.set_level(logging.INFO) - def mock_validate_settings(settings): +def test_exit_on_invalid_settings_valid(monkeypatch): + '''Returns, without exiting, when all settings are valid + ''' + def mock_validate_settings(errors, settings): pass monkeypatch.setattr(pgwui_server_init, 'validate_settings', @@ -298,10 +400,7 @@ def test_exit_on_invalid_settings_valid(monkeypatch, capsys, caplog): pgwui_server_init.exit_on_invalid_settings({}) - assert len(caplog.record_tuples) == 0 - (out, err) = capsys.readouterr() - assert out == '' - assert err == '' + assert True # parse_assignments() -- 2.34.1