From da34805e62cc2d61358752b6b7677aa543e81c6d Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Tue, 1 Dec 2020 00:09:51 -0500 Subject: [PATCH 01/17] Fail the integration tests on warnings or errors. Signed-off-by: Jakub Sobon --- test/integration/BUILD | 8 ++ test/integration/integration_test_fixtures.py | 31 +++++--- test/integration/nighthawk_test_server.py | 64 ++++++++++++++- .../integration/test_nighthawk_test_server.py | 77 +++++++++++++++++++ 4 files changed, 168 insertions(+), 12 deletions(-) create mode 100644 test/integration/test_nighthawk_test_server.py diff --git a/test/integration/BUILD b/test/integration/BUILD index 00765a29b..fbb5c5b57 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -9,6 +9,14 @@ licenses(["notice"]) # Apache 2 envoy_package() +py_test( + name = "test_nighthawk_test_server", + srcs = ["test_nighthawk_test_server.py"], + deps = [ + ":integration_test_base_lean", + ], +) + py_library( name = "integration_test_base", data = [ diff --git a/test/integration/integration_test_fixtures.py b/test/integration/integration_test_fixtures.py index e256d53e4..f9f8a3d35 100644 --- a/test/integration/integration_test_fixtures.py +++ b/test/integration/integration_test_fixtures.py @@ -123,8 +123,14 @@ def setUp(self): self.tag = "{timestamp}/{test_id}".format(timestamp=_TIMESTAMP, test_id=self._test_id) assert self._tryStartTestServers(), "Test server(s) failed to start" - def tearDown(self): - """Stop the server.""" + def tearDown(self, caplog): + """Stop the server. + + Fails the test if any warnings or errors were logged. + + Args: + caplog: The pytest `caplog` test fixture used to examine logged messages. + """ if self.grpc_service is not None: assert (self.grpc_service.stop() == 0) @@ -134,6 +140,11 @@ def tearDown(self): any_failed = True assert (not any_failed) + for when in ("setup", "call", "teardown"): + messages = [x.message for x in caplog.get_records(when) if x.levelno == logging.WARNING] + if messages: + pytest.fail("warning messages encountered during testing: {}".format(messages)) + def _tryStartTestServers(self): for i in range(self._backend_count): test_server = NighthawkTestServer(self._nighthawk_test_server_path, @@ -366,7 +377,7 @@ def server_config(): @pytest.fixture(params=determineIpVersionsFromEnvironment()) -def http_test_server_fixture(request, server_config): +def http_test_server_fixture(request, server_config, caplog): """Fixture for setting up a test environment with the stock http server configuration. Yields: @@ -375,11 +386,11 @@ def http_test_server_fixture(request, server_config): f = HttpIntegrationTestBase(request.param, server_config) f.setUp() yield f - f.tearDown() + f.tearDown(caplog) @pytest.fixture(params=determineIpVersionsFromEnvironment()) -def https_test_server_fixture(request, server_config): +def https_test_server_fixture(request, server_config, caplog): """Fixture for setting up a test environment with the stock https server configuration. Yields: @@ -388,11 +399,11 @@ def https_test_server_fixture(request, server_config): f = HttpsIntegrationTestBase(request.param, server_config) f.setUp() yield f - f.tearDown() + f.tearDown(caplog) @pytest.fixture(params=determineIpVersionsFromEnvironment()) -def multi_http_test_server_fixture(request, server_config): +def multi_http_test_server_fixture(request, server_config, caplog): """Fixture for setting up a test environment with multiple servers, using the stock http server configuration. Yields: @@ -401,11 +412,11 @@ def multi_http_test_server_fixture(request, server_config): f = MultiServerHttpIntegrationTestBase(request.param, server_config, backend_count=3) f.setUp() yield f - f.tearDown() + f.tearDown(caplog) @pytest.fixture(params=determineIpVersionsFromEnvironment()) -def multi_https_test_server_fixture(request, server_config): +def multi_https_test_server_fixture(request, server_config, caplog): """Fixture for setting up a test environment with multiple servers, using the stock https server configuration. Yields: @@ -414,4 +425,4 @@ def multi_https_test_server_fixture(request, server_config): f = MultiServerHttpsIntegrationTestBase(request.param, server_config, backend_count=3) f.setUp() yield f - f.tearDown() + f.tearDown(caplog) diff --git a/test/integration/nighthawk_test_server.py b/test/integration/nighthawk_test_server.py index 6a6deb849..537706c1a 100644 --- a/test/integration/nighthawk_test_server.py +++ b/test/integration/nighthawk_test_server.py @@ -40,6 +40,28 @@ def _substitute_yaml_values(runfiles_instance, obj, params): return obj +# A list of message pieces that should be ignored even if logged by the test +# server at a warning or an error severity. +# +# Other messages logged at either of these severities fail the test. +_TEST_SERVER_WARN_ERROR_IGNORE_LIST = frozenset([ + # TODO(#580) Remove these once our use of Envoy's API is updated to v3. + "Deprecated field: type envoy.api.v2.listener.Filter", + "Deprecated field: type envoy.config.filter.network.http_connection_manager.v2.HttpFilter", + "Configuration does not parse cleanly as v3", + + # TODO(mum4k): Identify these and file issues or add explanation as necessary. + "Unable to use runtime singleton for feature envoy.http.headermap.lazy_map_min_size", + "Using deprecated extension name 'envoy.http_connection_manager' for 'envoy.filters.network.http_connection_manager'.", + "Using deprecated extension name 'envoy.listener.tls_inspector' for 'envoy.filters.listener.tls_inspector'.", + "Using deprecated extension name 'envoy.router' for 'envoy.filters.http.router'.", + "there is no configured limit to the number of allowed active connections. Set a limit via the runtime key overload.global_downstream_max_connections", + + # Logged for normal termination, not really a warning. + "caught SIGTERM", +]) + + class TestServerBase(object): """Base class for running a server in a separate process. @@ -124,8 +146,14 @@ def _serverThreadRunner(self): logging.info("Test server popen() args: %s" % str.join(" ", args)) self._server_process = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE) stdout, stderr = self._server_process.communicate() - logging.debug(stdout.decode("utf-8")) - logging.debug(stderr.decode("utf-8")) + logging.info("Process stdout: %s", stdout.decode("utf-8")) + logging.info("Process stderr: %s", stderr.decode("utf-8")) + warnings, errors = _extractWarningsAndErrors(stdout.decode() + stderr.decode(), + _TEST_SERVER_WARN_ERROR_IGNORE_LIST) + if warnings: + [logging.warn("Process logged a warning: %s", w) for w in warnings] + if errors: + [logging.error("Process logged an error: %s", e) for e in errors] def fetchJsonFromAdminInterface(self, path): """Fetch and parse json from the admin interface. @@ -253,3 +281,35 @@ def getCliVersionString(self): stdout, stderr = process.communicate() assert process.wait() == 0 return stdout.decode("utf-8").strip() + + +def _extractWarningsAndErrors(process_output, ignore_list): + """Extract warnings and errors from the process_output. + + Args: + process_output: A string, the stdout or stderr after running a process. + ignore_list: A list of strings, message pieces to ignore. If a message that + was logged either at a warning or at an error severity contains one of + these message pieces, it will be excluded from the return values. + + Returns: + A tuple of two lists of strings, the first list contains the warnings found + in the process_output and the second list contains the errors found in the + process_output. + """ + warnings = [] + errors = [] + for line in process_output.split('\n'): + should_ignore = False + for ignore_message in ignore_list: + if ignore_message in line: + should_ignore = True + break + if should_ignore: + continue + + if "[warning]" in line: + warnings.append(line) + elif "[error]" in line: + errors.append(line) + return warnings, errors diff --git a/test/integration/test_nighthawk_test_server.py b/test/integration/test_nighthawk_test_server.py new file mode 100644 index 000000000..27b282456 --- /dev/null +++ b/test/integration/test_nighthawk_test_server.py @@ -0,0 +1,77 @@ +"""Contains unit tests for functions in nighthawk_test_server.py.""" + +import pytest + +from test.integration import nighthawk_test_server + + +def test_extractWarningsAndErrors_nothing_on_empty_output(): + """Test with an empty input.""" + warnings, errors = nighthawk_test_server._extractWarningsAndErrors("", []) + assert not warnings + assert not errors + + +def test_extractWarningsAndErrors_ignores_info_logs(): + """Test where the process output doesn't contain any warnings or errors.""" + process_output = """ + [2020-12-01 04:41:57.219][126][info][misc] Message. + """ + warnings, errors = nighthawk_test_server._extractWarningsAndErrors( + process_output, []) + assert not warnings + assert not errors + + +def test_extractWarningsAndErrors_extracts_a_warning(): + """Test where the process output contains a single warning.""" + process_output = "[2020-12-01 04:41:57.219][126][warning][misc] Message." + warnings, errors = nighthawk_test_server._extractWarningsAndErrors( + process_output, []) + assert warnings == ["[2020-12-01 04:41:57.219][126][warning][misc] Message."] + assert not errors + + +def test_extractWarningsAndErrors_extracts_an_error(): + """Test where the process output contains a single error.""" + process_output = "[2020-12-01 04:41:57.219][126][error][misc] Message." + warnings, errors = nighthawk_test_server._extractWarningsAndErrors( + process_output, []) + assert not warnings + assert errors == ["[2020-12-01 04:41:57.219][126][error][misc] Message."] + + +def test_extractWarningsAndErrors_extracts_multiple_messages(): + """Test where the process output contains multiple warnings and errors.""" + process_output = """[warning][misc] Warning1. +[error][misc] Error1. +[info][misc] Info1. +[error][runtime] Error2. +[warning][runtime] Warning2. + """ + warnings, errors = nighthawk_test_server._extractWarningsAndErrors( + process_output, []) + assert warnings == [ + "[warning][misc] Warning1.", "[warning][runtime] Warning2." + ] + assert errors == ["[error][misc] Error1.", "[error][runtime] Error2."] + + +def test_extractWarningsAndErrors_skips_messages_matching_ignore_list(): + """Test where the ignore list is used.""" + process_output = """[warning][misc] Warning1 foo. +[error][misc] Error1 bar. +[info][misc] Info1. +[error][runtime] Error2 baz. +[warning][runtime] Warning2 bar. + """ + + ignore_list = ["foo", "bar"] + warnings, errors = nighthawk_test_server._extractWarningsAndErrors( + process_output, ignore_list) + assert not warnings + assert errors == ["[error][runtime] Error2 baz."] + + +if __name__ == "__main__": + raise SystemExit(pytest.main([__file__])) From 0e3d4998c8f32428af2f5cb23cf38fc76003cd43 Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Tue, 1 Dec 2020 00:23:00 -0500 Subject: [PATCH 02/17] Autoformatting Python files. Signed-off-by: Jakub Sobon --- .../integration/test_nighthawk_test_server.py | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/test/integration/test_nighthawk_test_server.py b/test/integration/test_nighthawk_test_server.py index 27b282456..3de38992d 100644 --- a/test/integration/test_nighthawk_test_server.py +++ b/test/integration/test_nighthawk_test_server.py @@ -17,8 +17,7 @@ def test_extractWarningsAndErrors_ignores_info_logs(): process_output = """ [2020-12-01 04:41:57.219][126][info][misc] Message. """ - warnings, errors = nighthawk_test_server._extractWarningsAndErrors( - process_output, []) + warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, []) assert not warnings assert not errors @@ -26,8 +25,7 @@ def test_extractWarningsAndErrors_ignores_info_logs(): def test_extractWarningsAndErrors_extracts_a_warning(): """Test where the process output contains a single warning.""" process_output = "[2020-12-01 04:41:57.219][126][warning][misc] Message." - warnings, errors = nighthawk_test_server._extractWarningsAndErrors( - process_output, []) + warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, []) assert warnings == ["[2020-12-01 04:41:57.219][126][warning][misc] Message."] assert not errors @@ -35,8 +33,7 @@ def test_extractWarningsAndErrors_extracts_a_warning(): def test_extractWarningsAndErrors_extracts_an_error(): """Test where the process output contains a single error.""" process_output = "[2020-12-01 04:41:57.219][126][error][misc] Message." - warnings, errors = nighthawk_test_server._extractWarningsAndErrors( - process_output, []) + warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, []) assert not warnings assert errors == ["[2020-12-01 04:41:57.219][126][error][misc] Message."] @@ -49,11 +46,8 @@ def test_extractWarningsAndErrors_extracts_multiple_messages(): [error][runtime] Error2. [warning][runtime] Warning2. """ - warnings, errors = nighthawk_test_server._extractWarningsAndErrors( - process_output, []) - assert warnings == [ - "[warning][misc] Warning1.", "[warning][runtime] Warning2." - ] + warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, []) + assert warnings == ["[warning][misc] Warning1.", "[warning][runtime] Warning2."] assert errors == ["[error][misc] Error1.", "[error][runtime] Error2."] @@ -67,8 +61,7 @@ def test_extractWarningsAndErrors_skips_messages_matching_ignore_list(): """ ignore_list = ["foo", "bar"] - warnings, errors = nighthawk_test_server._extractWarningsAndErrors( - process_output, ignore_list) + warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, ignore_list) assert not warnings assert errors == ["[error][runtime] Error2 baz."] From 725e405bf33fd658170067176fc5efccaa375505 Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Tue, 1 Dec 2020 01:01:51 -0500 Subject: [PATCH 03/17] Adding the caplog fixture to the python benchmarks. Signed-off-by: Jakub Sobon --- benchmarks/envoy_proxy.py | 9 +++++---- test/integration/nighthawk_test_server.py | 3 ++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/benchmarks/envoy_proxy.py b/benchmarks/envoy_proxy.py index 06ce8bc08..f6cba311d 100644 --- a/benchmarks/envoy_proxy.py +++ b/benchmarks/envoy_proxy.py @@ -92,9 +92,9 @@ def setUp(self): port=proxy_server.server_port)) self.proxy_server = proxy_server - def tearDown(self): + def tearDown(self, caplog): """Tear down the proxy and test server. Assert that both exit succesfully.""" - super(InjectHttpProxyIntegrationTestBase, self).tearDown() + super(InjectHttpProxyIntegrationTestBase, self).tearDown(caplog) assert (self.proxy_server.stop() == 0) def getTestServerRootUri(self): @@ -106,7 +106,7 @@ def getTestServerRootUri(self): @pytest.fixture(params=determineIpVersionsFromEnvironment()) -def inject_envoy_http_proxy_fixture(request, server_config, proxy_config): +def inject_envoy_http_proxy_fixture(request, server_config, proxy_config, caplog): """Injects an Envoy proxy in front of the test server. NOTE: Depends on the proxy_config fixture, which must be explicitly imported @@ -116,10 +116,11 @@ def inject_envoy_http_proxy_fixture(request, server_config, proxy_config): request: supplies the ip version. server_config: path to the server configuration template. proxy_config: path to the proxy configuration template. + caplog: The pytest `caplog` test fixture used to examine logged messages. Yields: a successfully set up InjectHttpProxyIntegrationTestBase instance. """ fixture = InjectHttpProxyIntegrationTestBase(request.param, server_config, proxy_config) fixture.setUp() yield fixture - fixture.tearDown() + fixture.tearDown(caplog) diff --git a/test/integration/nighthawk_test_server.py b/test/integration/nighthawk_test_server.py index 537706c1a..e6f8d9f89 100644 --- a/test/integration/nighthawk_test_server.py +++ b/test/integration/nighthawk_test_server.py @@ -46,9 +46,10 @@ def _substitute_yaml_values(runfiles_instance, obj, params): # Other messages logged at either of these severities fail the test. _TEST_SERVER_WARN_ERROR_IGNORE_LIST = frozenset([ # TODO(#580) Remove these once our use of Envoy's API is updated to v3. + "Configuration does not parse cleanly as v3", + "Deprecated field: type envoy.api.v2.Cluster", "Deprecated field: type envoy.api.v2.listener.Filter", "Deprecated field: type envoy.config.filter.network.http_connection_manager.v2.HttpFilter", - "Configuration does not parse cleanly as v3", # TODO(mum4k): Identify these and file issues or add explanation as necessary. "Unable to use runtime singleton for feature envoy.http.headermap.lazy_map_min_size", From 89f83a76d950e0b371cfe96a0af4b9808881be0f Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Tue, 1 Dec 2020 22:40:21 -0500 Subject: [PATCH 04/17] Updating TODO to list the tracking issue. Signed-off-by: Jakub Sobon --- test/integration/nighthawk_test_server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/nighthawk_test_server.py b/test/integration/nighthawk_test_server.py index e6f8d9f89..248fb9b27 100644 --- a/test/integration/nighthawk_test_server.py +++ b/test/integration/nighthawk_test_server.py @@ -51,7 +51,7 @@ def _substitute_yaml_values(runfiles_instance, obj, params): "Deprecated field: type envoy.api.v2.listener.Filter", "Deprecated field: type envoy.config.filter.network.http_connection_manager.v2.HttpFilter", - # TODO(mum4k): Identify these and file issues or add explanation as necessary. + # TODO(#582): Identify these and file issues or add explanation as necessary. "Unable to use runtime singleton for feature envoy.http.headermap.lazy_map_min_size", "Using deprecated extension name 'envoy.http_connection_manager' for 'envoy.filters.network.http_connection_manager'.", "Using deprecated extension name 'envoy.listener.tls_inspector' for 'envoy.filters.listener.tls_inspector'.", From f70cc55bb5961afd7074908798ab7867dc463850 Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Tue, 1 Dec 2020 22:44:49 -0500 Subject: [PATCH 05/17] Moving tests for our test code into a unit_test subdirectory. Signed-off-by: Jakub Sobon --- test/integration/BUILD | 8 -------- test/integration/unit_tests/BUILD | 18 ++++++++++++++++++ .../test_nighthawk_test_server.py | 0 3 files changed, 18 insertions(+), 8 deletions(-) create mode 100644 test/integration/unit_tests/BUILD rename test/integration/{ => unit_tests}/test_nighthawk_test_server.py (100%) diff --git a/test/integration/BUILD b/test/integration/BUILD index fbb5c5b57..00765a29b 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -9,14 +9,6 @@ licenses(["notice"]) # Apache 2 envoy_package() -py_test( - name = "test_nighthawk_test_server", - srcs = ["test_nighthawk_test_server.py"], - deps = [ - ":integration_test_base_lean", - ], -) - py_library( name = "integration_test_base", data = [ diff --git a/test/integration/unit_tests/BUILD b/test/integration/unit_tests/BUILD new file mode 100644 index 000000000..4166f70c9 --- /dev/null +++ b/test/integration/unit_tests/BUILD @@ -0,0 +1,18 @@ +load("@rules_python//python:defs.bzl", "py_binary", "py_library") +load( + "@envoy//bazel:envoy_build_system.bzl", + "envoy_package", +) +load("@python_pip_deps//:requirements.bzl", "requirement") + +licenses(["notice"]) # Apache 2 + +envoy_package() + +py_test( + name = "test_nighthawk_test_server", + srcs = ["test_nighthawk_test_server.py"], + deps = [ + "//test/integration:integration_test_base_lean", + ], +) diff --git a/test/integration/test_nighthawk_test_server.py b/test/integration/unit_tests/test_nighthawk_test_server.py similarity index 100% rename from test/integration/test_nighthawk_test_server.py rename to test/integration/unit_tests/test_nighthawk_test_server.py From cf37f03f36c9f9a184e36681d094d4741aec46f3 Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Tue, 1 Dec 2020 22:48:33 -0500 Subject: [PATCH 06/17] Simplifying the unit_tests/BUILD file. Signed-off-by: Jakub Sobon --- test/integration/unit_tests/BUILD | 7 ------- 1 file changed, 7 deletions(-) diff --git a/test/integration/unit_tests/BUILD b/test/integration/unit_tests/BUILD index 4166f70c9..8da878e31 100644 --- a/test/integration/unit_tests/BUILD +++ b/test/integration/unit_tests/BUILD @@ -1,14 +1,7 @@ load("@rules_python//python:defs.bzl", "py_binary", "py_library") -load( - "@envoy//bazel:envoy_build_system.bzl", - "envoy_package", -) -load("@python_pip_deps//:requirements.bzl", "requirement") licenses(["notice"]) # Apache 2 -envoy_package() - py_test( name = "test_nighthawk_test_server", srcs = ["test_nighthawk_test_server.py"], From dc4327e027e871d4f9930e70b972d81b7df1be74 Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Tue, 1 Dec 2020 23:02:28 -0500 Subject: [PATCH 07/17] Also fail integration tests on ERRORs, not just WARNINGs. Signed-off-by: Jakub Sobon --- test/integration/integration_test_fixtures.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/test/integration/integration_test_fixtures.py b/test/integration/integration_test_fixtures.py index f9f8a3d35..b17072808 100644 --- a/test/integration/integration_test_fixtures.py +++ b/test/integration/integration_test_fixtures.py @@ -140,10 +140,14 @@ def tearDown(self, caplog): any_failed = True assert (not any_failed) + messages = [] for when in ("setup", "call", "teardown"): - messages = [x.message for x in caplog.get_records(when) if x.levelno == logging.WARNING] - if messages: - pytest.fail("warning messages encountered during testing: {}".format(messages)) + for message in caplog.get_records(when): + if x.levelno not in (logging.WARNING, logging.ERROR): + continue + messages.append(message) + if messages: + pytest.fail("warning messages encountered during testing: {}".format(messages)) def _tryStartTestServers(self): for i in range(self._backend_count): From 9133b9f984320ac68f505dff4f78690356cf085a Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Tue, 1 Dec 2020 23:26:44 -0500 Subject: [PATCH 08/17] Fixing nameerror in the teardown method. Signed-off-by: Jakub Sobon --- test/integration/integration_test_fixtures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/integration_test_fixtures.py b/test/integration/integration_test_fixtures.py index b17072808..dce6724c6 100644 --- a/test/integration/integration_test_fixtures.py +++ b/test/integration/integration_test_fixtures.py @@ -143,7 +143,7 @@ def tearDown(self, caplog): messages = [] for when in ("setup", "call", "teardown"): for message in caplog.get_records(when): - if x.levelno not in (logging.WARNING, logging.ERROR): + if message.levelno not in (logging.WARNING, logging.ERROR): continue messages.append(message) if messages: From 61821dbaeca4c7cddf6923cc38f0f693b760ca37 Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Wed, 2 Dec 2020 15:07:16 -0500 Subject: [PATCH 09/17] Renaming a variable to a more readable form. Signed-off-by: Jakub Sobon --- test/integration/integration_test_fixtures.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/integration/integration_test_fixtures.py b/test/integration/integration_test_fixtures.py index dce6724c6..d88c2915a 100644 --- a/test/integration/integration_test_fixtures.py +++ b/test/integration/integration_test_fixtures.py @@ -140,14 +140,14 @@ def tearDown(self, caplog): any_failed = True assert (not any_failed) - messages = [] + warnings_and_errors = [] for when in ("setup", "call", "teardown"): for message in caplog.get_records(when): if message.levelno not in (logging.WARNING, logging.ERROR): continue - messages.append(message) - if messages: - pytest.fail("warning messages encountered during testing: {}".format(messages)) + warnings_and_errors.append(message) + if warnings_and_errors: + pytest.fail("warnings or errors encountered during testing: {}".format(warnings_and_errors)) def _tryStartTestServers(self): for i in range(self._backend_count): From 9173ba3b277f0eef257773175085353d3bd6350c Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Wed, 2 Dec 2020 21:40:13 -0500 Subject: [PATCH 10/17] Changing variable `messages` to a more readable name. Signed-off-by: Jakub Sobon --- test/integration/integration_test_fixtures.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/integration/integration_test_fixtures.py b/test/integration/integration_test_fixtures.py index d88c2915a..3a10866e2 100644 --- a/test/integration/integration_test_fixtures.py +++ b/test/integration/integration_test_fixtures.py @@ -142,12 +142,12 @@ def tearDown(self, caplog): warnings_and_errors = [] for when in ("setup", "call", "teardown"): - for message in caplog.get_records(when): - if message.levelno not in (logging.WARNING, logging.ERROR): + for record in caplog.get_records(when): + if record.levelno not in (logging.WARNING, logging.ERROR): continue - warnings_and_errors.append(message) + warnings_and_errors.append(record.message) if warnings_and_errors: - pytest.fail("warnings or errors encountered during testing: {}".format(warnings_and_errors)) + pytest.fail("warnings or errors encountered during testing:\n{}".format(warnings_and_errors)) def _tryStartTestServers(self): for i in range(self._backend_count): From 420c2b64f3ef7e0a118df29f4cfb4b56ed5f6a64 Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Wed, 2 Dec 2020 21:41:00 -0500 Subject: [PATCH 11/17] Adjusting the list of ignored warnings. Removing all those related to the v2 bootstraps that are no longer present. Signed-off-by: Jakub Sobon --- test/integration/nighthawk_test_server.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/test/integration/nighthawk_test_server.py b/test/integration/nighthawk_test_server.py index 248fb9b27..7088553fd 100644 --- a/test/integration/nighthawk_test_server.py +++ b/test/integration/nighthawk_test_server.py @@ -45,19 +45,16 @@ def _substitute_yaml_values(runfiles_instance, obj, params): # # Other messages logged at either of these severities fail the test. _TEST_SERVER_WARN_ERROR_IGNORE_LIST = frozenset([ - # TODO(#580) Remove these once our use of Envoy's API is updated to v3. - "Configuration does not parse cleanly as v3", - "Deprecated field: type envoy.api.v2.Cluster", - "Deprecated field: type envoy.api.v2.listener.Filter", - "Deprecated field: type envoy.config.filter.network.http_connection_manager.v2.HttpFilter", - # TODO(#582): Identify these and file issues or add explanation as necessary. "Unable to use runtime singleton for feature envoy.http.headermap.lazy_map_min_size", - "Using deprecated extension name 'envoy.http_connection_manager' for 'envoy.filters.network.http_connection_manager'.", "Using deprecated extension name 'envoy.listener.tls_inspector' for 'envoy.filters.listener.tls_inspector'.", - "Using deprecated extension name 'envoy.router' for 'envoy.filters.http.router'.", "there is no configured limit to the number of allowed active connections. Set a limit via the runtime key overload.global_downstream_max_connections", + # A few of our filters use the same typed configuration, specifically + # 'test-server', 'time-tracking' and 'dynamic-delay'. + # For now this is by design. + "Double registration for type: 'nighthawk.server.ResponseOptions'", + # Logged for normal termination, not really a warning. "caught SIGTERM", ]) From f2418e9c0ef5284129334fd1bd00d6643017e40e Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Tue, 8 Dec 2020 01:52:54 -0500 Subject: [PATCH 12/17] Ignore messages can now be specified per test cases. Test cases are matched using regular expressions. Signed-off-by: Jakub Sobon --- test/integration/nighthawk_test_server.py | 104 +++++++++++++----- .../unit_tests/test_nighthawk_test_server.py | 75 +++++++++++-- 2 files changed, 144 insertions(+), 35 deletions(-) diff --git a/test/integration/nighthawk_test_server.py b/test/integration/nighthawk_test_server.py index 5c299ac17..cfb9645dd 100644 --- a/test/integration/nighthawk_test_server.py +++ b/test/integration/nighthawk_test_server.py @@ -1,14 +1,16 @@ """Contains the NighthawkTestServer class, which wraps the nighthawk_test_servern binary.""" +import collections import http.client import json import logging import os +import random +import re +import requests import socket import subprocess import sys -import random -import requests import tempfile import threading import time @@ -40,23 +42,46 @@ def _substitute_yaml_values(runfiles_instance, obj, params): return obj -# A list of message pieces that should be ignored even if logged by the test -# server at a warning or an error severity. +class _TestCaseWarnErrorIgnoreList(collections.namedtuple("_TestCaseWarnErrorIgnoreList", "test_case_regexp ignore_list")): + """Maps test case names to messages that should be ignored in the test server logs. + + If the name of the currently executing test case matches the test_case_regexp, + any messages logged by the test server as either a WARNING or an ERROR that + will be checked against the ignore_list. Matching messages will be ignored, + any unmatched messages will fail the test case. + + Attributes: + test_case_regexp: A compiled regular expression as returned by re.compile(), + the regexp that will be used to match test case names. + ignore_list: A tuple of strings, messages to ignore for matching test cases. + """ + +# A list of _TestCaseWarnErrorIgnoreList instances, message pieces that should +# be ignored even if logged by the test server at a warning or an error +# severity. # -# Other messages logged at either of these severities fail the test. +# This list is processed in the order as defined, if multiple test_case_regexp +# entries match the current test case name, all the corresponding ignore lists +# will be used. +# +# Non matching messages logged at either of these severities fail the test. _TEST_SERVER_WARN_ERROR_IGNORE_LIST = frozenset([ - # TODO(#582): Identify these and file issues or add explanation as necessary. - "Unable to use runtime singleton for feature envoy.http.headermap.lazy_map_min_size", - "Using deprecated extension name 'envoy.listener.tls_inspector' for 'envoy.filters.listener.tls_inspector'.", - "there is no configured limit to the number of allowed active connections. Set a limit via the runtime key overload.global_downstream_max_connections", - - # A few of our filters use the same typed configuration, specifically - # 'test-server', 'time-tracking' and 'dynamic-delay'. - # For now this is by design. - "Double registration for type: 'nighthawk.server.ResponseOptions'", - - # Logged for normal termination, not really a warning. - "caught SIGTERM", + # A catch-all that applies to all remaining test cases. + _TestCaseWarnErrorIgnoreList(re.compile('.*'), ( + # TODO(#582): Identify these and file issues or add explanation as necessary. + "Unable to use runtime singleton for feature envoy.http.headermap.lazy_map_min_size", + "Using deprecated extension name 'envoy.listener.tls_inspector' for 'envoy.filters.listener.tls_inspector'.", + "there is no configured limit to the number of allowed active connections. Set a limit via the runtime key overload.global_downstream_max_connections", + + # A few of our filters use the same typed configuration, specifically + # 'test-server', 'time-tracking' and 'dynamic-delay'. + # For now this is by design. + "Double registration for type: 'nighthawk.server.ResponseOptions'", + + # Logged for normal termination, not really a warning. + "caught SIGTERM", + ), + ), ]) @@ -159,6 +184,7 @@ def _serverThreadRunner(self): logging.info("Process stdout: %s", stdout.decode("utf-8")) logging.info("Process stderr: %s", stderr.decode("utf-8")) warnings, errors = _extractWarningsAndErrors(stdout.decode() + stderr.decode(), + "fake_test_case_name", _TEST_SERVER_WARN_ERROR_IGNORE_LIST) if warnings: [logging.warn("Process logged a warning: %s", w) for w in warnings] @@ -301,14 +327,39 @@ def getCliVersionString(self): return stdout.decode("utf-8").strip() -def _extractWarningsAndErrors(process_output, ignore_list): +def _matchesAnyIgnoreListEntry(line, test_case_name, ignore_list): + """Determines if the line matches any of the ignore list entries for this test case. + + Args: + line: A string, the logged line. + test_case_name: A string, name of the currently executed test case. + ignore_list: A list of _TestCaseWarnErrorIgnoreList instances, the ignore + lists to match against. + + Returns: + A boolean, True if the logged line matches any of the ignore list entries, + False otherwise. + """ + for test_case_ignore_list in ignore_list: + if not test_case_ignore_list.test_case_regexp.match(test_case_name): + continue + for ignore_message in test_case_ignore_list.ignore_list: + if ignore_message in line: + return True + return False + + +def _extractWarningsAndErrors(process_output, test_case_name, ignore_list): """Extract warnings and errors from the process_output. Args: process_output: A string, the stdout or stderr after running a process. - ignore_list: A list of strings, message pieces to ignore. If a message that - was logged either at a warning or at an error severity contains one of - these message pieces, it will be excluded from the return values. + test_case_name: A string, the name of the current test case. + ignore_list: A list of _TestCaseWarnErrorIgnoreList instances, message + pieces to ignore. If a message that was logged either at a warning or at + an error severity contains one of these message pieces and should be + ignored for the current test case, it will be excluded from the return + values. Returns: A tuple of two lists of strings, the first list contains the warnings found @@ -318,12 +369,11 @@ def _extractWarningsAndErrors(process_output, ignore_list): warnings = [] errors = [] for line in process_output.split('\n'): - should_ignore = False - for ignore_message in ignore_list: - if ignore_message in line: - should_ignore = True - break - if should_ignore: + # Optimization - no need to examine lines that aren't errors or warnings. + if "[warning]" not in line and "[error]" not in line: + continue + + if _matchesAnyIgnoreListEntry(line, test_case_name, ignore_list): continue if "[warning]" in line: diff --git a/test/integration/unit_tests/test_nighthawk_test_server.py b/test/integration/unit_tests/test_nighthawk_test_server.py index 3de38992d..867baf30f 100644 --- a/test/integration/unit_tests/test_nighthawk_test_server.py +++ b/test/integration/unit_tests/test_nighthawk_test_server.py @@ -1,13 +1,14 @@ """Contains unit tests for functions in nighthawk_test_server.py.""" import pytest +import re from test.integration import nighthawk_test_server def test_extractWarningsAndErrors_nothing_on_empty_output(): """Test with an empty input.""" - warnings, errors = nighthawk_test_server._extractWarningsAndErrors("", []) + warnings, errors = nighthawk_test_server._extractWarningsAndErrors("", "test_case", []) assert not warnings assert not errors @@ -17,7 +18,7 @@ def test_extractWarningsAndErrors_ignores_info_logs(): process_output = """ [2020-12-01 04:41:57.219][126][info][misc] Message. """ - warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, []) + warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case", []) assert not warnings assert not errors @@ -25,7 +26,7 @@ def test_extractWarningsAndErrors_ignores_info_logs(): def test_extractWarningsAndErrors_extracts_a_warning(): """Test where the process output contains a single warning.""" process_output = "[2020-12-01 04:41:57.219][126][warning][misc] Message." - warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, []) + warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case", []) assert warnings == ["[2020-12-01 04:41:57.219][126][warning][misc] Message."] assert not errors @@ -33,7 +34,7 @@ def test_extractWarningsAndErrors_extracts_a_warning(): def test_extractWarningsAndErrors_extracts_an_error(): """Test where the process output contains a single error.""" process_output = "[2020-12-01 04:41:57.219][126][error][misc] Message." - warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, []) + warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case", []) assert not warnings assert errors == ["[2020-12-01 04:41:57.219][126][error][misc] Message."] @@ -46,12 +47,12 @@ def test_extractWarningsAndErrors_extracts_multiple_messages(): [error][runtime] Error2. [warning][runtime] Warning2. """ - warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, []) + warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case", []) assert warnings == ["[warning][misc] Warning1.", "[warning][runtime] Warning2."] assert errors == ["[error][misc] Error1.", "[error][runtime] Error2."] -def test_extractWarningsAndErrors_skips_messages_matching_ignore_list(): +def test_extractWarningsAndErrors_skips_messages_matching_ignore_list_when_test_case_matched_with_a_glob(): """Test where the ignore list is used.""" process_output = """[warning][misc] Warning1 foo. [error][misc] Error1 bar. @@ -60,8 +61,66 @@ def test_extractWarningsAndErrors_skips_messages_matching_ignore_list(): [warning][runtime] Warning2 bar. """ - ignore_list = ["foo", "bar"] - warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, ignore_list) + ignore_list = [ + nighthawk_test_server._TestCaseWarnErrorIgnoreList(re.compile(".*"), ("foo", "bar")), + ] + warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case", ignore_list) + assert not warnings + assert errors == ["[error][runtime] Error2 baz."] + + +def test_extractWarningsAndErrors_skips_messages_matching_ignore_list_when_test_case_matched_exactly(): + """Test where the ignore list is used.""" + process_output = """[warning][misc] Warning1 foo. +[error][misc] Error1 bar. +[info][misc] Info1. +[error][runtime] Error2 baz. +[warning][runtime] Warning2 bar. + """ + + ignore_list = [ + nighthawk_test_server._TestCaseWarnErrorIgnoreList(re.compile("test_case"), ("foo", "bar")), + ] + warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case", ignore_list) + assert not warnings + assert errors == ["[error][runtime] Error2 baz."] + + +def test_extractWarningsAndErrors_does_not_apply_ignore_list_for_non_matching_test_case_name(): + """Test where the ignore list is used.""" + process_output = """[warning][misc] Warning1 foo. +[error][misc] Error1 bar. +[info][misc] Info1. +[error][runtime] Error2 baz. +[warning][runtime] Warning2 bar. + """ + + ignore_list = [ + nighthawk_test_server._TestCaseWarnErrorIgnoreList(re.compile("test_case1"), ("foo", )), + nighthawk_test_server._TestCaseWarnErrorIgnoreList(re.compile("test_case2"), ("bar", )), + ] + warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case1", ignore_list) + assert warnings == ["[warning][runtime] Warning2 bar."] + assert errors == [ + "[error][misc] Error1 bar.", + "[error][runtime] Error2 baz.", + ] + + +def test_extractWarningsAndErrors_applies_all_matching_ignore_lists(): + """Test where the ignore list is used.""" + process_output = """[warning][misc] Warning1 foo. +[error][misc] Error1 bar. +[info][misc] Info1. +[error][runtime] Error2 baz. +[warning][runtime] Warning2 bar. + """ + + ignore_list = [ + nighthawk_test_server._TestCaseWarnErrorIgnoreList(re.compile("test_case1"), ("foo", )), + nighthawk_test_server._TestCaseWarnErrorIgnoreList(re.compile(".*"), ("bar", )), + ] + warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case1", ignore_list) assert not warnings assert errors == ["[error][runtime] Error2 baz."] From a3569d9bbea1e757f7270b4a9a280052e57cce32 Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Tue, 8 Dec 2020 02:26:23 -0500 Subject: [PATCH 13/17] Passing the pytest request fixture down to the NighthawkTestServer. Signed-off-by: Jakub Sobon --- benchmarks/envoy_proxy.py | 13 ++-- test/integration/integration_test_fixtures.py | 68 ++++++++++--------- test/integration/nighthawk_test_server.py | 10 ++- 3 files changed, 54 insertions(+), 37 deletions(-) diff --git a/benchmarks/envoy_proxy.py b/benchmarks/envoy_proxy.py index f6cba311d..4f6834685 100644 --- a/benchmarks/envoy_proxy.py +++ b/benchmarks/envoy_proxy.py @@ -28,13 +28,15 @@ class EnvoyProxyServer(NighthawkTestServer): See InjectHttpProxyIntegrationTestBase below for usage. """ - def __init__(self, config_template_path, server_ip, ip_version, parameters=dict(), tag=""): + def __init__(self, config_template_path, server_ip, ip_version, request, parameters=dict(), tag=""): """Initialize an EnvoyProxyServer instance. Arguments: config_template_path: Configuration template for the proxy. server_ip: IP address for the proxy to use. ip_version: IP version that the proxy should use when listening. + request: The pytest `request` test fixture used to determine information + about the currently executing test case. parameters: Dictionary. Supply this to provide template parameter replacement values (optional). tag: String. Supply this to get recognizeable output locations (optional). """ @@ -43,6 +45,7 @@ def __init__(self, config_template_path, server_ip, ip_version, parameters=dict( config_template_path, server_ip, ip_version, + request, parameters=parameters, tag=tag) self.docker_image = os.getenv("ENVOY_DOCKER_IMAGE_TO_TEST", "") @@ -61,15 +64,16 @@ class InjectHttpProxyIntegrationTestBase(HttpIntegrationTestBase): which directs traffic to that. Both will be listing for plain http traffic. """ - def __init__(self, ip_version, server_config, proxy_config): + def __init__(self, request, server_config, proxy_config): """Initialize an InjectHttpProxyIntegrationTestBase. Arguments: - ip_version: Use ipv4 or ipv6 + request: The pytest `request` test fixture used to determine information + about the currently executing test case. server_config: Path to the server configuration. proxy_config: Path to the proxy configuration. """ - super(InjectHttpProxyIntegrationTestBase, self).__init__(ip_version, server_config) + super(InjectHttpProxyIntegrationTestBase, self).__init__(request, server_config) self._proxy_config = proxy_config def setUp(self): @@ -85,6 +89,7 @@ def setUp(self): proxy_server = EnvoyProxyServer(self._proxy_config, self.server_ip, self.ip_version, + self.request, parameters=self.parameters, tag=self.tag) assert (proxy_server.start()) diff --git a/test/integration/integration_test_fixtures.py b/test/integration/integration_test_fixtures.py index 14396bcc8..0988342e5 100644 --- a/test/integration/integration_test_fixtures.py +++ b/test/integration/integration_test_fixtures.py @@ -54,30 +54,35 @@ class IntegrationTestBase(): work when there is only one test server. This class will be refactored (https://github.com/envoyproxy/nighthawk/issues/258). + + Attributes: + ip_version: IP version that the proxy should use when listening. + server_ip: string containing the server ip that will be used to listen + tag: String. Supply this to get recognizeable output locations. + parameters: Dictionary. Supply this to provide template parameter replacement values. + grpc_service: NighthawkGrpcService instance or None. Set by startNighthawkGrpcService(). + test_server: NighthawkTestServer instance, set during setUp(). + nighthawk_client_path: String, path to the nighthawk_client binary. + request: The pytest `request` test fixture used to determine information + about the currently executing test case. """ - def __init__(self, ip_version, server_config, backend_count=1, bootstrap_version_arg=None): + def __init__(self, request, server_config, backend_count=1, bootstrap_version_arg=None): """Initialize the IntegrationTestBase instance. Args: ip_version: a single IP mode that this instance will test: IpVersion.IPV4 or IpVersion.IPV6 + request: The pytest `request` test fixture used to determine information + about the currently executing test case. server_config: path to the server configuration backend_count: number of Nighthawk Test Server backends to run, to allow testing MultiTarget mode bootstrap_version_arg: An optional int, specify a bootstrap cli argument value for the test server binary. If None is specified, no bootstrap cli argment will be passed. - - Attributes: - ip_version: IP version that the proxy should use when listening. - server_ip: string containing the server ip that will be used to listen - tag: String. Supply this to get recognizeable output locations. - parameters: Dictionary. Supply this to provide template parameter replacement values. - grpc_service: NighthawkGrpcService instance or None. Set by startNighthawkGrpcService(). - test_server: NighthawkTestServer instance, set during setUp(). - nighthawk_client_path: String, path to the nighthawk_client binary. """ super(IntegrationTestBase, self).__init__() - assert ip_version != IpVersion.UNKNOWN - self.ip_version = ip_version - self.server_ip = "::" if ip_version == IpVersion.IPV6 else "0.0.0.0" + self.request = request + self.ip_version = request.param + assert self.ip_version != IpVersion.UNKNOWN + self.server_ip = "::" if self.ip_version == IpVersion.IPV6 else "0.0.0.0" self.server_ip = os.getenv("TEST_SERVER_EXTERNAL_IP", self.server_ip) self.tag = "" self.parameters = {} @@ -88,7 +93,7 @@ def __init__(self, ip_version, server_config, backend_count=1, bootstrap_version self._nighthawk_test_config_path = server_config self._nighthawk_service_path = "nighthawk_service" self._nighthawk_output_transform_path = "nighthawk_output_transform" - self._socket_type = socket.AF_INET6 if ip_version == IpVersion.IPV6 else socket.AF_INET + self._socket_type = socket.AF_INET6 if self.ip_version == IpVersion.IPV6 else socket.AF_INET self._test_servers = [] self._backend_count = backend_count self._test_id = "" @@ -158,6 +163,7 @@ def _tryStartTestServers(self): self._nighthawk_test_config_path, self.server_ip, self.ip_version, + self.request, parameters=self.parameters, tag=self.tag, bootstrap_version_arg=self._bootstrap_version_arg) @@ -307,9 +313,9 @@ class HttpIntegrationTestBase(IntegrationTestBase): by pytest. """ - def __init__(self, ip_version, server_config): + def __init__(self, request, server_config): """See base class.""" - super(HttpIntegrationTestBase, self).__init__(ip_version, server_config) + super(HttpIntegrationTestBase, self).__init__(request, server_config) def getTestServerRootUri(self): """See base class.""" @@ -324,10 +330,10 @@ class HttpIntegrationTestBaseWithEnvoyDeprecatedV2Bootstrap(IntegrationTestBase) by pytest. """ - def __init__(self, ip_version, server_config): + def __init__(self, request, server_config): """See base class.""" super(HttpIntegrationTestBaseWithEnvoyDeprecatedV2Bootstrap, - self).__init__(ip_version, server_config, bootstrap_version_arg=2) + self).__init__(request, server_config, bootstrap_version_arg=2) def getTestServerRootUri(self): """See base class.""" @@ -338,9 +344,9 @@ def getTestServerRootUri(self): class MultiServerHttpIntegrationTestBase(IntegrationTestBase): """Base for running plain http tests against multiple Nighthawk test servers.""" - def __init__(self, ip_version, server_config, backend_count): + def __init__(self, request, server_config, backend_count): """See base class.""" - super(MultiServerHttpIntegrationTestBase, self).__init__(ip_version, server_config, + super(MultiServerHttpIntegrationTestBase, self).__init__(request, server_config, backend_count) def getTestServerRootUri(self): @@ -355,9 +361,9 @@ def getAllTestServerRootUris(self): class HttpsIntegrationTestBase(IntegrationTestBase): """Base for https tests against the Nighthawk test server.""" - def __init__(self, ip_version, server_config): + def __init__(self, request, server_config): """See base class.""" - super(HttpsIntegrationTestBase, self).__init__(ip_version, server_config) + super(HttpsIntegrationTestBase, self).__init__(request, server_config) def getTestServerRootUri(self): """See base class.""" @@ -367,9 +373,9 @@ def getTestServerRootUri(self): class SniIntegrationTestBase(HttpsIntegrationTestBase): """Base for https/sni tests against the Nighthawk test server.""" - def __init__(self, ip_version, server_config): + def __init__(self, request, server_config): """See base class.""" - super(SniIntegrationTestBase, self).__init__(ip_version, server_config) + super(SniIntegrationTestBase, self).__init__(request, server_config) def getTestServerRootUri(self): """See base class.""" @@ -379,9 +385,9 @@ def getTestServerRootUri(self): class MultiServerHttpsIntegrationTestBase(IntegrationTestBase): """Base for https tests against multiple Nighthawk test servers.""" - def __init__(self, ip_version, server_config, backend_count): + def __init__(self, request, server_config, backend_count): """See base class.""" - super(MultiServerHttpsIntegrationTestBase, self).__init__(ip_version, server_config, + super(MultiServerHttpsIntegrationTestBase, self).__init__(request, server_config, backend_count) def getTestServerRootUri(self): @@ -410,7 +416,7 @@ def http_test_server_fixture(request, server_config, caplog): Yields: HttpIntegrationTestBase: A fully set up instance. Tear down will happen automatically. """ - f = HttpIntegrationTestBase(request.param, server_config) + f = HttpIntegrationTestBase(request, server_config) f.setUp() yield f f.tearDown(caplog) @@ -423,7 +429,7 @@ def http_test_server_fixture_envoy_deprecated_v2_api(request, server_config, cap Yields: HttpIntegrationTestBaseWithEnvoyDeprecatedV2Bootstrap: A fully set up instance. Tear down will happen automatically. """ - f = HttpIntegrationTestBaseWithEnvoyDeprecatedV2Bootstrap(request.param, server_config) + f = HttpIntegrationTestBaseWithEnvoyDeprecatedV2Bootstrap(request, server_config) f.setUp() yield f f.tearDown(caplog) @@ -436,7 +442,7 @@ def https_test_server_fixture(request, server_config, caplog): Yields: HttpsIntegrationTestBase: A fully set up instance. Tear down will happen automatically. """ - f = HttpsIntegrationTestBase(request.param, server_config) + f = HttpsIntegrationTestBase(request, server_config) f.setUp() yield f f.tearDown(caplog) @@ -449,7 +455,7 @@ def multi_http_test_server_fixture(request, server_config, caplog): Yields: MultiServerHttpIntegrationTestBase: A fully set up instance. Tear down will happen automatically. """ - f = MultiServerHttpIntegrationTestBase(request.param, server_config, backend_count=3) + f = MultiServerHttpIntegrationTestBase(request, server_config, backend_count=3) f.setUp() yield f f.tearDown(caplog) @@ -462,7 +468,7 @@ def multi_https_test_server_fixture(request, server_config, caplog): Yields: MultiServerHttpsIntegrationTestBase: A fully set up instance. Tear down will happen automatically. """ - f = MultiServerHttpsIntegrationTestBase(request.param, server_config, backend_count=3) + f = MultiServerHttpsIntegrationTestBase(request, server_config, backend_count=3) f.setUp() yield f f.tearDown(caplog) diff --git a/test/integration/nighthawk_test_server.py b/test/integration/nighthawk_test_server.py index cfb9645dd..8f32a1974 100644 --- a/test/integration/nighthawk_test_server.py +++ b/test/integration/nighthawk_test_server.py @@ -66,7 +66,7 @@ class _TestCaseWarnErrorIgnoreList(collections.namedtuple("_TestCaseWarnErrorIgn # # Non matching messages logged at either of these severities fail the test. _TEST_SERVER_WARN_ERROR_IGNORE_LIST = frozenset([ - # A catch-all that applies to all remaining test cases. + # A catch-all that applies to all remaining test cases. _TestCaseWarnErrorIgnoreList(re.compile('.*'), ( # TODO(#582): Identify these and file issues or add explanation as necessary. "Unable to use runtime singleton for feature envoy.http.headermap.lazy_map_min_size", @@ -101,6 +101,7 @@ def __init__(self, config_template_path, server_ip, ip_version, + request, server_binary_config_path_arg, parameters, tag, @@ -112,6 +113,7 @@ def __init__(self, config_template_path (str): specify the path to the test server configuration template. server_ip (str): Specify the ip address the test server should use to listen for traffic. ip_version (IPAddress): Specify the ip version the server should use to listen for traffic. + request: The pytest `request` fixture used to determin information about the currently executed test. server_binary_config_path_arg (str): Specify the name of the CLI argument the test server binary uses to accept a configuration path. parameters (dict): Supply to provide configuration template parameter replacement values. tag (str): Supply to get recognizeable output locations. @@ -137,6 +139,7 @@ def __init__(self, self._server_binary_config_path_arg = server_binary_config_path_arg self._bootstrap_version_arg = bootstrap_version_arg self._prepareForExecution() + self._request = request def _prepareForExecution(self): runfiles_instance = runfiles.Create() @@ -184,7 +187,7 @@ def _serverThreadRunner(self): logging.info("Process stdout: %s", stdout.decode("utf-8")) logging.info("Process stderr: %s", stderr.decode("utf-8")) warnings, errors = _extractWarningsAndErrors(stdout.decode() + stderr.decode(), - "fake_test_case_name", + self._request.node.name, _TEST_SERVER_WARN_ERROR_IGNORE_LIST) if warnings: [logging.warn("Process logged a warning: %s", w) for w in warnings] @@ -291,6 +294,7 @@ def __init__(self, config_template_path, server_ip, ip_version, + request, parameters=dict(), tag="", bootstrap_version_arg=None): @@ -301,6 +305,7 @@ def __init__(self, config_template_path (String): Path to the nighthawk test server configuration template. server_ip (String): Ip address for the server to use when listening. ip_version (IPVersion): IPVersion enum member indicating the ip version that the server should use when listening. + request: The pytest `request` fixture used to determin information about the currently executed test. parameters (dictionary, optional): Directionary with replacement values for substition purposes in the server configuration template. Defaults to dict(). tag (str, optional): Tags. Supply this to get recognizeable output locations. Defaults to "". bootstrap_version_arg (String, optional): Specify a cli argument value for --bootstrap-version when running the server. @@ -309,6 +314,7 @@ def __init__(self, config_template_path, server_ip, ip_version, + request, "--config-path", parameters, tag, From 58c24d93d8ee57ee95e7661705690adcdf83203e Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Tue, 8 Dec 2020 02:30:42 -0500 Subject: [PATCH 14/17] Enabling a single test case to emit Envoy v2 API deprecation warnings. Signed-off-by: Jakub Sobon --- test/integration/nighthawk_test_server.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/test/integration/nighthawk_test_server.py b/test/integration/nighthawk_test_server.py index 8f32a1974..bdc792c23 100644 --- a/test/integration/nighthawk_test_server.py +++ b/test/integration/nighthawk_test_server.py @@ -66,7 +66,18 @@ class _TestCaseWarnErrorIgnoreList(collections.namedtuple("_TestCaseWarnErrorIgn # # Non matching messages logged at either of these severities fail the test. _TEST_SERVER_WARN_ERROR_IGNORE_LIST = frozenset([ - # A catch-all that applies to all remaining test cases. + # This test case purposefuly uses the deprecated Envoy v2 API which emits + # the following warnings. + _TestCaseWarnErrorIgnoreList(re.compile('test_nighthawk_test_server_envoy_deprecated_v2_api'), ( + "Configuration does not parse cleanly as v3. v2 configuration is deprecated", + "Deprecated field: type envoy.api.v2.listener.Filter", + "Deprecated field: type envoy.config.filter.network.http_connection_manager.v2.HttpFilter", + "Using deprecated extension name 'envoy.http_connection_manager'", + "Using deprecated extension name 'envoy.router'", + ), + ), + + # A catch-all that applies to all remaining test cases. _TestCaseWarnErrorIgnoreList(re.compile('.*'), ( # TODO(#582): Identify these and file issues or add explanation as necessary. "Unable to use runtime singleton for feature envoy.http.headermap.lazy_map_min_size", From 51c9792a437c1821f79863356341906267d2b927 Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Tue, 8 Dec 2020 02:31:49 -0500 Subject: [PATCH 15/17] Autoformatting Python files. Signed-off-by: Jakub Sobon --- benchmarks/envoy_proxy.py | 8 ++- test/integration/integration_test_fixtures.py | 6 +-- test/integration/nighthawk_test_server.py | 52 +++++++++++-------- .../unit_tests/test_nighthawk_test_server.py | 38 +++++++++----- 4 files changed, 62 insertions(+), 42 deletions(-) diff --git a/benchmarks/envoy_proxy.py b/benchmarks/envoy_proxy.py index 4f6834685..45486bda1 100644 --- a/benchmarks/envoy_proxy.py +++ b/benchmarks/envoy_proxy.py @@ -28,7 +28,13 @@ class EnvoyProxyServer(NighthawkTestServer): See InjectHttpProxyIntegrationTestBase below for usage. """ - def __init__(self, config_template_path, server_ip, ip_version, request, parameters=dict(), tag=""): + def __init__(self, + config_template_path, + server_ip, + ip_version, + request, + parameters=dict(), + tag=""): """Initialize an EnvoyProxyServer instance. Arguments: diff --git a/test/integration/integration_test_fixtures.py b/test/integration/integration_test_fixtures.py index 0988342e5..8e29a1940 100644 --- a/test/integration/integration_test_fixtures.py +++ b/test/integration/integration_test_fixtures.py @@ -346,8 +346,7 @@ class MultiServerHttpIntegrationTestBase(IntegrationTestBase): def __init__(self, request, server_config, backend_count): """See base class.""" - super(MultiServerHttpIntegrationTestBase, self).__init__(request, server_config, - backend_count) + super(MultiServerHttpIntegrationTestBase, self).__init__(request, server_config, backend_count) def getTestServerRootUri(self): """See base class.""" @@ -387,8 +386,7 @@ class MultiServerHttpsIntegrationTestBase(IntegrationTestBase): def __init__(self, request, server_config, backend_count): """See base class.""" - super(MultiServerHttpsIntegrationTestBase, self).__init__(request, server_config, - backend_count) + super(MultiServerHttpsIntegrationTestBase, self).__init__(request, server_config, backend_count) def getTestServerRootUri(self): """See base class.""" diff --git a/test/integration/nighthawk_test_server.py b/test/integration/nighthawk_test_server.py index bdc792c23..431d025dd 100644 --- a/test/integration/nighthawk_test_server.py +++ b/test/integration/nighthawk_test_server.py @@ -42,7 +42,8 @@ def _substitute_yaml_values(runfiles_instance, obj, params): return obj -class _TestCaseWarnErrorIgnoreList(collections.namedtuple("_TestCaseWarnErrorIgnoreList", "test_case_regexp ignore_list")): +class _TestCaseWarnErrorIgnoreList( + collections.namedtuple("_TestCaseWarnErrorIgnoreList", "test_case_regexp ignore_list")): """Maps test case names to messages that should be ignored in the test server logs. If the name of the currently executing test case matches the test_case_regexp, @@ -56,6 +57,7 @@ class _TestCaseWarnErrorIgnoreList(collections.namedtuple("_TestCaseWarnErrorIgn ignore_list: A tuple of strings, messages to ignore for matching test cases. """ + # A list of _TestCaseWarnErrorIgnoreList instances, message pieces that should # be ignored even if logged by the test server at a warning or an error # severity. @@ -68,31 +70,35 @@ class _TestCaseWarnErrorIgnoreList(collections.namedtuple("_TestCaseWarnErrorIgn _TEST_SERVER_WARN_ERROR_IGNORE_LIST = frozenset([ # This test case purposefuly uses the deprecated Envoy v2 API which emits # the following warnings. - _TestCaseWarnErrorIgnoreList(re.compile('test_nighthawk_test_server_envoy_deprecated_v2_api'), ( - "Configuration does not parse cleanly as v3. v2 configuration is deprecated", - "Deprecated field: type envoy.api.v2.listener.Filter", - "Deprecated field: type envoy.config.filter.network.http_connection_manager.v2.HttpFilter", - "Using deprecated extension name 'envoy.http_connection_manager'", - "Using deprecated extension name 'envoy.router'", + _TestCaseWarnErrorIgnoreList( + re.compile('test_nighthawk_test_server_envoy_deprecated_v2_api'), + ( + "Configuration does not parse cleanly as v3. v2 configuration is deprecated", + "Deprecated field: type envoy.api.v2.listener.Filter", + "Deprecated field: type envoy.config.filter.network.http_connection_manager.v2.HttpFilter", + "Using deprecated extension name 'envoy.http_connection_manager'", + "Using deprecated extension name 'envoy.router'", + ), ), - ), # A catch-all that applies to all remaining test cases. - _TestCaseWarnErrorIgnoreList(re.compile('.*'), ( - # TODO(#582): Identify these and file issues or add explanation as necessary. - "Unable to use runtime singleton for feature envoy.http.headermap.lazy_map_min_size", - "Using deprecated extension name 'envoy.listener.tls_inspector' for 'envoy.filters.listener.tls_inspector'.", - "there is no configured limit to the number of allowed active connections. Set a limit via the runtime key overload.global_downstream_max_connections", - - # A few of our filters use the same typed configuration, specifically - # 'test-server', 'time-tracking' and 'dynamic-delay'. - # For now this is by design. - "Double registration for type: 'nighthawk.server.ResponseOptions'", - - # Logged for normal termination, not really a warning. - "caught SIGTERM", + _TestCaseWarnErrorIgnoreList( + re.compile('.*'), + ( + # TODO(#582): Identify these and file issues or add explanation as necessary. + "Unable to use runtime singleton for feature envoy.http.headermap.lazy_map_min_size", + "Using deprecated extension name 'envoy.listener.tls_inspector' for 'envoy.filters.listener.tls_inspector'.", + "there is no configured limit to the number of allowed active connections. Set a limit via the runtime key overload.global_downstream_max_connections", + + # A few of our filters use the same typed configuration, specifically + # 'test-server', 'time-tracking' and 'dynamic-delay'. + # For now this is by design. + "Double registration for type: 'nighthawk.server.ResponseOptions'", + + # Logged for normal termination, not really a warning. + "caught SIGTERM", + ), ), - ), ]) @@ -345,7 +351,7 @@ def getCliVersionString(self): def _matchesAnyIgnoreListEntry(line, test_case_name, ignore_list): - """Determines if the line matches any of the ignore list entries for this test case. + """Determine if the line matches any of the ignore list entries for this test case. Args: line: A string, the logged line. diff --git a/test/integration/unit_tests/test_nighthawk_test_server.py b/test/integration/unit_tests/test_nighthawk_test_server.py index 867baf30f..cd7a25e83 100644 --- a/test/integration/unit_tests/test_nighthawk_test_server.py +++ b/test/integration/unit_tests/test_nighthawk_test_server.py @@ -18,7 +18,8 @@ def test_extractWarningsAndErrors_ignores_info_logs(): process_output = """ [2020-12-01 04:41:57.219][126][info][misc] Message. """ - warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case", []) + warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case", + []) assert not warnings assert not errors @@ -26,7 +27,8 @@ def test_extractWarningsAndErrors_ignores_info_logs(): def test_extractWarningsAndErrors_extracts_a_warning(): """Test where the process output contains a single warning.""" process_output = "[2020-12-01 04:41:57.219][126][warning][misc] Message." - warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case", []) + warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case", + []) assert warnings == ["[2020-12-01 04:41:57.219][126][warning][misc] Message."] assert not errors @@ -34,7 +36,8 @@ def test_extractWarningsAndErrors_extracts_a_warning(): def test_extractWarningsAndErrors_extracts_an_error(): """Test where the process output contains a single error.""" process_output = "[2020-12-01 04:41:57.219][126][error][misc] Message." - warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case", []) + warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case", + []) assert not warnings assert errors == ["[2020-12-01 04:41:57.219][126][error][misc] Message."] @@ -47,12 +50,14 @@ def test_extractWarningsAndErrors_extracts_multiple_messages(): [error][runtime] Error2. [warning][runtime] Warning2. """ - warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case", []) + warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case", + []) assert warnings == ["[warning][misc] Warning1.", "[warning][runtime] Warning2."] assert errors == ["[error][misc] Error1.", "[error][runtime] Error2."] -def test_extractWarningsAndErrors_skips_messages_matching_ignore_list_when_test_case_matched_with_a_glob(): +def test_extractWarningsAndErrors_skips_messages_matching_ignore_list_when_test_case_matched_with_a_glob( +): """Test where the ignore list is used.""" process_output = """[warning][misc] Warning1 foo. [error][misc] Error1 bar. @@ -64,12 +69,14 @@ def test_extractWarningsAndErrors_skips_messages_matching_ignore_list_when_test_ ignore_list = [ nighthawk_test_server._TestCaseWarnErrorIgnoreList(re.compile(".*"), ("foo", "bar")), ] - warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case", ignore_list) + warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case", + ignore_list) assert not warnings assert errors == ["[error][runtime] Error2 baz."] -def test_extractWarningsAndErrors_skips_messages_matching_ignore_list_when_test_case_matched_exactly(): +def test_extractWarningsAndErrors_skips_messages_matching_ignore_list_when_test_case_matched_exactly( +): """Test where the ignore list is used.""" process_output = """[warning][misc] Warning1 foo. [error][misc] Error1 bar. @@ -81,7 +88,8 @@ def test_extractWarningsAndErrors_skips_messages_matching_ignore_list_when_test_ ignore_list = [ nighthawk_test_server._TestCaseWarnErrorIgnoreList(re.compile("test_case"), ("foo", "bar")), ] - warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case", ignore_list) + warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case", + ignore_list) assert not warnings assert errors == ["[error][runtime] Error2 baz."] @@ -96,10 +104,11 @@ def test_extractWarningsAndErrors_does_not_apply_ignore_list_for_non_matching_te """ ignore_list = [ - nighthawk_test_server._TestCaseWarnErrorIgnoreList(re.compile("test_case1"), ("foo", )), - nighthawk_test_server._TestCaseWarnErrorIgnoreList(re.compile("test_case2"), ("bar", )), + nighthawk_test_server._TestCaseWarnErrorIgnoreList(re.compile("test_case1"), ("foo",)), + nighthawk_test_server._TestCaseWarnErrorIgnoreList(re.compile("test_case2"), ("bar",)), ] - warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case1", ignore_list) + warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case1", + ignore_list) assert warnings == ["[warning][runtime] Warning2 bar."] assert errors == [ "[error][misc] Error1 bar.", @@ -117,10 +126,11 @@ def test_extractWarningsAndErrors_applies_all_matching_ignore_lists(): """ ignore_list = [ - nighthawk_test_server._TestCaseWarnErrorIgnoreList(re.compile("test_case1"), ("foo", )), - nighthawk_test_server._TestCaseWarnErrorIgnoreList(re.compile(".*"), ("bar", )), + nighthawk_test_server._TestCaseWarnErrorIgnoreList(re.compile("test_case1"), ("foo",)), + nighthawk_test_server._TestCaseWarnErrorIgnoreList(re.compile(".*"), ("bar",)), ] - warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case1", ignore_list) + warnings, errors = nighthawk_test_server._extractWarningsAndErrors(process_output, "test_case1", + ignore_list) assert not warnings assert errors == ["[error][runtime] Error2 baz."] From e10c7b368f47bd174dec4f4b37610fe8f0b2a880 Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Tue, 8 Dec 2020 02:41:42 -0500 Subject: [PATCH 16/17] Self-review. Signed-off-by: Jakub Sobon --- test/integration/nighthawk_test_server.py | 25 +++++++++++------------ 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/test/integration/nighthawk_test_server.py b/test/integration/nighthawk_test_server.py index 431d025dd..e4c9ea6d4 100644 --- a/test/integration/nighthawk_test_server.py +++ b/test/integration/nighthawk_test_server.py @@ -47,9 +47,11 @@ class _TestCaseWarnErrorIgnoreList( """Maps test case names to messages that should be ignored in the test server logs. If the name of the currently executing test case matches the test_case_regexp, - any messages logged by the test server as either a WARNING or an ERROR that - will be checked against the ignore_list. Matching messages will be ignored, - any unmatched messages will fail the test case. + any messages logged by the test server as either a WARNING or an ERROR + will be checked against the ignore_list. If the logged messages contain any of + the messages in the ignore list as a substring, they will be ignored. + Any unmatched messages of either a WARNING or an ERROR severity will fail the + test case. Attributes: test_case_regexp: A compiled regular expression as returned by re.compile(), @@ -59,16 +61,13 @@ class _TestCaseWarnErrorIgnoreList( # A list of _TestCaseWarnErrorIgnoreList instances, message pieces that should -# be ignored even if logged by the test server at a warning or an error +# be ignored even if logged by the test server at a WARNING or an ERROR # severity. # -# This list is processed in the order as defined, if multiple test_case_regexp -# entries match the current test case name, all the corresponding ignore lists -# will be used. -# -# Non matching messages logged at either of these severities fail the test. +# If multiple test_case_regexp entries match the current test case name, all the +# corresponding ignore lists will be used. _TEST_SERVER_WARN_ERROR_IGNORE_LIST = frozenset([ - # This test case purposefuly uses the deprecated Envoy v2 API which emits + # This test case purposefully uses the deprecated Envoy v2 API which emits # the following warnings. _TestCaseWarnErrorIgnoreList( re.compile('test_nighthawk_test_server_envoy_deprecated_v2_api'), @@ -378,9 +377,9 @@ def _extractWarningsAndErrors(process_output, test_case_name, ignore_list): Args: process_output: A string, the stdout or stderr after running a process. test_case_name: A string, the name of the current test case. - ignore_list: A list of _TestCaseWarnErrorIgnoreList instances, message - pieces to ignore. If a message that was logged either at a warning or at - an error severity contains one of these message pieces and should be + ignore_list: A list of _TestCaseWarnErrorIgnoreList instances, the message + pieces to ignore. If a message that was logged either at a WARNING or at + an ERROR severity contains one of these message pieces and should be ignored for the current test case, it will be excluded from the return values. From 117ef25c8fe55ede298fdb721176a4ef81d9f007 Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Tue, 8 Dec 2020 03:07:48 -0500 Subject: [PATCH 17/17] Fix initialization of InjectHttpProxyIntegrationTestBase. Signed-off-by: Jakub Sobon --- benchmarks/envoy_proxy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmarks/envoy_proxy.py b/benchmarks/envoy_proxy.py index 45486bda1..8a48ad362 100644 --- a/benchmarks/envoy_proxy.py +++ b/benchmarks/envoy_proxy.py @@ -131,7 +131,7 @@ def inject_envoy_http_proxy_fixture(request, server_config, proxy_config, caplog Yields: a successfully set up InjectHttpProxyIntegrationTestBase instance. """ - fixture = InjectHttpProxyIntegrationTestBase(request.param, server_config, proxy_config) + fixture = InjectHttpProxyIntegrationTestBase(request, server_config, proxy_config) fixture.setUp() yield fixture fixture.tearDown(caplog)