From cadc71f073f373ea4188ab2a82ff38dc644d3f57 Mon Sep 17 00:00:00 2001 From: Vitaly Samigullin Date: Tue, 21 Nov 2023 22:40:26 +0100 Subject: [PATCH 1/6] Use testcontainers for integration testing --- setup.py | 4 ++- tests/test_integration.py | 64 ++++++++++++++++++++++++--------------- 2 files changed, 43 insertions(+), 25 deletions(-) diff --git a/setup.py b/setup.py index f5186cc7..23ee30f0 100644 --- a/setup.py +++ b/setup.py @@ -77,7 +77,9 @@ def read(path): extras_require=dict( test=[ 'crate[test]', - 'zc.customdoctests<2' + 'zc.customdoctests<2', + # FIXME once tested and merged, fix the name + 'cratedb-toolkit[test] @ git+https://github.com/pilosus/cratedb-toolkit@tech/18-adapt-testcontainers-to-unittest', ], devel=[ 'coverage<8', diff --git a/tests/test_integration.py b/tests/test_integration.py index 85244504..b5891da5 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -1,3 +1,4 @@ +import logging import os import ssl import sys @@ -7,6 +8,8 @@ from unittest import SkipTest, TestCase from unittest.mock import Mock, patch +from cratedb_toolkit.testing.testcontainers.cratedb import CrateDBTestAdapter +from cratedb_toolkit.util.common import setup_logging from urllib3.exceptions import LocationParseError from crate.client.exceptions import ProgrammingError @@ -22,32 +25,34 @@ from crate.crash.commands import Command from crate.crash.outputs import _val_len as val_len from crate.crash.printer import ColorPrinter -from crate.testing.layer import CrateLayer from tests import ftouch -if sys.platform != "linux": - raise SkipTest("Integration tests only supported on Linux") - -crate_version = os.getenv("CRATEDB_VERSION", "5.5.0") -crate_http_port = 44209 -crate_transport_port = 44309 -crate_settings = { - 'cluster.name': 'Testing44209', - 'node.name': 'crate', - 'psql.port': 45441, - 'lang.js.enabled': True, - 'http.port': crate_http_port, - 'transport.tcp.port': crate_transport_port -} -node = CrateLayer.from_uri( - f'https://cdn.crate.io/downloads/releases/cratedb/x64_linux/crate-{crate_version}.tar.gz', - 'crate', - settings=crate_settings -) + +class EntrypointOpts: + version = os.getenv("CRATEDB_VERSION", "5.4.5") + psql_port = 45441 + http_port = 44209 + transport_port = 44309 + settings = { + "cluster.name": "Testing44209", + "node.name": "crate", + "lang.js.enabled": True, + "psql.port": psql_port, + "http.port": http_port, + "transport.tcp.port": transport_port, + } +node = CrateDBTestAdapter(crate_version=EntrypointOpts.version) + def setUpModule(): - node.start() + node.start( + cmd_opts=EntrypointOpts.settings, + # all ports inside container to be bound to the randomly generated ports on the host + ports={EntrypointOpts.http_port: None, + EntrypointOpts.psql_port: None, + EntrypointOpts.transport_port: None}) + node.reset() def tearDownModule(): @@ -61,7 +66,6 @@ def fake_stdin(data): stdin.seek(0) return stdin - class RaiseOnceSideEffect: """ A callable class used for mock side_effect. @@ -92,6 +96,8 @@ def test_connect(self): class CommandTest(TestCase): + def setUp(self): + node.reset() def _output_format(self, format, func, query="select name from sys.cluster"): orig_argv = sys.argv[:] @@ -286,7 +292,7 @@ def test_multiple_hosts(self): output = output.getvalue() lines = output.split('\n') self.assertRegex(lines[3], r'^\| http://[\d\.:]+ .*\| NULL .*\| FALSE .*\| Server not available') - self.assertRegex(lines[4], r'^\| http://[\d\.:]+. *\| crate .*\| TRUE .*\| OK') + self.assertRegex(lines[4], r'^\| http://.*@?localhost:\d+ *\| crate .*\| TRUE .*\| OK') finally: try: os.remove(tmphistory) @@ -699,7 +705,14 @@ def test_command_timeout(self): timeout=timeout) as crash: crash.logger = Mock() crash.process(slow_query) - crash.logger.warn.assert_any_call("No more Servers available, exception from last server: HTTPConnectionPool(host='127.0.0.1', port=44209): Read timed out. (read timeout=0.1)") + + # Get randomly generated host port bound to the predefined HTTP Interface port inside container + node.cratedb._container.reload() + host_port = node.cratedb._container.ports.get("{}/tcp".format(EntrypointOpts.http_port), [])[0].get("HostPort") + + crash.logger.warn.assert_any_call( + "No more Servers available, exception from last server: " + "HTTPConnectionPool(host='localhost', port={}): Read timed out. (read timeout=0.1)".format(host_port)) crash.logger.warn.assert_any_call("Use \\connect to connect to one or more servers first.") def test_username_param(self): @@ -831,3 +844,6 @@ def test_connect_info_not_available(self, is_conn_available): schema='test') as crash: self.assertEqual(crash.connect_info.user, None) self.assertEqual(crash.connect_info.schema, None) + + +setup_logging(level=logging.INFO) From 00c97a1217c55629d87ee975489e939e3418eaa4 Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Fri, 19 Jan 2024 21:33:34 +0100 Subject: [PATCH 2/6] Update changelog --- CHANGES.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.txt b/CHANGES.txt index 79c6cb23..00f2e02c 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -5,6 +5,8 @@ Changes for crash Unreleased ========== +- Use Python Testcontainers for integration testing + 2024/02/08 0.31.2 ================= From 29e730096fc4eca89a1c5ab9c258cae7375e7098 Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Fri, 19 Jan 2024 21:33:25 +0100 Subject: [PATCH 3/6] Dependencies: Update cratedb-toolkit --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 23ee30f0..e09f4706 100644 --- a/setup.py +++ b/setup.py @@ -79,7 +79,7 @@ def read(path): 'crate[test]', 'zc.customdoctests<2', # FIXME once tested and merged, fix the name - 'cratedb-toolkit[test] @ git+https://github.com/pilosus/cratedb-toolkit@tech/18-adapt-testcontainers-to-unittest', + 'cratedb-toolkit[test] @ git+https://github.com/crate-workbench/cratedb-toolkit@main', ], devel=[ 'coverage<8', From da28d898eecd0c2fa01221a09ff7790dd655c5f0 Mon Sep 17 00:00:00 2001 From: Vitaly Samigullin Date: Sat, 10 Feb 2024 22:04:59 +0100 Subject: [PATCH 4/6] fix: check if integration tests can be run, skip if they cannot GitHub Runner don't support Docker on MacOS for licensing issues. We want to skip integration tests that require Docker though testcontainers. We still may want to be able to run such test for local development. --- .gitignore | 1 + tests/test_integration.py | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/.gitignore b/.gitignore index 1a7cd4f7..602c3bf7 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ .coverage .crate-docs +.python-version .idea/ .installed.cfg .mypy_cache/ diff --git a/tests/test_integration.py b/tests/test_integration.py index b5891da5..c6c6ab9e 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -27,6 +27,28 @@ from crate.crash.printer import ColorPrinter from tests import ftouch +logger = logging.getLogger(__name__) + + +def _skip_tests_in_ci() -> bool: + """ + Return true if integration tests cannot be run in the CI pipeline + """ + # GitHub Runners have some default envs set, let's use them as an indicator of the CI run + # https://docs.github.com/en/actions/learn-github-actions/variables#default-environment-variables + inside_ci = (os.environ.get("CI") == "true") and ("GITHUB_RUN_ID" in os.environ) + + # Due to licensing issues GitHub Runners with MacOS doesn't support Docker + # https://github.com/orgs/community/discussions/25777 + os_not_supported = sys.platform == "darwin" + + logger.debug("Inside CI: '%s', OS: '%s'", inside_ci, sys.platform) + return inside_ci and os_not_supported + + +if _skip_tests_in_ci(): + raise SkipTest("Platform is not supported") + class EntrypointOpts: version = os.getenv("CRATEDB_VERSION", "5.4.5") @@ -45,6 +67,7 @@ class EntrypointOpts: node = CrateDBTestAdapter(crate_version=EntrypointOpts.version) + def setUpModule(): node.start( cmd_opts=EntrypointOpts.settings, @@ -66,6 +89,7 @@ def fake_stdin(data): stdin.seek(0) return stdin + class RaiseOnceSideEffect: """ A callable class used for mock side_effect. From 80924e5172a53fb81d188f694b2d376a04ac0b0c Mon Sep 17 00:00:00 2001 From: Vitaly Samigullin Date: Sat, 10 Feb 2024 22:19:51 +0100 Subject: [PATCH 5/6] fix: pass in proper hosts to CrateShell CrateShell invocation in integration tests relied on the default localhost:4200 instead of the custom host urls used in Docker containers. That resulted in a flood of urllib3 warning logs coming from the crate client. This change fix the hosts urls. --- tests/test_integration.py | 52 +++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/tests/test_integration.py b/tests/test_integration.py index c6c6ab9e..877d1186 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -223,7 +223,7 @@ def test_pprint_duplicate_keys(self): "| name | name |", "+------+------+", "+------+------+\n"]) - with CrateShell() as cmd: + with CrateShell(crate_hosts=[node.http_url]) as cmd: with patch('sys.stdout', new_callable=StringIO) as output: cmd.pprint([], ['name', 'name']) self.assertEqual(expected, output.getvalue()) @@ -235,7 +235,7 @@ def test_pprint_dont_guess_type(self): "+---------+", "| 0.50 |", "+---------+\n"]) - with CrateShell() as cmd: + with CrateShell(crate_hosts=[node.http_url]) as cmd: with patch('sys.stdout', new_callable=StringIO) as output: cmd.pprint([["0.50"]], ['version']) self.assertEqual(expected, output.getvalue()) @@ -384,7 +384,7 @@ def test_tabulate_null_int_column(self): '| 1 |', '| NULL |', '+------+\n']) - with CrateShell() as cmd: + with CrateShell(crate_hosts=[node.http_url]) as cmd: with patch('sys.stdout', new_callable=StringIO) as output: cmd.pprint(rows, cols=['x']) self.assertEqual(expected, output.getvalue()) @@ -400,7 +400,7 @@ def test_tabulate_boolean_int_column(self): '| FALSE |', '| 1 |', '+-------+\n']) - with CrateShell() as cmd: + with CrateShell(crate_hosts=[node.http_url]) as cmd: with patch('sys.stdout', new_callable=StringIO) as output: cmd.pprint(rows, cols=['x']) self.assertEqual(expected, output.getvalue()) @@ -417,7 +417,7 @@ def test_multiline_header(self): '| FALSE |', '| 1 |', '+-------+\n']) - with CrateShell() as cmd: + with CrateShell(crate_hosts=[node.http_url]) as cmd: with patch('sys.stdout', new_callable=StringIO) as output: cmd.pprint(rows, cols=['x\ny']) self.assertEqual(expected, output.getvalue()) @@ -436,7 +436,7 @@ def test_multiline_row(self): '| name string | | |', '| ) | | |', '+-----------------------+-----+---+\n']) - with CrateShell() as cmd: + with CrateShell(crate_hosts=[node.http_url]) as cmd: with patch('sys.stdout', new_callable=StringIO) as output: cmd.pprint(rows, cols=['show create table foo', 'a', 'b']) self.assertEqual(expected, output.getvalue()) @@ -458,7 +458,7 @@ def test_tabulate_empty_line(self): '| | Planet |', '+------------------------------------+-------------+\n']) - with CrateShell() as cmd: + with CrateShell(crate_hosts=[node.http_url]) as cmd: with patch('sys.stdout', new_callable=StringIO) as output: cmd.pprint(rows, cols=['min(name)', 'kind']) # assert 0 @@ -481,7 +481,7 @@ def test_empty_line_first_row_first_column(self): '| Galactic Sector QQ7 Active J Gamma | Galaxy |', '+------------------------------------+-------------+\n']) - with CrateShell() as cmd: + with CrateShell(crate_hosts=[node.http_url]) as cmd: with patch('sys.stdout', new_callable=StringIO) as output: cmd.pprint(rows, cols=['min(name)', 'kind']) self.assertEqual(expected, output.getvalue()) @@ -505,7 +505,7 @@ def test_empty_first_row(self): '| Alpha Centauri | Alpha - Centauri |', '+---------------------+-----------------------+\n']) - with CrateShell() as cmd: + with CrateShell(crate_hosts=[node.http_url]) as cmd: with patch('sys.stdout', new_callable=StringIO) as output: cmd.pprint(rows, cols=['name', 'replaced']) self.assertEqual(expected, output.getvalue()) @@ -527,7 +527,7 @@ def test_any_empty(self): '| Features and conformance views | FALSE | 3 | SQL_LANGUAGES view |', '+--------------------------------+--------------+----------------+--------------------+\n']) - with CrateShell() as cmd: + with CrateShell(crate_hosts=[node.http_url]) as cmd: with patch('sys.stdout', new_callable=StringIO) as output: cmd.pprint(rows, cols=['feature_name', 'is_supported', 'sub_feature_id', 'sub_feature_name']) self.assertEqual(expected, output.getvalue()) @@ -563,7 +563,7 @@ def test_first_column_first_row_empty(self): '| NULL | 1.0 |', '+------------------------------------+--------+\n']) - with CrateShell() as cmd: + with CrateShell(crate_hosts=[node.http_url]) as cmd: with patch('sys.stdout', new_callable=StringIO) as output: cmd.pprint(rows, cols=['name', '_score']) self.assertEqual(expected, output.getvalue()) @@ -582,7 +582,7 @@ def test_error_exit_code(self): self.assertEqual(e.code, 1) def test_verbose_with_error_trace(self): - with CrateShell(error_trace=True) as cmd: + with CrateShell(crate_hosts=[node.http_url], error_trace=True) as cmd: cmd.logger = Mock() cmd.cursor.execute = Mock(side_effect=ProgrammingError(msg="the error message", error_trace="error trace")) @@ -591,7 +591,7 @@ def test_verbose_with_error_trace(self): cmd.logger.critical.assert_called_with("\nerror trace") def test_verbose_no_error_trace(self): - with CrateShell(error_trace=True) as cmd: + with CrateShell(crate_hosts=[node.http_url], error_trace=True) as cmd: cmd.logger = Mock() cmd.cursor.execute = Mock(side_effect=ProgrammingError(msg="the error message", error_trace=None)) @@ -607,7 +607,7 @@ def test_rendering_object(self): '+-------------------------------+', '| {"age": 42, "name": "Arthur"} |', '+-------------------------------+\n']) - with CrateShell() as cmd: + with CrateShell(crate_hosts=[node.http_url]) as cmd: with patch('sys.stdout', new_callable=StringIO) as output: cmd.pprint([[user]], ['user']) self.assertEqual(expected, output.getvalue()) @@ -620,7 +620,7 @@ def test_rendering_array(self): '+--------------------+', '| ["Arthur", "Ford"] |', '+--------------------+\n']) - with CrateShell() as cmd: + with CrateShell(crate_hosts=[node.http_url]) as cmd: with patch('sys.stdout', new_callable=StringIO) as output: cmd.pprint([[names]], ['names']) self.assertEqual(expected, output.getvalue()) @@ -633,14 +633,14 @@ def test_rendering_float(self): '| 3.1415926535 |', '| 42.0 |', '+---------------+\n']) - with CrateShell() as cmd: + with CrateShell(crate_hosts=[node.http_url]) as cmd: with patch('sys.stdout', new_callable=StringIO) as output: cmd.pprint([[3.1415926535], [42.0]], ['number']) self.assertEqual(expected, output.getvalue()) def test_help_command(self): """Test output of help command""" - command = CrateShell(is_tty=False) + command = CrateShell(crate_hosts=[node.http_url], is_tty=False) expected = "\n".join([ '\\? print this help', '\\autocapitalize toggle automatic capitalization of SQL keywords', @@ -660,7 +660,7 @@ def test_help_command(self): help_ = command.commands['?'] self.assertTrue(isinstance(help_, Command)) self.assertEqual(expected, help_(command)) - with CrateShell(is_tty=False) as cmd: + with CrateShell(crate_hosts=[node.http_url], is_tty=False) as cmd: output = StringIO() cmd.logger = ColorPrinter(False, stream=output) text = help_(cmd, 'arg1', 'arg2') @@ -701,7 +701,7 @@ def test_wrong_host_format(self): _create_shell(crate_hosts, False, None, False, args) def test_command_timeout(self): - with CrateShell(node.http_url) as crash: + with CrateShell(crate_hosts=[node.http_url]) as crash: crash.process(""" CREATE FUNCTION fib(long) RETURNS LONG @@ -716,7 +716,7 @@ def test_command_timeout(self): slow_query = "SELECT fib(35)" # without verbose - with CrateShell(node.http_url, + with CrateShell(crate_hosts=[node.http_url], error_trace=False, timeout=timeout) as crash: crash.logger = Mock() @@ -724,7 +724,7 @@ def test_command_timeout(self): crash.logger.warn.assert_any_call("Use \\connect to connect to one or more servers first.") # with verbose - with CrateShell(node.http_url, + with CrateShell(crate_hosts=[node.http_url], error_trace=True, timeout=timeout) as crash: crash.logger = Mock() @@ -740,7 +740,7 @@ def test_command_timeout(self): crash.logger.warn.assert_any_call("Use \\connect to connect to one or more servers first.") def test_username_param(self): - with CrateShell(node.http_url, + with CrateShell(crate_hosts=[node.http_url], username='crate') as crash: self.assertEqual(crash.username, "crate") self.assertEqual(crash.connection.client.username, "crate") @@ -755,7 +755,7 @@ def test_ssl_params(self): ftouch(key_filename) ftouch(ca_cert_filename) - with CrateShell(node.http_url, + with CrateShell(crate_hosts=[node.http_url], verify_ssl=False, cert_file=cert_filename, key_file=key_filename, @@ -799,7 +799,7 @@ def test_ssl_params_wrong_permision_file(self): parser.parse_args(argv) def test_close_shell(self): - crash = CrateShell(node.http_url) + crash = CrateShell(crate_hosts=[node.http_url]) self.assertFalse(crash.is_closed()) self.assertTrue(crash.is_conn_available()) @@ -814,7 +814,7 @@ def test_close_shell(self): ctx.exception.message) def test_connect_info(self): - with CrateShell(node.http_url, + with CrateShell(crate_hosts=[node.http_url], username='crate', schema='test') as crash: self.assertEqual(crash.connect_info.user, "crate") @@ -863,7 +863,7 @@ def test_connect_info(self): @patch.object(CrateShell, "is_conn_available") def test_connect_info_not_available(self, is_conn_available): is_conn_available.return_value = False - with CrateShell(node.http_url, + with CrateShell(crate_hosts=[node.http_url], username='crate', schema='test') as crash: self.assertEqual(crash.connect_info.user, None) From 46401095024427ebcf3862be0d2c08b9677cd0c1 Mon Sep 17 00:00:00 2001 From: Vitaly Samigullin Date: Sun, 11 Feb 2024 09:10:54 +0100 Subject: [PATCH 6/6] chore: use canonical name for cratedb-toolkit package --- setup.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/setup.py b/setup.py index e09f4706..a5715c82 100644 --- a/setup.py +++ b/setup.py @@ -78,8 +78,7 @@ def read(path): test=[ 'crate[test]', 'zc.customdoctests<2', - # FIXME once tested and merged, fix the name - 'cratedb-toolkit[test] @ git+https://github.com/crate-workbench/cratedb-toolkit@main', + 'cratedb-toolkit[test]', ], devel=[ 'coverage<8',