This is an automated email from the ASF dual-hosted git repository.

joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit 35dc24fbc8bb5ea27cc8e887512672070a05bcb7
Author: Michael Smith <[email protected]>
AuthorDate: Thu Oct 6 12:14:42 2022 -0700

    IMPALA-10148: Cleanup cores in TestHooksStartupFail
    
    Generalizes coredump cleanup and expecting startup failure from
    test_provider.py and uses it in test_query_event_hooks.py
    TestHooksStartupFail to ensure core dumps are cleaned up.
    
    Testing: ran changed tests, observed core files being created and
    cleaned up while they ran. Observed other core files already present
    were not cleaned up, as expected.
    
    Change-Id: Iec32e0acbadd65aa78264594c85ffcd574cf3458
    Reviewed-on: http://gerrit.cloudera.org:8080/19103
    Reviewed-by: Laszlo Gaal <[email protected]>
    Reviewed-by: Joe McDonnell <[email protected]>
    Tested-by: Impala Public Jenkins <[email protected]>
---
 tests/authorization/test_provider.py           | 35 +-----------------------
 tests/common/custom_cluster_test_suite.py      | 37 +++++++++++++++++++++++---
 tests/custom_cluster/test_query_event_hooks.py | 20 ++------------
 3 files changed, 36 insertions(+), 56 deletions(-)

diff --git a/tests/authorization/test_provider.py 
b/tests/authorization/test_provider.py
index c80db7849..362ca7ae9 100644
--- a/tests/authorization/test_provider.py
+++ b/tests/authorization/test_provider.py
@@ -17,17 +17,13 @@
 #
 # Client tests for SQL statement authorization
 
-import logging
 import pytest
 import os
 import tempfile
 
-from impala_py_lib.helpers import find_all_files, is_core_dump
 from tests.common.file_utils import assert_file_in_dir_contains
 from tests.common.custom_cluster_test_suite import CustomClusterTestSuite
 
-LOG = logging.getLogger('test_provider')
-
 
 class TestAuthorizationProvider(CustomClusterTestSuite):
   """
@@ -46,6 +42,7 @@ class TestAuthorizationProvider(CustomClusterTestSuite):
 
   @pytest.mark.execute_serially
   @CustomClusterTestSuite.with_args(
+      expect_cores=True,
       impala_log_dir=LOG_DIR,
       impalad_args="--minidump_path={0}"
                    "--server-name=server1 "
@@ -63,33 +60,3 @@ class TestAuthorizationProvider(CustomClusterTestSuite):
                                 "InternalException: Could not parse "
                                 "authorization_provider flag: {0}"
                                 .format(TestAuthorizationProvider.BAD_FLAG))
-
-  def setup_method(self, method):
-    # Make a note of any core files that already exist
-    possible_cores = find_all_files('*core*')
-    self.pre_test_cores = set([f for f in possible_cores if is_core_dump(f)])
-
-    # Explicitly override CustomClusterTestSuite.setup_method() to
-    # allow it to exception, since this testsuite is for cases where
-    # startup fails
-    try:
-      super(TestAuthorizationProvider, self).setup_method(method)
-    except Exception:
-      self._stop_impala_cluster()
-
-  def teardown_method(self, method):
-    try:
-      # The core dumps expected to be generated by this test should be cleaned 
up
-      possible_cores = find_all_files('*core*')
-      post_test_cores = set([f for f in possible_cores if is_core_dump(f)])
-
-      for f in (post_test_cores - self.pre_test_cores):
-        LOG.info("Cleaned up {core} created by 
test_invalid_provider_flag".format(core=f))
-        os.remove(f)
-
-      # Explicitly override CustomClusterTestSuite.teardown_method() to
-      # allow it to exception, since it relies on setup_method() having
-      # completed successfully
-      super(TestAuthorizationProvider, self).teardown_method(method)
-    except Exception:
-      self._stop_impala_cluster()
diff --git a/tests/common/custom_cluster_test_suite.py 
b/tests/common/custom_cluster_test_suite.py
index 6a831bb28..2bb43e23f 100644
--- a/tests/common/custom_cluster_test_suite.py
+++ b/tests/common/custom_cluster_test_suite.py
@@ -24,6 +24,7 @@ import os.path
 import pipes
 import pytest
 import subprocess
+from impala_py_lib.helpers import find_all_files, is_core_dump
 from subprocess import check_call
 from tests.common.impala_test_suite import ImpalaTestSuite
 from tests.common.impala_cluster import ImpalaCluster
@@ -51,6 +52,7 @@ IMPALA_LOG_DIR = 'impala_log_dir'
 NUM_EXCLUSIVE_COORDINATORS = 'num_exclusive_coordinators'
 STATESTORED_TIMEOUT_S = 'statestored_timeout_s'
 IMPALAD_TIMEOUT_S = 'impalad_timeout_s'
+EXPECT_CORES = 'expect_cores'
 
 # Run with fast topic updates by default to reduce time to first query running.
 DEFAULT_STATESTORE_ARGS = '--statestore_update_frequency_ms=50 \
@@ -104,7 +106,7 @@ class CustomClusterTestSuite(ImpalaTestSuite):
       start_args=None, default_query_options=None,
       impala_log_dir=None, hive_conf_dir=None, cluster_size=None,
       num_exclusive_coordinators=None, kudu_args=None, 
statestored_timeout_s=None,
-      impalad_timeout_s=None):
+      impalad_timeout_s=None, expect_cores=None):
     """Records arguments to be passed to a cluster by adding them to the 
decorated
     method's func_dict"""
     def decorate(func):
@@ -131,6 +133,8 @@ class CustomClusterTestSuite(ImpalaTestSuite):
         func.func_dict[STATESTORED_TIMEOUT_S] = statestored_timeout_s
       if impalad_timeout_s is not None:
         func.func_dict[IMPALAD_TIMEOUT_S] = impalad_timeout_s
+      if expect_cores is not None:
+        func.func_dict[EXPECT_CORES] = expect_cores
       return func
     return decorate
 
@@ -177,13 +181,38 @@ class CustomClusterTestSuite(ImpalaTestSuite):
       kwargs["statestored_timeout_s"] = method.func_dict[STATESTORED_TIMEOUT_S]
     if IMPALAD_TIMEOUT_S in method.func_dict:
       kwargs["impalad_timeout_s"] = method.func_dict[IMPALAD_TIMEOUT_S]
-    self._start_impala_cluster(cluster_args, **kwargs)
-    super(CustomClusterTestSuite, self).setup_class()
+
+    if method.func_dict.get(EXPECT_CORES, False):
+      # Make a note of any core files that already exist
+      possible_cores = find_all_files('*core*')
+      self.pre_test_cores = set([f for f in possible_cores if is_core_dump(f)])
+
+      # Explicitly allow startup to exception, since startup is expected to 
fail
+      try:
+        self._start_impala_cluster(cluster_args, **kwargs)
+        pytest.fail("cluster startup should have failed")
+      except Exception:
+        self._stop_impala_cluster()
+    else:
+      self._start_impala_cluster(cluster_args, **kwargs)
+      super(CustomClusterTestSuite, self).setup_class()
 
   def teardown_method(self, method):
     if HIVE_CONF_DIR in method.func_dict:
       self._start_hive_service(None)  # Restart Hive Service using default 
configs
-    super(CustomClusterTestSuite, self).teardown_class()
+
+    if method.func_dict.get(EXPECT_CORES, False):
+      # The core dumps expected to be generated by this test should be cleaned 
up
+      possible_cores = find_all_files('*core*')
+      post_test_cores = set([f for f in possible_cores if is_core_dump(f)])
+
+      for f in (post_test_cores - self.pre_test_cores):
+        logging.info(
+            "Cleaned up {core} created by {name}".format(core=f, 
name=method.__name__))
+        os.remove(f)
+      # Skip teardown_class as setup was skipped.
+    else:
+      super(CustomClusterTestSuite, self).teardown_class()
 
   @classmethod
   def _stop_impala_cluster(cls):
diff --git a/tests/custom_cluster/test_query_event_hooks.py 
b/tests/custom_cluster/test_query_event_hooks.py
index a884a22f9..36fe36047 100644
--- a/tests/custom_cluster/test_query_event_hooks.py
+++ b/tests/custom_cluster/test_query_event_hooks.py
@@ -79,6 +79,7 @@ class TestHooksStartupFail(CustomClusterTestSuite):
 
   @pytest.mark.execute_serially
   @CustomClusterTestSuite.with_args(
+      expect_cores=True,
       cluster_size=1,
       impalad_timeout_s=5,
       impala_log_dir=LOG_DIR1,
@@ -99,6 +100,7 @@ class TestHooksStartupFail(CustomClusterTestSuite):
 
   @pytest.mark.execute_serially
   @CustomClusterTestSuite.with_args(
+      expect_cores=True,
       cluster_size=1,
       impalad_timeout_s=5,
       impala_log_dir=LOG_DIR2,
@@ -115,21 +117,3 @@ class TestHooksStartupFail(CustomClusterTestSuite):
     self.assert_impalad_log_contains("INFO",
                                      "Unable to instantiate query event hook 
class {0}"
                                      .format(self.NONEXIST_HOOK), 
expected_count=-1)
-
-  def setup_method(self, method):
-    # Explicitly override CustomClusterTestSuite.setup_method() to
-    # allow it to exception, since this test suite is for cases where
-    # startup fails
-    try:
-      super(TestHooksStartupFail, self).setup_method(method)
-    except Exception:
-      self._stop_impala_cluster()
-
-  def teardown_method(self, method):
-    # Explicitly override CustomClusterTestSuite.teardown_method() to
-    # allow it to exception, since it relies on setup_method() having
-    # completed successfully
-    try:
-      super(TestHooksStartupFail, self).teardown_method(method)
-    except Exception:
-      self._stop_impala_cluster()

Reply via email to