This is an automated email from the ASF dual-hosted git repository. boroknagyz pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit 78ec9903cc317dad3879358fb8ae6f2db2f4802e Author: Riza Suminto <[email protected]> AuthorDate: Wed Jan 7 21:02:43 2026 -0800 IMPALA-14668: Upgrade to pytest 6.2.5 This patch upgrades pytest version from 2.9.2 to 6.2.5, the highest pytest version available without the need to upgrade setuptools. The pytest requirement is moved to py3-requirements.txt. We can not go back to test with Python2 + pytest-2.9.2 after this patch because our test script need to be adjusted as well. This is OK since we have default testing using Python3 since IMPALA-14333 and stop building impala-shell for Python2 since IMPALA-14606. The adjustment are follows: - Replace deprecated @pytest.yield_fixture with plain @pytest.fixture. - Replace --resultlog parameter (removed in version 6.0) with --report-log from pytest-reportlog plugin. - Make impala-shell.sh bootstrap Python3 venv (infra/python/env-gcc10.4.0-py3/) by default. Python2 venv (infra/python/env-gcc10.4.0/) is not bootstrapped automatically anymore. - Upgrade execnet to version 1.9.0. This is required by pytest-xdist==2.4.0. - Remove unused pytest-runner plugin - Add -Wonce argument in pytest.ini to slightly suppress warnings. - Add "junit_logging = system-err" option at pytest.ini to continue logging stderr output to junit xml file. - Fix rootdir and pytest.ini path programmatically at run-tests.py because individual pytest-xdist worker often does not pick this up correctly during parallel run. With pytest-xdist==2.4.0, parallel EE tests does not show verbose individual test names. It only shows pytest progress marker like following line ...ss....s................................s............................. This is a known limitation in pytest-xdist because execnet, the underlying library used for communication between master and workers, does not support transferring stdout/stderr from workers. https://pytest-xdist.readthedocs.io/en/stable/known-limitations.html Read following links for more detail about the deprecation notes: https://docs.pytest.org/en/stable/deprecations.html Change SKIP_SSL_MSG default to empty string because skipif does not accept None reason anymore. Removed run-process-failure-tests.sh (unused after IMPALA-5534) and unused pytest fixtures. Fixed erroneous log formatting in test_restart_services.py. Fixed small warnings found by pytest-6.2.5 that help stabilize exhaustive tests run at: - test_automatic_invalidation.py - test_calcite_planner.py - test_events_custom_configs.py - test_session_expiration.py - test_shell_interactive.py - auto_scaler.py - concurrent_workload.py - hdfs_util.py Most of the remaining warnings are about not closing resources properly at the end of test. These warnings should be addressed in follow up patches. Testing: - Pass exhaustive tests. Change-Id: Ic3812fe976ef09ac48753dee30151714f4752c24 Reviewed-on: http://gerrit.cloudera.org:8080/23842 Reviewed-by: Impala Public Jenkins <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> --- bin/impala-shell.sh | 6 +- bin/run-all-tests.sh | 15 +-- infra/python/deps/py3-requirements.txt | 11 ++ infra/python/deps/requirements.txt | 9 +- tests/common/network.py | 2 +- tests/comparison/tests/conftest.py | 4 +- tests/comparison/tests/test_cursor.py | 2 +- tests/comparison/tests/test_query_objects.py | 2 +- tests/conftest.py | 102 +-------------- .../custom_cluster/test_automatic_invalidation.py | 14 +- tests/custom_cluster/test_calcite_planner.py | 7 +- tests/custom_cluster/test_client_ssl.py | 10 +- tests/custom_cluster/test_events_custom_configs.py | 12 +- tests/custom_cluster/test_ipv6.py | 4 +- tests/custom_cluster/test_restart_services.py | 9 +- tests/custom_cluster/test_session_expiration.py | 143 ++++++++++----------- tests/pytest.ini | 3 +- tests/query_test/test_observability.py | 4 +- tests/run-custom-cluster-tests.sh | 3 +- tests/run-process-failure-tests.sh | 36 ------ tests/run-tests.py | 53 +++++--- tests/shell/test_shell_commandline.py | 2 +- tests/shell/test_shell_interactive.py | 24 ++-- tests/shell/util.py | 2 +- tests/util/auto_scaler.py | 7 +- tests/util/concurrent_workload.py | 8 +- tests/util/hdfs_util.py | 2 +- 27 files changed, 195 insertions(+), 301 deletions(-) diff --git a/bin/impala-shell.sh b/bin/impala-shell.sh index b44f91251..2fff437d9 100755 --- a/bin/impala-shell.sh +++ b/bin/impala-shell.sh @@ -36,8 +36,10 @@ IMPALA_PY3_ENV_DIR="${IMPALA_PY_DIR}/env-gcc${IMPALA_GCC_VERSION}-py3" # Allow overriding the python executable IMPALA_PYTHON_EXECUTABLE="${IMPALA_PYTHON_EXECUTABLE:-${IMPALA_PY3_ENV_DIR}/bin/python3}" -# Note that this uses the external system python executable -PYTHONPATH=${PYTHONPATH} python "${IMPALA_PY_DIR}/bootstrap_virtualenv.py" +# Note that this uses the external system python executable. +# IMPALA-14620: At some point, we need to explicitly invoke python3 instead of python +# here. This is not an issue if python-is-python3 is installed. +PYTHONPATH=${PYTHONPATH} python "${IMPALA_PY_DIR}/bootstrap_virtualenv.py" --python3 # Enable remote debugging if port was specified via environment variable if [[ ${IMPALA_SHELL_DEBUG_PORT:-0} -ne 0 ]]; then diff --git a/bin/run-all-tests.sh b/bin/run-all-tests.sh index c010be0e3..c1b0702a3 100755 --- a/bin/run-all-tests.sh +++ b/bin/run-all-tests.sh @@ -244,11 +244,13 @@ run_ee_tests() { fi # Run end-to-end tests. # KERBEROS TODO - this will need to deal with ${KERB_ARGS} - if ! "${IMPALA_HOME}/tests/run-tests.py" ${COMMON_PYTEST_ARGS} \ + pushd "${IMPALA_HOME}/tests" + if ! ./run-tests.py ${COMMON_PYTEST_ARGS} \ ${RUN_TESTS_ARGS} ${EXTRA_ARGS} ${EE_TEST_FILES}; then #${KERB_ARGS}; TEST_RET_CODE=1 fi + popd } for i in $(seq 1 $NUM_TEST_ITERATIONS) @@ -401,10 +403,12 @@ do # Run the custom-cluster tests after all other tests, since they will restart the # cluster repeatedly and lose state. # TODO: Consider moving in to run-tests.py. - if ! "${IMPALA_HOME}/tests/run-custom-cluster-tests.sh" ${COMMON_PYTEST_ARGS} \ + pushd "${IMPALA_HOME}/tests" + if ! ./run-custom-cluster-tests.sh ${COMMON_PYTEST_ARGS} \ ${RUN_CUSTOM_CLUSTER_TESTS_ARGS}; then TEST_RET_CODE=1 fi + popd export IMPALA_MAX_LOG_FILES="${IMPALA_MAX_LOG_FILES_SAVE}" fi @@ -413,17 +417,12 @@ do pushd "${IMPALA_HOME}/tests" if ! impala-py.test ${TEST_SUITE_VERIFIERS} \ --junitxml=${TEST_SUITE_VERIFIERS_LOG_DIR}/TEST-impala-verifiers.xml \ - --resultlog=${TEST_SUITE_VERIFIERS_LOG_DIR}/TEST-impala-verifiers.log; then + --report-log=${TEST_SUITE_VERIFIERS_LOG_DIR}/TEST-impala-verifiers.log; then TEST_RET_CODE=1 fi popd fi - # Run the process failure tests. - # Disabled temporarily until we figure out the proper timeouts required to make the test - # succeed. - # ${IMPALA_HOME}/tests/run-process-failure-tests.sh - # Store a list of the files at the end of each iteration. This can be compared # to the file-list-begin*.log from the beginning of the iteration to see if files # are not being cleaned up. This is most useful on the first iteration, when diff --git a/infra/python/deps/py3-requirements.txt b/infra/python/deps/py3-requirements.txt index 7ea875d0e..b95044e73 100644 --- a/infra/python/deps/py3-requirements.txt +++ b/infra/python/deps/py3-requirements.txt @@ -33,6 +33,17 @@ pylint == 2.10.2 k5test==0.10.3 kazoo==2.8.0 +# Pytest 6.2.5 and its dependents +pytest == 6.2.5 + attrs == 19.2.0 + iniconfig == 1.1.1 + pluggy == 0.12.0 + py == 1.8.2 + pytest-forked == 1.6.0 + pytest-reportlog == 0.4.0 + pytest-timeout == 2.2.0 + pytest-xdist == 2.4.0 + # Below are needed only for dev flake8==3.9.2 flake8-unused-arguments==0.0.13 diff --git a/infra/python/deps/requirements.txt b/infra/python/deps/requirements.txt index ad4f43e11..dd958fc0d 100644 --- a/infra/python/deps/requirements.txt +++ b/infra/python/deps/requirements.txt @@ -25,8 +25,7 @@ argparse == 1.4.0 future == 0.18.3 hdfs == 2.0.2 docopt == 0.6.2 - execnet == 1.4.0 - apipkg == 1.4 + execnet == 1.9.0 impyla == 0.21a3 bitarray == 2.3.0 pure-sasl == 0.6.2 @@ -39,12 +38,6 @@ prettytable == 0.7.2 prometheus-client == 0.12.0 psutil == 5.6.3 pyparsing == 2.4.7 -pytest == 2.9.2 - py == 1.4.32 - pytest-forked == 0.2 - pytest-runner == 4.2 - pytest-xdist == 1.17.1 - pytest-timeout == 1.2.1 python-magic == 0.4.11 pywebhdfs == 0.3.2 pbr == 3.1.1 diff --git a/tests/common/network.py b/tests/common/network.py index 7a2b11814..ccbb1fcbb 100644 --- a/tests/common/network.py +++ b/tests/common/network.py @@ -77,4 +77,4 @@ elif _openssl_version_number < REQUIRED_MIN_OPENSSL_VERSION: SKIP_SSL_MSG = "Only have OpenSSL version %X, but test requires %X" % ( ssl.OPENSSL_VERSION_NUMBER, REQUIRED_MIN_OPENSSL_VERSION) else: - SKIP_SSL_MSG = None + SKIP_SSL_MSG = "" diff --git a/tests/comparison/tests/conftest.py b/tests/comparison/tests/conftest.py index 996e6aee7..1820051d9 100644 --- a/tests/comparison/tests/conftest.py +++ b/tests/comparison/tests/conftest.py @@ -37,14 +37,14 @@ def cluster(request): return __cluster [email protected]_fixture [email protected] def hive_cursor(request): with cluster(request).hive.connect() as conn: with conn.cursor() as cur: yield cur [email protected]_fixture [email protected] def cursor(request): with cluster(request).impala.connect() as conn: with conn.cursor() as cur: diff --git a/tests/comparison/tests/test_cursor.py b/tests/comparison/tests/test_cursor.py index ee66e3fb3..d88066a06 100644 --- a/tests/comparison/tests/test_cursor.py +++ b/tests/comparison/tests/test_cursor.py @@ -58,7 +58,7 @@ def unittest_cursor_map(request): return request.param [email protected]_fixture [email protected] def postgresql_cursor(): """ Yield a PostgresqlCursor object needed for test infra system tests. diff --git a/tests/comparison/tests/test_query_objects.py b/tests/comparison/tests/test_query_objects.py index 177b98e79..d764d2921 100644 --- a/tests/comparison/tests/test_query_objects.py +++ b/tests/comparison/tests/test_query_objects.py @@ -66,7 +66,7 @@ def verify_sql_matches(actual, expected, strip=True): actual=actual, expected=expected) [email protected]_fixture [email protected] def sql_writer(request): """ Return a SqlWriter object that is torn down at the end of each test. diff --git a/tests/conftest.py b/tests/conftest.py index c10f0bc2c..383aeae4b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -28,10 +28,8 @@ import contextlib import logging import os import pytest -import sys import tests.common -from impala_py_lib.helpers import find_all_files, is_core_dump import tests.common.base_test_suite from tests.common.environ import build_flavor_timeout from tests.common.test_result_verifier import QueryTestResult @@ -276,27 +274,6 @@ def pytest_generate_tests(metafunc): metafunc.parametrize('vector', vectors, ids=vector_names) [email protected]_fixture -def cleanup_generated_core_dumps(request): - """ - A fixture to cleanup core dumps intentionally generated by tests (for negative testing). - - Only core dumps generated by the decorated test function will be removed. Pre-existing - cores that need to be triaged from prior test failures are retained. - """ - possible_cores = find_all_files('*core*') - pre_test_cores = set([f for f in possible_cores if is_core_dump(f)]) - - yield # Wait for test to execute - - 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 - pre_test_cores): - LOG.info("Cleaned up {core} created by {test}".format(core=f, test=request.node.name)) - os.remove(f) - - @pytest.fixture def testid_checksum(request): """ @@ -363,8 +340,8 @@ def unique_database(request, testid_checksum): sync_ddl: boolean (defaults to False) - indicates whether the unique database should be created with sync_ddl - For a similar DB-API 2 compliant connection/cursor that uses HS2 see the 'conn' and - 'unique_cursor' fixtures below. + For a similar DB-API 2 compliant connection/cursor that uses HS2 see the 'conn' + 'cursor' fixtures below. """ # Test cases are at the function level, so no one should "accidentally" re-scope this. @@ -437,18 +414,6 @@ def unique_database(request, testid_checksum): return first_db_name [email protected] -def unique_role(request, testid_checksum): - """Returns a unique role to any test using the fixture. The fixture does not create - a role.""" - role_name_prefix = request.function.__name__ - fixture_params = getattr(request, 'param', None) - if fixture_params is not None: - if 'name_prefix' in fixture_params: - role_name_prefix = fixture_params['name_prefix'] - return '{0}_{1}_role'.format(role_name_prefix, testid_checksum) - - @pytest.fixture def unique_name(request, testid_checksum): """Returns a unique name to any test using the fixture.""" @@ -460,7 +425,7 @@ def unique_name(request, testid_checksum): return '{0}_{1}'.format(name_prefix, testid_checksum) [email protected]_fixture [email protected] def kudu_client(request): """Provides a new Kudu client as a pytest fixture. The client only exists for the duration of the method it is used in. @@ -482,7 +447,7 @@ def kudu_client(request): LOG.warn("Error closing Kudu client: %s", e) [email protected]_fixture(scope="class") [email protected](scope="class") def conn(request): """Provides a new DB-API compliant connection to Impala as a pytest fixture. The same connection is used for all test methods in a class. The class may provide the @@ -585,7 +550,7 @@ def __auto_closed_conn(pytest_config, db_name=None, timeout=DEFAULT_CONN_TIMEOUT LOG.warn("Error closing Impala connection: %s", e) [email protected]_fixture [email protected] def cursor(conn): """Provides a new DB-API compliant cursor from a connection provided by the conn() fixture. The cursor only exists for the duration of the method it is used in. @@ -601,40 +566,6 @@ def cursor(conn): yield cur [email protected]_fixture(scope="class") -def cls_cursor(conn): - """Provides a new DB-API compliant cursor from a connection provided by the conn() - fixture. The cursor exists for the duration of the class it is used in. - - The returned cursor will have a 'conn' property. The 'conn' will have a 'db_name' - property. - - DEPRECATED: - See the 'unique_database' fixture above to use Impala's custom python - API instead of DB-API. - """ - with __auto_closed_cursor(conn) as cur: - yield cur - - [email protected]_fixture -def unique_cursor(request): - """Provides a new DB-API compliant cursor to a newly created Impala database. The - cursor only exists for the duration of the method it is used in. The database will - be dropped after the test executes. - - The returned cursor will have a 'conn' property. The 'conn' will have a 'db_name' - property. - - DEPRECATED: - See the 'unique_database' fixture above to use Impala's custom python - API instead of DB-API. - """ - with __unique_conn(request.config) as conn: - with __auto_closed_cursor(conn) as cur: - yield cur - - @contextlib.contextmanager def __auto_closed_cursor(conn): """Returns a cursor created from conn. This is intended to be used in a "with" block. @@ -651,7 +582,7 @@ def __auto_closed_cursor(conn): LOG.warn("Error closing Impala cursor: %s", e) [email protected]_fixture [email protected] def impala_testinfra_cursor(): """ Return ImpalaCursor object. Used for "tests of tests" for the infra for the query @@ -669,18 +600,6 @@ def impala_testinfra_cursor(): @pytest.fixture(autouse=True, scope='session') -def validate_pytest_config(request): - """ - Validate that pytest command line options make sense. - """ - if request.config.option.testing_remote_cluster: - local_prefixes = ('localhost', '127.', '0.0.0.0') - if any(request.config.option.impalad.startswith(loc) for loc in local_prefixes): - logging.error("--testing_remote_cluster can not be used with a local impalad") - pytest.exit("Invalid pytest config option: --testing_remote_cluster") - - [email protected]_fixture(autouse=True, scope='session') def cluster_properties(): """Set up test cluster properties for the test session""" # Don't import at top level to avoid circular dependency between conftest and @@ -690,15 +609,6 @@ def cluster_properties(): yield cluster_properties [email protected](autouse=True, scope='session') -def validate_python_version(): - """Check the Python runtime version before running any tests. Since Impala switched - to the toolchain Python, which is at least v2.7, the tests will not run on a version - below that. - """ - assert sys.version_info > (2, 7), "Tests only support Python 2.7+" - - @pytest.hookimpl(trylast=True) def pytest_collection_modifyitems(items, config, session): """Hook to handle --shard_tests command line option. diff --git a/tests/custom_cluster/test_automatic_invalidation.py b/tests/custom_cluster/test_automatic_invalidation.py index 6abfff453..cde233560 100644 --- a/tests/custom_cluster/test_automatic_invalidation.py +++ b/tests/custom_cluster/test_automatic_invalidation.py @@ -29,6 +29,7 @@ from tests.common.custom_cluster_test_suite import CustomClusterTestSuite from tests.common.impala_test_suite import ImpalaTestSuite from tests.util.event_processor_utils import EventProcessorUtils + class TestAutomaticCatalogInvalidation(CustomClusterTestSuite): """ Test that tables are cached in the catalogd after usage for the configured time and invalidated afterwards.""" @@ -49,9 +50,8 @@ class TestAutomaticCatalogInvalidation(CustomClusterTestSuite): return self.cluster.catalogd.service.read_debug_webpage( "catalog_object?object_type=TABLE&object_name=functional.alltypes") - def _run_test(self, cursor): - cursor.execute(self.query) - cursor.fetchall() + def _run_test(self): + self.client.execute(self.query) # The table is cached after usage. assert self.metadata_cache_string in self._get_catalog_object() # Wait 5 * table TTL for the invalidation to take effect. @@ -65,15 +65,15 @@ class TestAutomaticCatalogInvalidation(CustomClusterTestSuite): @pytest.mark.execute_serially @CustomClusterTestSuite.with_args(catalogd_args=timeout_flag, impalad_args=timeout_flag) - def test_v1_catalog(self, cursor): - self._run_test(cursor) + def test_v1_catalog(self): + self._run_test() @pytest.mark.execute_serially @CustomClusterTestSuite.with_args( catalogd_args=timeout_flag + " --catalog_topic_mode=minimal", impalad_args=timeout_flag + " --use_local_catalog") - def test_local_catalog(self, cursor): - self._run_test(cursor) + def test_local_catalog(self): + self._run_test() @pytest.mark.execute_serially @CustomClusterTestSuite.with_args(catalogd_args="--invalidate_tables_timeout_s=1", diff --git a/tests/custom_cluster/test_calcite_planner.py b/tests/custom_cluster/test_calcite_planner.py index 30d12ac9c..93e1b8ab6 100644 --- a/tests/custom_cluster/test_calcite_planner.py +++ b/tests/custom_cluster/test_calcite_planner.py @@ -44,6 +44,7 @@ class TestCalcitePlanner(CustomClusterTestSuite): @pytest.mark.execute_serially @CustomClusterTestSuite.with_args(start_args="--env_vars=USE_CALCITE_PLANNER=true") - def test_semicolon(self, cursor): - cursor.execute("set use_calcite_planner=true;") - cursor.execute("select 4;") + def test_semicolon(self): + with self.create_impala_client() as client: + client.execute("set use_calcite_planner=true;") + client.execute("select 4;") diff --git a/tests/custom_cluster/test_client_ssl.py b/tests/custom_cluster/test_client_ssl.py index 5987627fc..7cc9e2b9d 100644 --- a/tests/custom_cluster/test_client_ssl.py +++ b/tests/custom_cluster/test_client_ssl.py @@ -172,7 +172,7 @@ class TestClientSsl(CustomClusterTestSuite): @CustomClusterTestSuite.with_args(impalad_args=TLS_ECDH_ARGS, statestored_args=TLS_ECDH_ARGS, catalogd_args=TLS_ECDH_ARGS) - @pytest.mark.skipif(SKIP_SSL_MSG is not None, reason=SKIP_SSL_MSG) + @pytest.mark.skipif(SKIP_SSL_MSG != "", reason=SKIP_SSL_MSG) def test_tls_ecdh(self, vector): self._verify_negative_cases(vector, "%s/server-cert.pem" % CERT_DIR) self._validate_positive_cases(vector, "%s/server-cert.pem" % CERT_DIR) @@ -190,7 +190,7 @@ class TestClientSsl(CustomClusterTestSuite): @CustomClusterTestSuite.with_args(impalad_args=TLS_V10_ARGS, statestored_args=TLS_V10_ARGS, catalogd_args=TLS_V10_ARGS) - @pytest.mark.skipif(SKIP_SSL_MSG is not None, reason=SKIP_SSL_MSG) + @pytest.mark.skipif(SKIP_SSL_MSG != "", reason=SKIP_SSL_MSG) def test_tls_v10(self, vector): self._validate_positive_cases(vector, "%s/server-cert.pem" % CERT_DIR) @@ -198,7 +198,7 @@ class TestClientSsl(CustomClusterTestSuite): @CustomClusterTestSuite.with_args(impalad_args=SSL_WILDCARD_ARGS, statestored_args=SSL_WILDCARD_ARGS, catalogd_args=SSL_WILDCARD_ARGS) - @pytest.mark.skipif(SKIP_SSL_MSG is not None, reason=SKIP_SSL_MSG) + @pytest.mark.skipif(SKIP_SSL_MSG != "", reason=SKIP_SSL_MSG) def test_wildcard_ssl(self, vector): """ Test for IMPALA-3159: Test with a certificate which has a wildcard for the CommonName. @@ -214,7 +214,7 @@ class TestClientSsl(CustomClusterTestSuite): @CustomClusterTestSuite.with_args(impalad_args=SSL_WILDCARD_SAN_ARGS, statestored_args=SSL_WILDCARD_SAN_ARGS, catalogd_args=SSL_WILDCARD_SAN_ARGS) - @pytest.mark.skipif(SKIP_SSL_MSG is not None, reason=SKIP_SSL_MSG) + @pytest.mark.skipif(SKIP_SSL_MSG != "", reason=SKIP_SSL_MSG) def test_wildcard_san_ssl(self, vector): """ Test for IMPALA-3159: Test with a certificate which has a wildcard as a SAN. """ # This block of code is the same as _validate_positive_cases() but we want to check @@ -311,7 +311,7 @@ class TestClientSslUnsupported(CustomClusterTestSuite): @CustomClusterTestSuite.with_args(impalad_args=SSL_ARGS, statestored_args=SSL_ARGS, catalogd_args=SSL_ARGS) - @pytest.mark.skipif(SKIP_SSL_MSG is not None, reason=SKIP_SSL_MSG) + @pytest.mark.skipif(SKIP_SSL_MSG != "", reason=SKIP_SSL_MSG) def test_shell_warning(self, vector): result = run_impala_shell_cmd_no_expect(vector, ["--ssl", "-q", "select 1 + 2"]) assert "Warning: TLSv1.2 is not supported for Python < 2.7.9" in result.stderr, \ diff --git a/tests/custom_cluster/test_events_custom_configs.py b/tests/custom_cluster/test_events_custom_configs.py index 3483bd323..d8ced8838 100644 --- a/tests/custom_cluster/test_events_custom_configs.py +++ b/tests/custom_cluster/test_events_custom_configs.py @@ -491,11 +491,11 @@ class TestEventProcessingCustomConfigs(TestEventProcessingCustomConfigsBase): else: db_1 = "{}_1".format(db) if rename_db: - self.execute_query_expect_success(self.create_impala_client(), + self.execute_query_expect_success(self.client, "drop database if exists {0} cascade".format(db_1)) - self.execute_query_expect_success(self.create_impala_client(), + self.execute_query_expect_success(self.client, "create database {0}".format(db_1)) - self.execute_query_expect_success(self.create_impala_client(), + self.execute_query_expect_success(self.client, "create table if not exists {0}.rename_test_1 (i int)".format(db)) if rename_db: queries = [ @@ -511,7 +511,7 @@ class TestEventProcessingCustomConfigs(TestEventProcessingCustomConfigsBase): create_metric_name = "tables-added" removed_metric_name = "tables-removed" elif type == "database": - self.execute_query_expect_success(self.create_impala_client(), + self.execute_query_expect_success(self.client, "drop database if exists {0}".format("test_create_drop_db")) queries = [ "create database {db}".format(db="test_create_drop_db"), @@ -521,7 +521,7 @@ class TestEventProcessingCustomConfigs(TestEventProcessingCustomConfigsBase): removed_metric_name = "databases-removed" else: tbl_name = "test_create_drop_partition" - self.execute_query_expect_success(self.create_impala_client(), + self.execute_query_expect_success(self.client, "create table {db}.{tbl} (c int) partitioned by (p int)".format( db=db, tbl=tbl_name)) queries = [ @@ -540,7 +540,7 @@ class TestEventProcessingCustomConfigs(TestEventProcessingCustomConfigsBase): for iter in range(num_iters): for q in queries: try: - self.execute_query_expect_success(self.create_impala_client(), q) + self.execute_query_expect_success(self.client, q) except Exception as e: print("Failed in {} iterations. Error {}".format(iter, str(e))) raise diff --git a/tests/custom_cluster/test_ipv6.py b/tests/custom_cluster/test_ipv6.py index ee2f90c4d..00c77dce8 100644 --- a/tests/custom_cluster/test_ipv6.py +++ b/tests/custom_cluster/test_ipv6.py @@ -199,7 +199,7 @@ class TestIPv6OnlyNoSsl(TestIPv6Base): class TestIPv6DualSsl(TestIPv6Base): ca_cert = WILDCARD_CA_CERT_PATH - @pytest.mark.skipif(SKIP_SSL_MSG is not None, reason=SKIP_SSL_MSG) + @pytest.mark.skipif(SKIP_SSL_MSG != "", reason=SKIP_SSL_MSG) def test_ipv6_dual_ssl(self, vector): self.check_connections() for port in WEBUI_PORTS: @@ -232,7 +232,7 @@ class TestIPv6DualSsl(TestIPv6Base): class TestIPv6OnlySsl(TestIPv6Base): ca_cert = WILDCARD_CA_CERT_PATH - @pytest.mark.skipif(SKIP_SSL_MSG is not None, reason=SKIP_SSL_MSG) + @pytest.mark.skipif(SKIP_SSL_MSG != "", reason=SKIP_SSL_MSG) def test_ipv6_only_ssl(self, vector): self.check_connections() for port in WEBUI_PORTS: diff --git a/tests/custom_cluster/test_restart_services.py b/tests/custom_cluster/test_restart_services.py index 3548bb200..7cc1f5a40 100644 --- a/tests/custom_cluster/test_restart_services.py +++ b/tests/custom_cluster/test_restart_services.py @@ -1066,8 +1066,8 @@ class TestGracefulShutdown(CustomClusterTestSuite, HS2TestSuite): period elapses.""" impalad = psutil.Process(self.cluster.impalads[0].get_pid()) LOG.info( - "Sending IMPALA_SHUTDOWN_SIGNAL(SIGRTMIN = {0}) signal to impalad PID = {1}", - self.IMPALA_SHUTDOWN_SIGNAL, impalad.pid) + "Sending IMPALA_SHUTDOWN_SIGNAL(SIGRTMIN = {0}) signal " + "to impalad PID = {1}".format(self.IMPALA_SHUTDOWN_SIGNAL, impalad.pid)) impalad.send_signal(self.IMPALA_SHUTDOWN_SIGNAL) # Make sure that the impala daemon exits after the shutdown grace period plus a 10 # second margin of error. @@ -1090,8 +1090,9 @@ class TestGracefulShutdown(CustomClusterTestSuite, HS2TestSuite): impalad = psutil.Process(self.cluster.impalads[0].get_pid()) NUM_SIGNALS_TO_SEND = 10 LOG.info( - "Sending {0} IMPALA_SHUTDOWN_SIGNAL(SIGRTMIN = {1}) signals to impalad PID = {2}", - NUM_SIGNALS_TO_SEND, self.IMPALA_SHUTDOWN_SIGNAL, impalad.pid) + "Sending {0} IMPALA_SHUTDOWN_SIGNAL(SIGRTMIN = {1}) signals " + "to impalad PID = {2}".format( + NUM_SIGNALS_TO_SEND, self.IMPALA_SHUTDOWN_SIGNAL, impalad.pid)) for i in range(NUM_SIGNALS_TO_SEND): impalad.send_signal(self.IMPALA_SHUTDOWN_SIGNAL) # Give shutdown thread some time to wake up and handle all the signals to avoid diff --git a/tests/custom_cluster/test_session_expiration.py b/tests/custom_cluster/test_session_expiration.py index 6694abd90..149e4ed58 100644 --- a/tests/custom_cluster/test_session_expiration.py +++ b/tests/custom_cluster/test_session_expiration.py @@ -43,20 +43,21 @@ class TestSessionExpiration(CustomClusterTestSuite): num_expired = impalad.service.get_metric_value("impala-server.num-sessions-expired") num_connections = impalad.service.get_metric_value( "impala.thrift-server.beeswax-frontend.connections-in-use") - client = impalad.service.create_hs2_client() - client.execute('select 1') - # Sleep for half the expiration time to confirm that the session is not expired early - # (see IMPALA-838) - sleep(3) - assert client is not None - assert num_expired == impalad.service.get_metric_value( - "impala-server.num-sessions-expired") - # Wait for session expiration. Impala will poll the session expiry queue every second - impalad.service.wait_for_metric_value( - "impala-server.num-sessions-expired", num_expired + 1, 20) - # Verify that the idle connection is not closed. - assert 1 + num_connections == impalad.service.get_metric_value( - "impala.thrift-server.hiveserver2-frontend.connections-in-use") + with impalad.service.create_hs2_client() as client: + client.execute('select 1') + # Sleep for half the expiration time to confirm that the session is not expired + # early (see IMPALA-838) + sleep(3) + assert client is not None + assert num_expired == impalad.service.get_metric_value( + "impala-server.num-sessions-expired") + # Wait for session expiration. Impala will poll the session expiry queue every + # second + impalad.service.wait_for_metric_value( + "impala-server.num-sessions-expired", num_expired + 1, 20) + # Verify that the idle connection is not closed. + assert 1 + num_connections == impalad.service.get_metric_value( + "impala.thrift-server.hiveserver2-frontend.connections-in-use") @pytest.mark.execute_serially @CustomClusterTestSuite.with_args("--idle_session_timeout=3 " @@ -67,18 +68,18 @@ class TestSessionExpiration(CustomClusterTestSuite): num_expired = impalad.service.get_metric_value("impala-server.num-sessions-expired") # Test if we can set a shorter timeout than the process-wide option - client = impalad.service.create_hs2_client() - client.execute("SET IDLE_SESSION_TIMEOUT=1") - sleep(2.5) - assert num_expired + 1 == impalad.service.get_metric_value( - "impala-server.num-sessions-expired") + with impalad.service.create_hs2_client() as client: + client.execute("SET IDLE_SESSION_TIMEOUT=1") + sleep(2.5) + assert num_expired + 1 == impalad.service.get_metric_value( + "impala-server.num-sessions-expired") # Test if we can set a longer timeout than the process-wide option - client = impalad.service.create_hs2_client() - client.execute("SET IDLE_SESSION_TIMEOUT=10") - sleep(5) - assert num_expired + 1 == impalad.service.get_metric_value( - "impala-server.num-sessions-expired") + with impalad.service.create_hs2_client() as client: + client.execute("SET IDLE_SESSION_TIMEOUT=10") + sleep(5) + assert num_expired + 1 == impalad.service.get_metric_value( + "impala-server.num-sessions-expired") @pytest.mark.execute_serially @CustomClusterTestSuite.with_args("--idle_session_timeout=5 " @@ -89,19 +90,19 @@ class TestSessionExpiration(CustomClusterTestSuite): num_expired = impalad.service.get_metric_value("impala-server.num-sessions-expired") # Test unsetting IDLE_SESSION_TIMEOUT - client = impalad.service.create_hs2_client() - client.execute("SET IDLE_SESSION_TIMEOUT=1") - - # Unset to 5 sec - client.execute('SET IDLE_SESSION_TIMEOUT=""') - sleep(2) - # client session should be alive at this point - assert num_expired == impalad.service.get_metric_value( - "impala-server.num-sessions-expired") - sleep(5) - # now client should have expired - assert num_expired + 1 == impalad.service.get_metric_value( - "impala-server.num-sessions-expired") + with impalad.service.create_hs2_client() as client: + client.execute("SET IDLE_SESSION_TIMEOUT=1") + + # Unset to 5 sec + client.execute('SET IDLE_SESSION_TIMEOUT=""') + sleep(2) + # client session should be alive at this point + assert num_expired == impalad.service.get_metric_value( + "impala-server.num-sessions-expired") + sleep(5) + # now client should have expired + assert num_expired + 1 == impalad.service.get_metric_value( + "impala-server.num-sessions-expired") def _get_fast_timeout_cursor_from_hs2_client(self, connection, idle_session_timeout=3): """Get a fast timing out HiveServer2Cursor from a HiveServer2Connection.""" @@ -157,18 +158,18 @@ class TestSessionExpiration(CustomClusterTestSuite): # Connect to Impala using either beeswax or HS2 client and verify the number of # opened connections. - client = impalad.service.create_client( - protocol=('hs2' if protocol == 'hiveserver2' else protocol)) - client.execute("select 1") - impalad.service.wait_for_metric_value(num_connections_metrics_name, - num_connections + 1, 20) - - # Wait till the session has expired. - impalad.service.wait_for_metric_value("impala-server.num-sessions-expired", - num_expired + 1, 20) - # Wait till the idle connection is closed. - impalad.service.wait_for_metric_value(num_connections_metrics_name, - num_connections, 5) + with impalad.service.create_client( + protocol=('hs2' if protocol == 'hiveserver2' else protocol)) as client: + client.execute("select 1") + impalad.service.wait_for_metric_value(num_connections_metrics_name, + num_connections + 1, 20) + + # Wait till the session has expired. + impalad.service.wait_for_metric_value("impala-server.num-sessions-expired", + num_expired + 1, 20) + # Wait till the idle connection is closed. + impalad.service.wait_for_metric_value(num_connections_metrics_name, + num_connections, 5) # Verify that connecting to HS2 port without establishing a session will not cause # the connection to be closed. @@ -194,28 +195,26 @@ class TestSessionExpiration(CustomClusterTestSuite): impalad = self.cluster.get_first_impalad() self.close_impala_clients() # Create 2 clients. - client1 = impalad.service.create_hs2_client() - client2 = impalad.service.create_hs2_client() - # Run query to open session. - client1.execute('select "client1 should succeed"') - client2.execute('select "client2 should succeed"') - try: - # Trying to open a third session should fail. - client3 = impalad.service.create_hs2_client() - client3.execute('select "client3 should fail"') - assert False, "should have failed" - except Exception as e: - assert re.match(r".*Number of sessions for user \S+ exceeds coordinator limit 2", - str(e), re.DOTALL), "Unexpected exception: " + str(e) - - # Test webui for hs2 sessions. - res = impalad.service.get_debug_webpage_json("/sessions") - assert res['num_sessions'] == 2 - assert res['users'][0]['user'] is not None - assert res['users'][0]['session_count'] == 2 - # Let queries finish, session count should go to zero. - sleep(6) - client1.close() - client2.close() + with impalad.service.create_hs2_client() as client1, \ + impalad.service.create_hs2_client() as client2: + # Run query to open session. + client1.execute('select "client1 should succeed"') + client2.execute('select "client2 should succeed"') + try: + # Trying to open a third session should fail. + client3 = impalad.service.create_hs2_client() + client3.execute('select "client3 should fail"') + assert False, "should have failed" + except Exception as e: + assert re.match(r".*Number of sessions for user \S+ exceeds coordinator limit 2", + str(e), re.DOTALL), "Unexpected exception: " + str(e) + + # Test webui for hs2 sessions. + res = impalad.service.get_debug_webpage_json("/sessions") + assert res['num_sessions'] == 2 + assert res['users'][0]['user'] is not None + assert res['users'][0]['session_count'] == 2 + # Let queries finish, session count should go to zero. + sleep(6) res = impalad.service.get_debug_webpage_json("/sessions") assert res['num_sessions'] == 0 diff --git a/tests/pytest.ini b/tests/pytest.ini index 2b77ee8fa..d16434849 100644 --- a/tests/pytest.ini +++ b/tests/pytest.ini @@ -1,5 +1,6 @@ [pytest] -addopts = -r xfE -v --tb=short --showlocals +addopts = -r xfE -v --tb=short --showlocals -Wonce +junit_logging = system-err markers = execute_serially: tests that must not be executed in parallel stress: stress tests running multiple clients with the test_index parameter, will be run independent of other tests diff --git a/tests/query_test/test_observability.py b/tests/query_test/test_observability.py index ce1e0fab7..6f02be978 100644 --- a/tests/query_test/test_observability.py +++ b/tests/query_test/test_observability.py @@ -168,7 +168,7 @@ class TestObservability(ImpalaTestSuite): assert "Query Options (set by configuration): MEM_LIMIT=8589934592" in profile,\ profile assert "CLIENT_IDENTIFIER=" + \ - "query_test/test_observability.py::TestObservability::()::test_query_options" \ + "query_test/test_observability.py::TestObservability::test_query_options" \ in profile # Get the TIMEZONE value. server_timezone = None @@ -186,7 +186,7 @@ class TestObservability(ImpalaTestSuite): "NUM_NODES=1,NUM_SCANNER_THREADS=1," "RUNTIME_FILTER_MODE=OFF,MT_DOP=0,TIMEZONE={timezone}," "CLIENT_IDENTIFIER=" - "query_test/test_observability.py::TestObservability::()::test_query_options," + "query_test/test_observability.py::TestObservability::test_query_options," "SPOOL_QUERY_RESULTS=0" "\n") expected_str = expected_str.format(timezone=server_timezone) diff --git a/tests/run-custom-cluster-tests.sh b/tests/run-custom-cluster-tests.sh index 1484bb6a7..0850187e1 100755 --- a/tests/run-custom-cluster-tests.sh +++ b/tests/run-custom-cluster-tests.sh @@ -25,7 +25,6 @@ set -euo pipefail setup_report_build_error # Disable HEAPCHECK for the process failure tests because they can cause false positives. -# TODO: Combine with run-process-failure-tests.sh export HEAPCHECK= export LOG_DIR="${IMPALA_CUSTOM_CLUSTER_TEST_LOGS_DIR}" @@ -48,7 +47,7 @@ then ARGS+=("${AUX_CUSTOM_DIR}") fi ARGS+=("--junitxml=${RESULTS_DIR}/TEST-impala-custom-cluster.xml") -ARGS+=("--resultlog=${RESULTS_DIR}/TEST-impala-custom-cluster.log") +ARGS+=("--report-log=${RESULTS_DIR}/TEST-impala-custom-cluster.log") ARGS+=("$@") impala-py.test "${ARGS[@]}" diff --git a/tests/run-process-failure-tests.sh b/tests/run-process-failure-tests.sh deleted file mode 100755 index db4757172..000000000 --- a/tests/run-process-failure-tests.sh +++ /dev/null @@ -1,36 +0,0 @@ -#!/bin/bash -# -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. -# -# Runs the Impala process failure tests. - -set -euo pipefail -. $IMPALA_HOME/bin/report_build_error.sh -setup_report_build_error - -# Disable HEAPCHECK for the process failure tests because they can cause false positives. -export HEAPCHECK= - -RESULTS_DIR="${IMPALA_LOGS_DIR}/process_failure_tests" -mkdir -p "${RESULTS_DIR}" - -cd "${IMPALA_HOME}/tests" -. "${IMPALA_HOME}/bin/set-classpath.sh" &> /dev/null -impala-py.test experiments/test_process_failures.py \ - --junitxml="\"${RESULTS_DIR}/TEST-impala-proc-failure.xml\"" \ - --resultlog="\"${RESULTS_DIR}/TEST-impala-proc-failure.log\"" "$@" diff --git a/tests/run-tests.py b/tests/run-tests.py index b63dc9bfc..90e4a1855 100755 --- a/tests/run-tests.py +++ b/tests/run-tests.py @@ -32,9 +32,8 @@ import multiprocessing import os import pytest import sys -from _pytest.main import EXIT_NOTESTSCOLLECTED -# change to "from _pytest.config.argparsing import Parser" in pytest >= 6.2.0 -from _pytest.config import Parser +from _pytest.config import ExitCode +from _pytest.config.argparsing import Parser # We whitelist valid test directories. If a new test directory is added, update this. VALID_TEST_DIRS = ['failure', 'query_test', 'stress', 'unittests', 'aux_query_tests', @@ -57,7 +56,7 @@ RESULT_DIR = os.path.join(os.environ['IMPALA_EE_TEST_LOGS_DIR'], 'results') # Arguments that control output logging. If additional default arguments are needed they # should go in the pytest.ini file. LOGGING_ARGS = {'--junitxml': 'TEST-impala-{0}.xml', - '--resultlog': 'TEST-impala-{0}.log'} + '--report-log': 'TEST-impala-{0}.log'} # Default the number of concurrent tests defaults to the cpu cores in the system. # This can be overridden by setting the NUM_CONCURRENT_TESTS environment variable. @@ -75,6 +74,7 @@ NUM_STRESS_CLIENTS = min(multiprocessing.cpu_count() * 4, 64) if 'NUM_STRESS_CLIENTS' in os.environ: NUM_STRESS_CLIENTS = int(os.environ['NUM_STRESS_CLIENTS']) + class TestCounterPlugin(object): """ Custom pytest plugin to count the number of tests collected and executed over multiple pytest runs @@ -87,21 +87,23 @@ class TestCounterPlugin(object): self.tests_executed = set() # pytest hook to handle test collection when xdist is used (parallel tests) - # https://github.com/pytest-dev/pytest-xdist/pull/35/commits (No official documentation available) - def pytest_xdist_node_collection_finished(self, node, ids): + # https://github.com/pytest-dev/pytest-xdist/pull/35/commits + # (No official documentation available) + def pytest_xdist_node_collection_finished(self, node, ids): # noqa: U100 self.tests_collected.update(set(ids)) # link to pytest_collection_modifyitems - # https://docs.pytest.org/en/2.9.2/writing_plugins.html#_pytest.hookspec.pytest_collection_modifyitems + # https://docs.pytest.org/en/6.2.x/writing_plugins.html#hook-function-validation-and-execution def pytest_collection_modifyitems(self, items): for item in items: self.tests_collected.add(item.nodeid) # link to pytest_runtest_logreport - # https://docs.pytest.org/en/2.9.2/_modules/_pytest/hookspec.html#pytest_runtest_logreport + # https://docs.pytest.org/en/6.2.x/_modules/_pytest/hookspec.html#pytest_runtest_logreport def pytest_runtest_logreport(self, report): if report.passed: - self.tests_executed.add(report.nodeid) + self.tests_executed.add(report.nodeid) + class TestExecutor(object): def __init__(self, exit_on_error=True): @@ -113,8 +115,9 @@ class TestExecutor(object): testcounterplugin = TestCounterPlugin() try: - pytest_exit_code = pytest.main(args, plugins=[testcounterplugin]) - except: + string_args = [str(a) for a in args] + pytest_exit_code = pytest.main(string_args, plugins=[testcounterplugin]) + except Exception: sys.stderr.write("Unexpected exception with pytest {0}".format(args)) raise @@ -124,12 +127,19 @@ class TestExecutor(object): self.total_executed += len(testcounterplugin.tests_executed) - if 0 < pytest_exit_code < EXIT_NOTESTSCOLLECTED and self._exit_on_error: + if 0 < pytest_exit_code < ExitCode.NO_TESTS_COLLECTED and self._exit_on_error: sys.exit(pytest_exit_code) - self.tests_failed = 0 < pytest_exit_code < EXIT_NOTESTSCOLLECTED or self.tests_failed + self.tests_failed = (0 < pytest_exit_code < ExitCode.NO_TESTS_COLLECTED + or self.tests_failed) def get_explicit_tests(args): + """ Returns a list of explicitly specified tests from the given args. + + Note that access to _pytest.config.argparsing.Parser() constructor is deprecated + in pytest 7.0. + https://docs.pytest.org/en/stable/deprecations.html#directly-constructing-internal-classes + """ pytest_arg_parser = Parser() parsed_args = pytest_arg_parser.parse_known_args(args) return parsed_args.file_or_dir @@ -204,7 +214,7 @@ def build_test_args(base_name, valid_dirs=VALID_TEST_DIRS): # We also want to strip out any --shard_tests option and its corresponding value. while "--shard_tests" in config_options: i = config_options.index("--shard_tests") - del config_options[i:i+2] + del config_options[i:i + 2] test_args = ignored_dirs + logging_args + config_options return test_args @@ -280,6 +290,10 @@ if __name__ == "__main__": test_executor.run_tests(args) os.chdir(TEST_DIR) + # Fix rootdir and pytest.ini path programmatically because individual pytest-xdist + # worker often does not pick this up correctly during parallel run. + pytest_ini_path = os.path.join(TEST_DIR, 'pytest.ini') + conf_args = ["--rootdir", TEST_DIR, '-c', pytest_ini_path] # Create the test result directory if it doesn't already exist. if not os.path.exists(RESULT_DIR): @@ -290,7 +304,7 @@ if __name__ == "__main__": # pytest warnings/messages and displays collected tests if '--collect-only' in sys.argv: - run(sys.argv[1:]) + run(conf_args + sys.argv[1:]) else: print_metrics('connections') @@ -317,19 +331,20 @@ if __name__ == "__main__": # First run query tests that need to be executed serially if not skip_serial: - base_args = ['-m', 'execute_serially'] + base_args = conf_args + ['-m', 'execute_serially'] run(base_args + build_test_args("serial{0}".format(shard_identifier))) print_metrics('connections') # Run the stress tests if not skip_stress: - base_args = ['-m', 'stress', '-n', NUM_STRESS_CLIENTS] + base_args = conf_args + ['-m', 'stress', '-n', NUM_STRESS_CLIENTS] run(base_args + build_test_args("stress{0}".format(shard_identifier))) print_metrics('connections') # Run the remaining query tests in parallel if not skip_parallel: - base_args = ['-m', 'not execute_serially and not stress', '-n', NUM_CONCURRENT_TESTS] + base_args = conf_args + ['-m', 'not execute_serially and not stress', + '-n', NUM_CONCURRENT_TESTS] run(base_args + build_test_args("parallel{0}".format(shard_identifier))) # The total number of tests executed at this point is expected to be >0 @@ -343,7 +358,7 @@ if __name__ == "__main__": args = build_test_args(base_name="verify-metrics{0}".format(shard_identifier), valid_dirs=['verifiers']) args.append('verifiers/test_verify_metrics.py') - run(args) + run(conf_args + args) if test_executor.tests_failed: sys.exit(1) diff --git a/tests/shell/test_shell_commandline.py b/tests/shell/test_shell_commandline.py index 4a69c67f5..095ec4ed5 100644 --- a/tests/shell/test_shell_commandline.py +++ b/tests/shell/test_shell_commandline.py @@ -134,7 +134,7 @@ def populated_table(empty_table, request): return fq_table_name [email protected]_fixture [email protected] def tmp_file(): """ Test fixture which manages a temporary file diff --git a/tests/shell/test_shell_interactive.py b/tests/shell/test_shell_interactive.py index be56147f2..01164b258 100755 --- a/tests/shell/test_shell_interactive.py +++ b/tests/shell/test_shell_interactive.py @@ -132,7 +132,7 @@ class RequestHandler503Extra(RequestHandler503): return True -class TestHTTPServer503(object): +class HTTPServer503(object): def __init__(self, clazz): self.HOST = "localhost" self.PORT = get_unused_port() @@ -150,21 +150,21 @@ def shutdown_server(server): server.http_server_thread.join() [email protected]_fixture [email protected] def http_503_server(): """A fixture that creates an http server that returns a 503 http code.""" - server = TestHTTPServer503(RequestHandler503) + server = HTTPServer503(RequestHandler503) yield server # Cleanup after test. shutdown_server(server) [email protected]_fixture [email protected] def http_503_server_extra(): """A fixture that creates an http server that returns a 503 http code with extra body text.""" - server = TestHTTPServer503(RequestHandler503Extra) + server = HTTPServer503(RequestHandler503Extra) yield server # Cleanup after test. @@ -704,8 +704,9 @@ class TestImpalaShellInteractive(ImpalaTestSuite): self._expect_with_cmd(child_proc, "select 'hi'", vector, ('hi')) child_proc.sendline('exit;') child_proc.expect(pexpect.EOF) - history_contents = open(new_hist.name).read() - assert "select 'hi'" in history_contents + with open(new_hist.name) as f: + history_contents = f.read() + assert "select 'hi'" in history_contents def test_rerun(self, vector, tmp_history_file): # noqa: U100 """Smoke test for the 'rerun' command""" @@ -753,10 +754,11 @@ class TestImpalaShellInteractive(ImpalaTestSuite): assert False, "No tip found in output %s" % result.stderr def test_var_substitution(self, vector): - cmds = open(os.path.join(QUERY_FILE_PATH, 'test_var_substitution.sql')).read() - args = ["--var=foo=123", "--var=BAR=456", "--delimited", "--output_delimiter= "] - result = run_impala_shell_interactive(vector, cmds, shell_args=args) - assert_var_substitution(result) + with open(os.path.join(QUERY_FILE_PATH, 'test_var_substitution.sql')) as f: + cmds = f.read() + args = ["--var=foo=123", "--var=BAR=456", "--delimited", "--output_delimiter= "] + result = run_impala_shell_interactive(vector, cmds, shell_args=args) + assert_var_substitution(result) def test_query_option_configuration(self, vector): if vector.get_value('strict_hs2_protocol'): diff --git a/tests/shell/util.py b/tests/shell/util.py index 16fee4509..455b0ad98 100755 --- a/tests/shell/util.py +++ b/tests/shell/util.py @@ -378,7 +378,7 @@ def get_dev_impala_shell_executable(): def create_impala_shell_executable_dimension(dev_only=False): _, include_pypi = get_dev_impala_shell_executable() dimensions = [] - python3_pytest = (os.getenv("IMPALA_USE_PYTHON3_TESTS", "false") == "true") + python3_pytest = (os.getenv("IMPALA_USE_PYTHON3_TESTS", "true") == "true") assert os.getenv("IMPALA_SYSTEM_PYTHON3"), "Must set env var IMPALA_SYSTEM_PYTHON3!" dimensions.append('dev3') if os.getenv("IMPALA_SYSTEM_PYTHON2") and not python3_pytest: diff --git a/tests/util/auto_scaler.py b/tests/util/auto_scaler.py index c6d95d1d8..b39e53665 100755 --- a/tests/util/auto_scaler.py +++ b/tests/util/auto_scaler.py @@ -25,7 +25,6 @@ import os import pipes from subprocess import check_call from tests.common.impala_cluster import ImpalaCluster -from tests.util.filesystem_utils import IS_EC from threading import Event, Thread IMPALA_HOME = os.environ["IMPALA_HOME"] @@ -90,9 +89,6 @@ class AutoScaler(object): def get_service(self): return self.get_coordinator().service - def get_client(self): - return self.get_coordinator().service.create_hs2_client() - def group_name(self, idx): # By convention, group names must start with their associated resource pool name # followed by a "-". @@ -163,7 +159,8 @@ class AutoScaler(object): return self.get_service().get_executor_groups() def execute(self, query): - return self.get_client().execute(query) + with self.get_coordinator().service.create_hs2_client() as client: + return client.execute(query) def get_num_queued_queries(self): """Returns the number of queries currently queued in the default pool on the diff --git a/tests/util/concurrent_workload.py b/tests/util/concurrent_workload.py index b0b81cbbd..1621eafac 100755 --- a/tests/util/concurrent_workload.py +++ b/tests/util/concurrent_workload.py @@ -63,8 +63,8 @@ class ConcurrentWorkload(object): cluster = ImpalaCluster.get_e2e_test_cluster() if len(cluster.impalads) == 0: raise Exception("Coordinator not running") - client = cluster.get_first_impalad().service.create_hs2_client() - return client.execute(query) + with cluster.get_first_impalad().service.create_hs2_client() as client: + return client.execute(query) def loop_query(self, query, output_q, stop_ev): """Executes 'query' in a loop while 'stop_ev' is not set and inserts the result into @@ -113,12 +113,12 @@ class ConcurrentWorkload(object): def print_query_rate(self): """Prints the current query throughput until user presses ctrl-c.""" try: - self._print_query_rate(self.output_q, self.stop_ev) + self._print_query_rate(self.stop_ev) except KeyboardInterrupt: self.stop() assert self.stop_ev.is_set(), "Stop event expected to be set but it isn't" - def _print_query_rate(self, queue_obj, stop_ev): + def _print_query_rate(self, stop_ev): """Prints the query throughput rate until 'stop_ev' is set by the caller.""" PERIOD_S = 1 diff --git a/tests/util/hdfs_util.py b/tests/util/hdfs_util.py index 1dd2ce4e2..f9e9375b3 100644 --- a/tests/util/hdfs_util.py +++ b/tests/util/hdfs_util.py @@ -41,7 +41,7 @@ class HdfsConfig(object): self.conf = {} for arg in filename: tree = parse(arg) - for property in tree.getroot().getiterator('property'): + for property in tree.getroot().iter('property'): self.conf[property.find('name').text] = property.find('value').text def get(self, key):
