dsuhinin commented on code in PR #68393: URL: https://github.com/apache/airflow/pull/68393#discussion_r3659044702
##########
airflow-core/tests/unit/core/test_settings.py:
##########
@@ -533,3 +533,50 @@ def test_early_return_when_all_none(self):
settings.dispose_orm(do_log=False)
mock_close.assert_not_called()
+
+
+class TestInitializeStats:
Review Comment:
Fair point, and I've tightened this up in two ways.
On the aside: fixed — test_stats_initialized_during_initialize now mocks
all the other initialize() steps (configure_orm, configure_logging,
configure_otel, etc.), so nothing real runs in the unit test.
On the main concern: the removed tests asserted the call sites (lifespan /
DFP run() / triggerer execute() calling stats.initialize), and those call sites
are what this PR deletes — so they can't survive in that form. The property
they indirectly guarded — "every component process ends up with a working Stats
backend" — now decomposes into exactly two invariants, and both are tested
directly:
1. every process that does import airflow gets Stats initialized —
TestInitializeStats (this file);
2. every forked child rebuilds its own backend — TestBackendResetAfterFork
in the shared observability tests does a real os.fork() and asserts the child's
backend is reset and rebuilt through the factory.
Since components no longer carry any stats-init code of their own, there's
no component-specific behaviour left that could regress independently of those
two invariants. The gunicorn preload case (the one place where "import happens
in a different process than the work") is additionally covered by the
multi-worker run described in the thread above.
--
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]
