Abacn commented on code in PR #36852:
URL: https://github.com/apache/beam/pull/36852#discussion_r2550380276
##########
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 0.1 s overhead comes from here:
https://github.com/apache/beam/pull/35889/files#diff-76dd9e3509f907a47fc83cfd5094ffdfeed3101f73ac31bfffe113605aef0b46R135
I feel skeptical about using busy wait to solve issues in general. I would
suggest revert ensure_clean_state and enhance_mock_stability fixtures or move
then to per-test-class based (if possible)
--
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]
