From c6d470194b0821f766da83d4388ef88784cdf6c4 Mon Sep 17 00:00:00 2001 From: Fiona Klute Date: Wed, 10 Mar 2021 09:25:32 +0100 Subject: [PATCH 1/5] mgstest.tests.TestRequest: Move tests to unittest The tests previously done via doctest didn't really document anything. Running them via unittest is more flexible and might allow for future extensions. --- test/Makefile.am | 9 ++++--- test/mgstest/test_tests.py | 54 ++++++++++++++++++++++++++++++++++++++ test/mgstest/tests.py | 45 ------------------------------- test/unittest-mgstest.py | 11 ++++++++ 4 files changed, 70 insertions(+), 49 deletions(-) create mode 100644 test/mgstest/test_tests.py create mode 100755 test/unittest-mgstest.py diff --git a/test/Makefile.am b/test/Makefile.am index 3f1b402..5e0086f 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -49,7 +49,7 @@ dist_check_SCRIPTS = netns_py.bash test-template.bash.in TEST_EXTENSIONS = .bash .py PY_LOG_COMPILER = $(PYTHON) -TESTS = doctest-mgstest.py $(test_scripts) +TESTS = doctest-mgstest.py unittest-mgstest.py $(test_scripts) check_PROGRAMS = pgpcrc pgpcrc_SOURCES = pgpcrc.c @@ -66,9 +66,10 @@ endif # Python tools for tests noinst_PYTHON = https-test-client.py mgstest/http.py mgstest/__init__.py \ mgstest/hooks.py mgstest/ocsp.py mgstest/services.py \ - mgstest/softhsm.py mgstest/tests.py mgstest/valgrind.py runtest.py \ - softhsm-init.py doctest-mgstest.py required-modules.py data/ocsp.py \ - check_test_ips.py + mgstest/softhsm.py mgstest/tests.py mgstest/valgrind.py \ + mgstest/test_tests.py runtest.py \ + softhsm-init.py doctest-mgstest.py unittest-mgstest.py \ + required-modules.py data/ocsp.py check_test_ips.py # Identities in the miniature CA, server, and client environment for # the test suite diff --git a/test/mgstest/test_tests.py b/test/mgstest/test_tests.py new file mode 100644 index 0000000..f957e51 --- /dev/null +++ b/test/mgstest/test_tests.py @@ -0,0 +1,54 @@ +import unittest +from . import TestExpectationFailed +from .tests import TestRequest + + +class TestTestRequest(unittest.TestCase): + def test_check_headers(self): + r = TestRequest(path='/test.txt', + expect={'headers': {'X-Forbidden-Header': None, + 'X-Required-Header': 'Hi!'}}) + r.check_headers({'X-Required-Header': 'Hi!'}) + + with self.assertRaisesRegex(TestExpectationFailed, + 'Unexpected value in header'): + r.check_headers({'X-Required-Header': 'Hello!'}) + + with self.assertRaisesRegex(TestExpectationFailed, + 'Unexpected value in header'): + r.check_headers({'X-Forbidden-Header': 'Hi!'}) + + def test_check_body_exact(self): + r = TestRequest( + path='/test.txt', method='GET', headers={}, + expect={'status': 200, 'body': {'exactly': 'test\n'}}) + r.check_body('test\n') + + with self.assertRaisesRegex( + TestExpectationFailed, + r"Unexpected body: 'xyz\\n' != 'test\\n'"): + r.check_body('xyz\n') + + def test_check_body_contains(self): + r1 = TestRequest( + path='/test.txt', method='GET', headers={}, + expect={'status': 200, 'body': {'contains': ['tes', 'est']}}) + r1.check_body('test\n') + with self.assertRaisesRegex( + TestExpectationFailed, + r"Unexpected body: 'est\\n' does not contain 'tes'"): + r1.check_body('est\n') + + r2 = TestRequest( + path='/test.txt', method='GET', headers={}, + expect={'status': 200, 'body': {'contains': 'test'}}) + r2.check_body('test\n') + + def test_expects_conn_reset(self): + r1 = TestRequest(path='/test.txt', method='GET', headers={}, + expect={'status': 200, 'body': {'contains': 'test'}}) + self.assertFalse(r1.expects_conn_reset()) + + r2 = TestRequest(path='/test.txt', method='GET', headers={}, + expect={'reset': True}) + self.assertTrue(r2.expects_conn_reset()) diff --git a/test/mgstest/tests.py b/test/mgstest/tests.py index a2b42ce..f83702c 100644 --- a/test/mgstest/tests.py +++ b/test/mgstest/tests.py @@ -250,22 +250,6 @@ def run(self, conn, response_log=None): self.check_response(resp, body) def check_headers(self, headers): - """ - >>> r1 = TestRequest(path='/test.txt', - ... expect={'headers': {'X-Forbidden-Header': None, - ... 'X-Required-Header': 'Hi!'}}) - >>> r1.check_headers({'X-Required-Header': 'Hi!'}) - >>> r1.check_headers({'X-Required-Header': 'Hello!'}) - Traceback (most recent call last): - ... - mgstest.TestExpectationFailed: Unexpected value in header \ -X-Required-Header: 'Hello!', expected 'Hi!' - >>> r1.check_headers({'X-Forbidden-Header': 'Hi!'}) - Traceback (most recent call last): - ... - mgstest.TestExpectationFailed: Unexpected value in header \ -X-Forbidden-Header: 'Hi!', expected None - """ for name, expected in self.expect['headers'].items(): value = headers.get(name) expected = subst_env(expected) @@ -275,26 +259,6 @@ def check_headers(self, headers): f'expected {expected!r}') def check_body(self, body): - """ - >>> r1 = TestRequest(path='/test.txt', method='GET', headers={}, \ -expect={'status': 200, 'body': {'exactly': 'test\\n'}}) - >>> r1.check_body('test\\n') - >>> r1.check_body('xyz\\n') - Traceback (most recent call last): - ... - mgstest.TestExpectationFailed: Unexpected body: 'xyz\\n' != 'test\\n' - >>> r2 = TestRequest(path='/test.txt', method='GET', headers={}, \ -expect={'status': 200, 'body': {'contains': ['tes', 'est']}}) - >>> r2.check_body('test\\n') - >>> r2.check_body('est\\n') - Traceback (most recent call last): - ... - mgstest.TestExpectationFailed: Unexpected body: 'est\\n' \ -does not contain 'tes' - >>> r3 = TestRequest(path='/test.txt', method='GET', headers={}, \ -expect={'status': 200, 'body': {'contains': 'test'}}) - >>> r3.check_body('test\\n') - """ if 'exactly' in self.expect['body'] \ and body != self.expect['body']['exactly']: raise TestExpectationFailed( @@ -327,15 +291,6 @@ def expects_conn_reset(self): """Returns True if running this request is expected to fail due to the connection being reset. That usually means the underlying TLS connection failed. - - >>> r1 = TestRequest(path='/test.txt', method='GET', headers={}, \ -expect={'status': 200, 'body': {'contains': 'test'}}) - >>> r1.expects_conn_reset() - False - >>> r2 = TestRequest(path='/test.txt', method='GET', headers={}, \ -expect={'reset': True}) - >>> r2.expects_conn_reset() - True """ if 'reset' in self.expect: return self.expect['reset'] diff --git a/test/unittest-mgstest.py b/test/unittest-mgstest.py new file mode 100755 index 0000000..bbed377 --- /dev/null +++ b/test/unittest-mgstest.py @@ -0,0 +1,11 @@ +#!/usr/bin/python3 +import os +import sys +import unittest + +if __name__ == "__main__": + suite = unittest.defaultTestLoader.discover( + 'mgstest', top_level_dir=os.environ.get('srcdir', '.')) + result = unittest.TextTestRunner(verbosity=2, buffer=True).run(suite) + if not result.wasSuccessful(): + sys.exit(1) From 520ab08c7466168e0800784011031b8383727a5b Mon Sep 17 00:00:00 2001 From: Fiona Klute Date: Wed, 10 Mar 2021 23:54:46 +0100 Subject: [PATCH 2/5] Test mgstest.tests.TestRequest using a mocked connection --- test/mgstest/test_tests.py | 49 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/test/mgstest/test_tests.py b/test/mgstest/test_tests.py index f957e51..797543b 100644 --- a/test/mgstest/test_tests.py +++ b/test/mgstest/test_tests.py @@ -1,9 +1,58 @@ import unittest +from http import HTTPStatus +from unittest.mock import Mock, patch + from . import TestExpectationFailed from .tests import TestRequest +def mock_response(status=HTTPStatus.OK, headers=dict(), body=b''): + response = Mock() + response.status = status + response.reason = status.phrase + response.getheaders.return_value = [(k, v) for k, v in headers.items()] + response.read.return_value = body + return response + + class TestTestRequest(unittest.TestCase): + def test_run(self): + """Check that TestRequest matches regular response correctly.""" + response = mock_response( + HTTPStatus.OK, {'X-Required-Header': 'Hi!'}, b'Hello World!\n') + conn = Mock() + conn.getresponse.return_value = response + + r = TestRequest(path='/test.txt', + expect={'status': 200, + 'headers': {'X-Forbidden-Header': None, + 'X-Required-Header': 'Hi!'}, + 'body': {'contains': 'Hello'}}) + r.run(conn) + conn.request.assert_called_with( + 'GET', '/test.txt', body=None, headers={}) + + def test_run_unexpected_reset(self): + """An unexpected exception breaks out of the TestRequest run.""" + conn = Mock() + conn.request.side_effect = ConnectionResetError + r = TestRequest(path='/test.txt', + expect={'status': 200}) + with self.assertRaises(ConnectionResetError): + r.run(conn) + conn.request.assert_called_with( + 'GET', '/test.txt', body=None, headers={}) + + def test_run_expected_reset(self): + """If the TestRequest expects an exception, it gets caught.""" + conn = Mock() + conn.request.side_effect = ConnectionResetError + r = TestRequest(path='/test.txt', + expect={'reset': True}) + r.run(conn) + conn.request.assert_called_with( + 'GET', '/test.txt', body=None, headers={}) + def test_check_headers(self): r = TestRequest(path='/test.txt', expect={'headers': {'X-Forbidden-Header': None, From 8a63c765f451cf83951a9c579d50f85ab25f5a17 Mon Sep 17 00:00:00 2001 From: Fiona Klute Date: Fri, 12 Mar 2021 09:59:24 +0100 Subject: [PATCH 3/5] Unittest for mgstest.tests.TestConnection --- test/mgstest/test_tests.py | 58 +++++++++++++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/test/mgstest/test_tests.py b/test/mgstest/test_tests.py index 797543b..5f629d5 100644 --- a/test/mgstest/test_tests.py +++ b/test/mgstest/test_tests.py @@ -1,6 +1,7 @@ import unittest +import yaml from http import HTTPStatus -from unittest.mock import Mock, patch +from unittest.mock import Mock, call, patch from . import TestExpectationFailed from .tests import TestRequest @@ -101,3 +102,58 @@ def test_expects_conn_reset(self): r2 = TestRequest(path='/test.txt', method='GET', headers={}, expect={'reset': True}) self.assertTrue(r2.expects_conn_reset()) + + +class TestTestConnection(unittest.TestCase): + def test_run(self): + """TestConnection with a successful and a failing TestRequest.""" + test = """ + !connection + gnutls_params: + - x509cafile=authority/x509.pem + actions: + - !request + path: /test.txt + headers: + X-Test: mgstest + expect: + status: 200 + headers: + X-Required: 'Hi!' + body: + exactly: | + Hello World! + - !request + method: POST + path: /test.txt + expect: + status: 200 + body: + exactly: | + Hello World! + """ + conn = yaml.load(test, Loader=yaml.Loader) + + responses = [ + mock_response( + HTTPStatus.OK, {'X-Required': 'Hi!'}, + b'Hello World!\n'), + mock_response( + HTTPStatus.METHOD_NOT_ALLOWED, {}, b'No such file!\n')] + + # note that this patches HTTPSubprocessConnection as imported + # into mgstest.tests, not in the origin package + with patch('mgstest.tests.HTTPSubprocessConnection', spec=True) as P: + # the mock provided by patch acts as class, get the instance + instance = P.return_value + instance.getresponse.side_effect = responses + with self.assertRaisesRegex( + TestExpectationFailed, + r"Unexpected status: 405 != 200"): + conn.run() + + instance.request.assert_has_calls([ + call('GET', '/test.txt', body=None, headers={'X-Test': 'mgstest'}), + call('POST', '/test.txt', body=None, headers={}) + ]) + self.assertEqual(instance.request.call_count, 2) From 8f5dc4b1cab21b53e5ab31f96b20bd1737f3aa9b Mon Sep 17 00:00:00 2001 From: Fiona Klute Date: Fri, 12 Mar 2021 10:35:41 +0100 Subject: [PATCH 4/5] Import unittest.mock instead of components for doctest compatibility --- test/mgstest/test_tests.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/test/mgstest/test_tests.py b/test/mgstest/test_tests.py index 5f629d5..6153a33 100644 --- a/test/mgstest/test_tests.py +++ b/test/mgstest/test_tests.py @@ -1,14 +1,14 @@ import unittest +import unittest.mock import yaml from http import HTTPStatus -from unittest.mock import Mock, call, patch from . import TestExpectationFailed from .tests import TestRequest def mock_response(status=HTTPStatus.OK, headers=dict(), body=b''): - response = Mock() + response = unittest.mock.Mock() response.status = status response.reason = status.phrase response.getheaders.return_value = [(k, v) for k, v in headers.items()] @@ -21,7 +21,7 @@ def test_run(self): """Check that TestRequest matches regular response correctly.""" response = mock_response( HTTPStatus.OK, {'X-Required-Header': 'Hi!'}, b'Hello World!\n') - conn = Mock() + conn = unittest.mock.Mock() conn.getresponse.return_value = response r = TestRequest(path='/test.txt', @@ -35,7 +35,7 @@ def test_run(self): def test_run_unexpected_reset(self): """An unexpected exception breaks out of the TestRequest run.""" - conn = Mock() + conn = unittest.mock.Mock() conn.request.side_effect = ConnectionResetError r = TestRequest(path='/test.txt', expect={'status': 200}) @@ -46,7 +46,7 @@ def test_run_unexpected_reset(self): def test_run_expected_reset(self): """If the TestRequest expects an exception, it gets caught.""" - conn = Mock() + conn = unittest.mock.Mock() conn.request.side_effect = ConnectionResetError r = TestRequest(path='/test.txt', expect={'reset': True}) @@ -107,6 +107,7 @@ def test_expects_conn_reset(self): class TestTestConnection(unittest.TestCase): def test_run(self): """TestConnection with a successful and a failing TestRequest.""" + test = """ !connection gnutls_params: @@ -143,7 +144,8 @@ def test_run(self): # note that this patches HTTPSubprocessConnection as imported # into mgstest.tests, not in the origin package - with patch('mgstest.tests.HTTPSubprocessConnection', spec=True) as P: + with unittest.mock.patch( + 'mgstest.tests.HTTPSubprocessConnection', spec=True) as P: # the mock provided by patch acts as class, get the instance instance = P.return_value instance.getresponse.side_effect = responses @@ -153,7 +155,8 @@ def test_run(self): conn.run() instance.request.assert_has_calls([ - call('GET', '/test.txt', body=None, headers={'X-Test': 'mgstest'}), - call('POST', '/test.txt', body=None, headers={}) + unittest.mock.call( + 'GET', '/test.txt', body=None, headers={'X-Test': 'mgstest'}), + unittest.mock.call('POST', '/test.txt', body=None, headers={}) ]) self.assertEqual(instance.request.call_count, 2) From caffc866cc56f1c0fb855ed3b9522ab666456e0a Mon Sep 17 00:00:00 2001 From: Fiona Klute Date: Fri, 12 Mar 2021 19:23:40 +0100 Subject: [PATCH 5/5] Fix error text in mock response --- test/mgstest/test_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/mgstest/test_tests.py b/test/mgstest/test_tests.py index 6153a33..2a72f2b 100644 --- a/test/mgstest/test_tests.py +++ b/test/mgstest/test_tests.py @@ -140,7 +140,7 @@ def test_run(self): HTTPStatus.OK, {'X-Required': 'Hi!'}, b'Hello World!\n'), mock_response( - HTTPStatus.METHOD_NOT_ALLOWED, {}, b'No such file!\n')] + HTTPStatus.METHOD_NOT_ALLOWED, {}, b'Cannot POST here!\n')] # note that this patches HTTPSubprocessConnection as imported # into mgstest.tests, not in the origin package