This is an automated email from the ASF dual-hosted git repository.
bolke pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 7e7ac10947 Fix cache_clear behaviour in all fixtures and tests (#35746)
7e7ac10947 is described below
commit 7e7ac10947554f2b993aa1947f8e2ca5bc35f23e
Author: Jarek Potiuk <ja...@potiuk.com>
AuthorDate: Mon Nov 20 09:24:35 2023 +0100
Fix cache_clear behaviour in all fixtures and tests (#35746)
When running test with pytest-xdist, tests can be executed in random
order in multiple workers. This means that if a test leaves
a side-effect, it might affect another test - much more easily,
because you are not able to rely on the sequence of test
execution.
The `recalculate_patterns` fixture in "test_serde" had such a
side effect and caused #35699 to happen.
It was used in order to clear the cache BEFORE the test so that
it could reload the patterns using different configugration
provided by the test as new configuration.
The side effect of it was however such, that after the test completed,
the new configuration remained in the cache AFTER the test has been
completed - thus if THE SAME xdist WORKER started a test that relied on
the cache containting "regular" value (and it did not use the same
fixtuere to clean it), it - surpisingly - got the value that was read
from the special configuration provided by the previous test.
This PR fixes it by clearing the cache also AFTER the test has
completed, which means that any other test running in the same
worker will refresh the test again - with the configuration
it is supposed to have. This is done with try/finally pattern,
because context manager / yield will not execute the post-yield
code when an exception is thrown, so if there is a failed
assertion, the cache_clear afer yield might not happen and such
`cache_clear` test that fails might still cause cache pollution.
Fixes: #35699
---
tests/conftest.py | 9 +++++-
.../executors/test_kubernetes_executor.py | 34 ++++++++++++----------
.../providers/openlineage/extractors/test_bash.py | 4 +++
.../openlineage/extractors/test_python.py | 4 +++
tests/serialization/test_serde.py | 4 +++
tests/utils/log/test_secrets_masker.py | 6 ++--
6 files changed, 43 insertions(+), 18 deletions(-)
diff --git a/tests/conftest.py b/tests/conftest.py
index 8fd97e95a0..b48c51a0cb 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1075,7 +1075,14 @@ def clear_lru_cache():
from airflow.utils.entry_points import _get_grouped_entry_points
ExecutorLoader.validate_database_executor_compatibility.cache_clear()
- _get_grouped_entry_points.cache_clear()
+ try:
+ _get_grouped_entry_points.cache_clear()
+ try:
+ yield
+ finally:
+ _get_grouped_entry_points.cache_clear()
+ finally:
+ ExecutorLoader.validate_database_executor_compatibility.cache_clear()
@pytest.fixture(autouse=True)
diff --git
a/tests/providers/cncf/kubernetes/executors/test_kubernetes_executor.py
b/tests/providers/cncf/kubernetes/executors/test_kubernetes_executor.py
index f28902b185..35584a4d82 100644
--- a/tests/providers/cncf/kubernetes/executors/test_kubernetes_executor.py
+++ b/tests/providers/cncf/kubernetes/executors/test_kubernetes_executor.py
@@ -1243,16 +1243,18 @@ class TestKubernetesExecutor:
"try_number": "1",
}
get_logs_task_metadata.cache_clear()
- with conf_vars({("kubernetes", "logs_task_metadata"): "True"}):
- expected_annotations = {
- "dag_id": "dag",
- "run_id": "run_id",
- "task_id": "task",
- "try_number": "1",
- }
- annotations_actual =
annotations_for_logging_task_metadata(annotations_test)
- assert annotations_actual == expected_annotations
- get_logs_task_metadata.cache_clear()
+ try:
+ with conf_vars({("kubernetes", "logs_task_metadata"): "True"}):
+ expected_annotations = {
+ "dag_id": "dag",
+ "run_id": "run_id",
+ "task_id": "task",
+ "try_number": "1",
+ }
+ annotations_actual =
annotations_for_logging_task_metadata(annotations_test)
+ assert annotations_actual == expected_annotations
+ finally:
+ get_logs_task_metadata.cache_clear()
def test_annotations_for_logging_task_metadata_fallback(self):
annotations_test = {
@@ -1262,11 +1264,13 @@ class TestKubernetesExecutor:
"try_number": "1",
}
get_logs_task_metadata.cache_clear()
- with conf_vars({("kubernetes", "logs_task_metadata"): "False"}):
- expected_annotations = "<omitted>"
- annotations_actual =
annotations_for_logging_task_metadata(annotations_test)
- assert annotations_actual == expected_annotations
- get_logs_task_metadata.cache_clear()
+ try:
+ with conf_vars({("kubernetes", "logs_task_metadata"): "False"}):
+ expected_annotations = "<omitted>"
+ annotations_actual =
annotations_for_logging_task_metadata(annotations_test)
+ assert annotations_actual == expected_annotations
+ finally:
+ get_logs_task_metadata.cache_clear()
class TestKubernetesJobWatcher:
diff --git a/tests/providers/openlineage/extractors/test_bash.py
b/tests/providers/openlineage/extractors/test_bash.py
index 201583c0ed..4919f2b873 100644
--- a/tests/providers/openlineage/extractors/test_bash.py
+++ b/tests/providers/openlineage/extractors/test_bash.py
@@ -46,6 +46,10 @@ with DAG(
@pytest.fixture(autouse=True, scope="function")
def clear_cache():
is_source_enabled.cache_clear()
+ try:
+ yield
+ finally:
+ is_source_enabled.cache_clear()
def test_extract_operator_bash_command_disables_without_env():
diff --git a/tests/providers/openlineage/extractors/test_python.py
b/tests/providers/openlineage/extractors/test_python.py
index 45cb35288f..f90366e58e 100644
--- a/tests/providers/openlineage/extractors/test_python.py
+++ b/tests/providers/openlineage/extractors/test_python.py
@@ -61,6 +61,10 @@ CODE = "def callable():\n print(10)\n"
@pytest.fixture(autouse=True, scope="function")
def clear_cache():
is_source_enabled.cache_clear()
+ try:
+ yield
+ finally:
+ is_source_enabled.cache_clear()
def test_extract_source_code():
diff --git a/tests/serialization/test_serde.py
b/tests/serialization/test_serde.py
index 23b4a2ea03..dc3d3faf1e 100644
--- a/tests/serialization/test_serde.py
+++ b/tests/serialization/test_serde.py
@@ -44,6 +44,10 @@ from tests.test_utils.config import conf_vars
@pytest.fixture()
def recalculate_patterns():
_get_patterns.cache_clear()
+ try:
+ yield
+ finally:
+ _get_patterns.cache_clear()
class Z:
diff --git a/tests/utils/log/test_secrets_masker.py
b/tests/utils/log/test_secrets_masker.py
index c250229687..ffaf2977ae 100644
--- a/tests/utils/log/test_secrets_masker.py
+++ b/tests/utils/log/test_secrets_masker.py
@@ -357,8 +357,10 @@ class TestShouldHideValueForKey:
with conf_vars({("core", "sensitive_var_conn_names"):
str(sensitive_variable_fields)}):
get_sensitive_variables_fields.cache_clear()
- assert expected_result == should_hide_value_for_key(key)
- get_sensitive_variables_fields.cache_clear()
+ try:
+ assert expected_result == should_hide_value_for_key(key)
+ finally:
+ get_sensitive_variables_fields.cache_clear()
class ShortExcFormatter(logging.Formatter):
