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()
 

Reply via email to