This is an automated email from the ASF dual-hosted git repository.
csringhofer pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
The following commit(s) were added to refs/heads/master by this push:
new 300509233 IMPALA-13668: Add default_test_protocol parameter to py.test
300509233 is described below
commit 30050923320e5bbd2de0c17b3c48548b0c7a8080
Author: Riza Suminto <[email protected]>
AuthorDate: Sun Jan 12 21:45:19 2025 -0800
IMPALA-13668: Add default_test_protocol parameter to py.test
ImpalaTestSuite.client is always initialized as beeswax client. And many
tests use it directly rather than going through helper method such as
execute_query().
This patch add add default_test_protocol parameter to conftest.py. It
control whether to initialize ImpalaTestSuite.client equals to
'beeswax_client', 'hs2_client', or 'hs2_http_client'. This parameter is
still default to 'beeswax'.
This patch also adds helper method 'default_client_protocol_dimension',
'beeswax_client_protocol_dimension' and 'hs2_client_protocol_dimension'
for convenience and traceability.
Reduced occurrence where test method manually override
ImpalaTestSuite.client. They are replaced by combination of
ImpalaTestSuite.create_impala_clients and
ImpalaTestSuite.close_impala_clients.
Testing:
- Pass core tests.
Change-Id: I9165ea220b2c83ca36d6e68ef3b88b128310af23
Reviewed-on: http://gerrit.cloudera.org:8080/22336
Reviewed-by: Impala Public Jenkins <[email protected]>
Tested-by: Impala Public Jenkins <[email protected]>
---
tests/common/impala_test_suite.py | 46 ++++++++++++++--------
tests/common/test_dimensions.py | 13 ++++++
tests/conftest.py | 6 +++
tests/custom_cluster/test_coordinators.py | 4 +-
.../custom_cluster/test_parquet_max_page_header.py | 5 +--
tests/custom_cluster/test_partition.py | 9 +++--
tests/custom_cluster/test_permanent_udfs.py | 8 ++--
tests/custom_cluster/test_workload_mgmt_init.py | 4 +-
8 files changed, 64 insertions(+), 31 deletions(-)
diff --git a/tests/common/impala_test_suite.py
b/tests/common/impala_test_suite.py
index 425fca5d4..46c2a550c 100644
--- a/tests/common/impala_test_suite.py
+++ b/tests/common/impala_test_suite.py
@@ -54,6 +54,7 @@ from tests.common.test_dimensions import (
ALL_NODES_ONLY,
TableFormatInfo,
create_exec_option_dimension,
+ default_client_protocol_dimension,
get_dataset_from_workload,
load_table_info_dimension)
from tests.common.test_result_verifier import (
@@ -200,7 +201,7 @@ class ImpalaTestSuite(BaseTestSuite):
cls.ImpalaTestMatrix.add_dimension(cls.__create_exec_option_dimension())
# Execute tests through Beeswax by default. Individual tests that have
been converted
# to work with the HS2 client can add HS2 in addition to or instead of
beeswax.
- cls.ImpalaTestMatrix.add_dimension(ImpalaTestDimension('protocol',
'beeswax'))
+ cls.ImpalaTestMatrix.add_dimension(default_client_protocol_dimension())
@staticmethod
def create_hive_client(port):
@@ -224,6 +225,7 @@ class ImpalaTestSuite(BaseTestSuite):
def setup_class(cls):
"""Setup section that runs before each test suite"""
cls.client = None
+ cls.beeswax_client = None
cls.hive_client = None
cls.hs2_client = None
cls.hs2_http_client = None
@@ -318,8 +320,9 @@ class ImpalaTestSuite(BaseTestSuite):
cls.close_impala_clients()
@classmethod
- def create_impala_client(cls, host_port=None, protocol='beeswax',
- is_hive=False):
+ def create_impala_client(cls, host_port=None,
+ protocol=pytest.config.option.default_test_protocol,
+ is_hive=False):
if host_port is None:
host_port = cls.__get_default_host_port(protocol)
client = create_connection(host_port=host_port,
@@ -330,10 +333,11 @@ class ImpalaTestSuite(BaseTestSuite):
@classmethod
def get_impalad_cluster_size(cls):
- return len(cls.__get_cluster_host_ports('beeswax'))
+ return
len(cls.__get_cluster_host_ports(pytest.config.option.default_test_protocol))
@classmethod
- def create_client_for_nth_impalad(cls, nth=0, protocol='beeswax'):
+ def create_client_for_nth_impalad(cls, nth=0,
+
protocol=pytest.config.option.default_test_protocol):
host_ports = cls.__get_cluster_host_ports(protocol)
if nth < len(IMPALAD_HOST_PORT_LIST):
host_port = host_ports[nth]
@@ -351,7 +355,7 @@ class ImpalaTestSuite(BaseTestSuite):
"""Creates Impala clients for all supported protocols."""
# The default connection (self.client) is Beeswax so that existing tests,
which assume
# Beeswax do not need modification (yet).
- cls.client = cls.create_impala_client(protocol='beeswax')
+ cls.beeswax_client = cls.create_impala_client(protocol='beeswax')
cls.hs2_client = None
try:
cls.hs2_client = cls.create_impala_client(protocol='hs2')
@@ -365,19 +369,35 @@ class ImpalaTestSuite(BaseTestSuite):
# HS2 HTTP connection can fail for benign reasons, e.g. running with
unsupported
# auth.
LOG.info("HS2 HTTP connection setup failed, continuing...:
{0}".format(e))
+ cls.client =
cls.default_impala_client(pytest.config.option.default_test_protocol)
@classmethod
def close_impala_clients(cls):
"""Closes Impala clients created by create_impala_clients()."""
- if cls.client:
- cls.client.close()
- cls.client = None
+ if cls.beeswax_client:
+ cls.beeswax_client.close()
+ cls.beeswax_client = None
if cls.hs2_client:
cls.hs2_client.close()
cls.hs2_client = None
if cls.hs2_http_client:
cls.hs2_http_client.close()
cls.hs2_http_client = None
+ # cls.client should be equal to one of above, unless test method
implicitly override.
+ # Closing twice should be OK.
+ if cls.client:
+ cls.client.close()
+ cls.client = None
+
+ @classmethod
+ def default_impala_client(cls, protocol):
+ if protocol == 'beeswax':
+ return cls.beeswax_client
+ if protocol == 'hs2':
+ return cls.hs2_client
+ if protocol == 'hs2-http':
+ return cls.hs2_http_client
+ raise Exception("unknown protocol: {0}".format(protocol))
@classmethod
def __get_default_host_port(cls, protocol):
@@ -667,13 +687,7 @@ class ImpalaTestSuite(BaseTestSuite):
[ImpalaTestSuite.create_impala_client(host_port, protocol=protocol)
for host_port in self.__get_cluster_host_ports(protocol)]
else:
- if protocol == 'beeswax':
- target_impalad_clients = [self.client]
- elif protocol == 'hs2-http':
- target_impalad_clients = [self.hs2_http_client]
- else:
- assert protocol == 'hs2'
- target_impalad_clients = [self.hs2_client]
+ target_impalad_clients = [self.default_impala_client(protocol)]
# Change the database to reflect the file_format, compression codec etc,
or the
# user specified database for all targeted impalad.
diff --git a/tests/common/test_dimensions.py b/tests/common/test_dimensions.py
index 69ea5dd08..7e8f33a6f 100644
--- a/tests/common/test_dimensions.py
+++ b/tests/common/test_dimensions.py
@@ -21,6 +21,7 @@ from __future__ import absolute_import, division,
print_function
from builtins import range
import copy
import os
+import pytest
from itertools import product
from tests.common.test_vector import (
@@ -148,6 +149,18 @@ def create_kudu_dimension(workload):
return create_table_format_dimension(workload, 'kudu/none')
+def default_client_protocol_dimension():
+ return ImpalaTestDimension('protocol',
pytest.config.option.default_test_protocol)
+
+
+def beeswax_client_protocol_dimension():
+ return ImpalaTestDimension('protocol', 'beeswax')
+
+
+def hs2_client_protocol_dimension():
+ return ImpalaTestDimension('protocol', 'hs2')
+
+
def create_client_protocol_dimension():
# IMPALA-8864: Older python versions do not support SSLContext object that
the thrift
# http client implementation depends on. Falls back to a dimension without
http
diff --git a/tests/conftest.py b/tests/conftest.py
index 60ef23cb3..9366e021c 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -54,6 +54,7 @@ DEFAULT_KUDU_MASTER_HOSTS = os.getenv('KUDU_MASTER_HOSTS',
'127.0.0.1')
DEFAULT_KUDU_MASTER_PORT = os.getenv('KUDU_MASTER_PORT', '7051')
DEFAULT_METASTORE_SERVER = 'localhost:9083'
DEFAULT_NAMENODE_ADDR = None
+DEFAULT_TEST_PROTOCOL = os.getenv('DEFAULT_TEST_PROTOCOL', 'beeswax')
if FILESYSTEM == 'isilon':
DEFAULT_NAMENODE_ADDR =
"{node}:{port}".format(node=os.getenv("ISILON_NAMENODE"),
port=ISILON_WEBHDFS_PORT)
@@ -62,6 +63,7 @@ if FILESYSTEM == 'isilon':
PYTEST_TIMEOUT_S = \
build_flavor_timeout(2 * 60 * 60, slow_build_timeout=4 * 60 * 60)
+
def pytest_configure(config):
""" Hook startup of pytest. Sets up log format and per-test timeout. """
configure_logging()
@@ -185,6 +187,10 @@ def pytest_addoption(parser):
help="Location to store the output JSON files for "
"calcite_report_mode. Defaults to
${IMPALA_LOGS_DIR}/calcite_report.")
+ parser.addoption("--default_test_protocol", default=DEFAULT_TEST_PROTOCOL,
+ choices=("beeswax", "hs2", "hs2-http"),
+ help="Impala protocol to run test if test does not specify
any.")
+
def pytest_assertrepr_compare(op, left, right):
"""
diff --git a/tests/custom_cluster/test_coordinators.py
b/tests/custom_cluster/test_coordinators.py
index 83309998f..0fc75e4dd 100644
--- a/tests/custom_cluster/test_coordinators.py
+++ b/tests/custom_cluster/test_coordinators.py
@@ -266,14 +266,11 @@ class TestCoordinators(CustomClusterTestSuite):
assert len(self.cluster.impalads) == 3
coordinator = self.cluster.impalads[0]
- worker1 = self.cluster.impalads[1]
- worker2 = self.cluster.impalads[2]
client = None
try:
client = coordinator.service.create_beeswax_client()
assert client is not None
- self.client = client
client.execute("SET EXPLAIN_LEVEL=2")
client.execute("SET TEST_REPLAN=0")
@@ -290,6 +287,7 @@ class TestCoordinators(CustomClusterTestSuite):
assert 'F02:PLAN FRAGMENT [RANDOM] hosts=2 instances=2' in result
finally:
assert client is not None
+ client.close()
self._stop_impala_cluster()
@pytest.mark.execute_serially
diff --git a/tests/custom_cluster/test_parquet_max_page_header.py
b/tests/custom_cluster/test_parquet_max_page_header.py
index 5f618982c..448817637 100644
--- a/tests/custom_cluster/test_parquet_max_page_header.py
+++ b/tests/custom_cluster/test_parquet_max_page_header.py
@@ -60,13 +60,12 @@ class TestParquetMaxPageHeader(CustomClusterTestSuite):
def setup_method(self, method):
super(TestParquetMaxPageHeader, self).setup_method(method)
- impalad = self.cluster.impalads[0]
- client = impalad.service.create_beeswax_client()
- self.client = client
+ self.create_impala_clients()
self.__create_test_tbls()
def teardown_method(self, method):
self.__drop_test_tbls()
+ self.close_impala_clients()
def __drop_test_tbls(self):
self.client.execute("DROP TABLE IF EXISTS %s PURGE" % self.TEXT_TABLE_NAME)
diff --git a/tests/custom_cluster/test_partition.py
b/tests/custom_cluster/test_partition.py
index e0ef8ecf6..10a81d462 100644
--- a/tests/custom_cluster/test_partition.py
+++ b/tests/custom_cluster/test_partition.py
@@ -166,8 +166,9 @@ class TestPartitionDeletion(CustomClusterTestSuite):
self.assert_catalogd_log_contains("INFO",
deletion_log_regex.format(tbl, i))
# Restart impalad and check the partitions on it
+ self.close_impala_clients()
self.cluster.impalads[0].restart()
- self.client = self.create_impala_client()
+ self.create_impala_clients()
new_res = self.client.execute("show partitions {}".format(tbl))
assert new_res.data == res.data
self.assert_impalad_log_contains("WARNING", "stale partition",
expected_count=0)
@@ -190,8 +191,9 @@ class TestPartitionDeletion(CustomClusterTestSuite):
assert len(res.data) == 3
# Restart impalad and check the partitions on it
+ self.close_impala_clients()
self.cluster.impalads[0].restart()
- self.client = self.create_impala_client()
+ self.create_impala_clients()
new_res = self.client.execute("show partitions {}".format(tbl))
assert new_res.data == res.data
self.assert_impalad_log_contains("WARNING", "stale partition",
expected_count=0)
@@ -209,8 +211,9 @@ class TestPartitionDeletion(CustomClusterTestSuite):
self.client.execute("create table {}(i int) partitioned by (p
int)".format(tbl))
self.client.execute("describe " + tbl)
# Restart impalad and check the partitions on it
+ self.close_impala_clients()
self.cluster.impalads[0].restart()
- self.client = self.create_impala_client()
+ self.create_impala_clients()
res = self.client.execute("show partitions {}".format(tbl))
assert len(res.data) == 1
self.assert_impalad_log_contains("WARNING", "stale partition",
expected_count=0)
diff --git a/tests/custom_cluster/test_permanent_udfs.py
b/tests/custom_cluster/test_permanent_udfs.py
index bd9428f76..4b7ea54ce 100644
--- a/tests/custom_cluster/test_permanent_udfs.py
+++ b/tests/custom_cluster/test_permanent_udfs.py
@@ -57,8 +57,7 @@ class TestUdfPersistence(CustomClusterTestSuite):
def setup_method(self, method):
super(TestUdfPersistence, self).setup_method(method)
- impalad = self.cluster.impalads[0]
- self.client = impalad.service.create_beeswax_client()
+ self.create_impala_clients()
self.__cleanup()
self.__load_drop_functions(
self.CREATE_UDFS_TEMPLATE, self.DATABASE,
@@ -82,6 +81,7 @@ class TestUdfPersistence(CustomClusterTestSuite):
def teardown_method(self, method):
self.__cleanup()
+ self.close_impala_clients()
self.clear_tmp_dirs()
def __cleanup(self):
@@ -99,10 +99,10 @@ class TestUdfPersistence(CustomClusterTestSuite):
assert result is not None
def __restart_cluster(self):
+ self.close_impala_clients()
self._stop_impala_cluster()
self._start_impala_cluster(list())
- impalad = self.cluster.impalads[0]
- self.client = impalad.service.create_beeswax_client()
+ self.create_impala_clients()
def verify_function_count(self, query, count):
result = self.client.execute(query)
diff --git a/tests/custom_cluster/test_workload_mgmt_init.py
b/tests/custom_cluster/test_workload_mgmt_init.py
index 55b4fdcf0..7918592c1 100644
--- a/tests/custom_cluster/test_workload_mgmt_init.py
+++ b/tests/custom_cluster/test_workload_mgmt_init.py
@@ -72,6 +72,7 @@ class TestWorkloadManagementInitBase(CustomClusterTestSuite):
catalog_opts += "--workload_mgmt_schema_version={}
".format(schema_version)
try:
+ self.close_impala_clients()
num_coords = cluster_size
if cluster_size > 1:
num_coords = cluster_size - 1
@@ -80,12 +81,11 @@ class
TestWorkloadManagementInitBase(CustomClusterTestSuite):
cluster_size=cluster_size, expected_num_impalads=cluster_size,
num_coordinators=num_coords, wait_for_backends=wait_for_backends,
log_symlinks=log_symlinks)
+ self.create_impala_clients()
except CalledProcessError as e:
if not expect_startup_err:
raise e
- self.client =
self.create_impala_client(protocol=vector.get_value("protocol"))
-
if wait_for_init_complete:
self.wait_for_wm_init_complete()