aIbrahiim commented on code in PR #36852:
URL: https://github.com/apache/beam/pull/36852#discussion_r2550307313
##########
sdks/python/conftest.py:
##########
@@ -101,56 +110,86 @@ def configure_beam_rpc_timeouts():
print("Successfully configured Beam RPC timeouts")
+def _running_in_ci():
+ """Returns True if running in a CI environment."""
+ return (
+ os.getenv('GITHUB_ACTIONS') == 'true' or
+ os.getenv('CI') == 'true' or
+ os.getenv('CONTINUOUS_INTEGRATION') == 'true'
+ )
+
+
+def _should_enable_test_cleanup(config):
+ """Returns True if expensive cleanup operations should run.
+
+ Result is cached on config object to avoid re-computation per test.
+ """
+ if hasattr(config, '_should_enable_test_cleanup_result'):
+ return config._should_enable_test_cleanup_result
+
+ if config.getoption('--enable-test-cleanup'):
+ result = True
+ reason = "enabled via --enable-test-cleanup"
+ else:
+ if _running_in_ci():
+ result = True
+ reason = "CI detected"
+ else:
+ result = False
+ reason = "local development"
+
+ # Log once per session
+ if not hasattr(config, '_cleanup_decision_logged'):
+ print(f"\n[Test Cleanup] Enabled: {result} ({reason})")
+ config._cleanup_decision_logged = True
+
+ config._should_enable_test_cleanup_result = result
+ return result
+
+
@pytest.fixture(autouse=True)
-def ensure_clean_state():
+def ensure_clean_state(request):
"""
- Ensure clean state before each test
- to prevent cross-test contamination.
+ Ensures clean state between tests to prevent contamination.
+
+ Expensive operations (sleeps, extra GC) only run in CI or when
+ explicitly enabled to keep local tests fast.
"""
- import gc
- import threading
- import time
+ enable_cleanup = _should_enable_test_cleanup(request.config)
- # Force garbage collection to clean up any lingering resources
- gc.collect()
+ if enable_cleanup:
Review Comment:
The thing is as discussed in the issue
[here](https://github.com/apache/beam/issues/36222#issuecomment-2393868683),
the
ensure_clean_state
adds significant overhead (~100ms/test). While we need this strict
isolation in CI to prevent flakiness (hence enable_cleanup is True), it makes
running tests locally very painful.
and this primplements the suggested interim solution by keeping the strict
cleanup in CI (so CI remains stable) while disabling it by default locally so
it can iterate more quickly.
##########
sdks/python/conftest.py:
##########
@@ -101,56 +110,86 @@ def configure_beam_rpc_timeouts():
print("Successfully configured Beam RPC timeouts")
+def _running_in_ci():
+ """Returns True if running in a CI environment."""
+ return (
+ os.getenv('GITHUB_ACTIONS') == 'true' or
+ os.getenv('CI') == 'true' or
+ os.getenv('CONTINUOUS_INTEGRATION') == 'true'
+ )
+
+
+def _should_enable_test_cleanup(config):
+ """Returns True if expensive cleanup operations should run.
+
+ Result is cached on config object to avoid re-computation per test.
+ """
+ if hasattr(config, '_should_enable_test_cleanup_result'):
+ return config._should_enable_test_cleanup_result
+
+ if config.getoption('--enable-test-cleanup'):
+ result = True
+ reason = "enabled via --enable-test-cleanup"
+ else:
+ if _running_in_ci():
+ result = True
+ reason = "CI detected"
+ else:
+ result = False
+ reason = "local development"
+
+ # Log once per session
+ if not hasattr(config, '_cleanup_decision_logged'):
+ print(f"\n[Test Cleanup] Enabled: {result} ({reason})")
+ config._cleanup_decision_logged = True
+
+ config._should_enable_test_cleanup_result = result
+ return result
+
+
@pytest.fixture(autouse=True)
-def ensure_clean_state():
+def ensure_clean_state(request):
"""
- Ensure clean state before each test
- to prevent cross-test contamination.
+ Ensures clean state between tests to prevent contamination.
+
+ Expensive operations (sleeps, extra GC) only run in CI or when
+ explicitly enabled to keep local tests fast.
"""
- import gc
- import threading
- import time
+ enable_cleanup = _should_enable_test_cleanup(request.config)
- # Force garbage collection to clean up any lingering resources
- gc.collect()
+ if enable_cleanup:
Review Comment:
The thing is as discussed in the issue
[here](https://github.com/apache/beam/issues/36222#issuecomment-2393868683),
the ensure_clean_state adds significant overhead (~100ms/test). While we need
this strict isolation in CI to prevent flakiness (hence enable_cleanup is
True), it makes running tests locally very painful and this pr implements the
suggested interim solution by keeping the strict cleanup in CI (so CI remains
stable) while disabling it by default locally so it can iterate more quickly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
