ci(python-unit): run the unit suite in parallel with pytest-xdist (#44178)

Co-authored-by: Mike Bridge <michael.bridge@ext.preset.io>
Co-authored-by: Claude Code <noreply@anthropic.com>

Author: 3 people

.github/workflows/superset-python-unittest.yml

@@ -63,12 +63,23 @@ jobs:
         uses: $/.github/actions/setup-backend/
         with:
           python-version: ${{ matrix.python-version }}
+      # Runs in parallel via pytest-xdist: the serial suite had grown to
+      # ~27-28 min against this job's 30-minute timeout, so the trailing
+      # coverage/upload steps were being killed on ordinary runner variance
+      # even though every test passed. `--dist loadfile` keeps all tests from
+      # one file on one worker, which preserves module-scoped fixture state;
+      # pytest-cov combines the per-worker coverage data automatically.
+      # `--durations=0` makes `--durations-min` effective (the min flag only
+      # filters an enabled report) so the slow-file critical path under
+      # loadfile stays visible. `--maxfail` is a controller-level stop: tests
+      # already dispatched to workers finish, so the final failure count can
+      # slightly exceed it.
       - name: Python unit tests
         env:
           SUPERSET_TESTENV: true
           SUPERSET_SECRET_KEY: not-a-secret
         run: |
-          pytest --durations-min=0.5 --cov-report= --cov=superset ./tests/common ./tests/unit_tests --cache-clear --maxfail=50 --junit-xml=test-results/junit-unit.xml
+          pytest -n auto --dist loadfile --durations=0 --durations-min=0.5 --cov-report= --cov=superset ./tests/common ./tests/unit_tests --cache-clear --maxfail=50 --junit-xml=test-results/junit-unit.xml
       # COVERAGE_FILE keeps these scoped gates off the default .coverage that
       # the step above wrote. pytest-cov starts a fresh data file per run, so
       # without it the last gate replaces the full-suite data and the report

pyproject.toml

@@ -326,6 +326,7 @@ development = [
     "pytest-asyncio",
     "pytest-cov",
     "pytest-mock",
+    "pytest-xdist",
     "python-ldap>=3.4.7",
     "ruff",
     "sqloxide",

requirements/development.txt

@@ -274,6 +274,8 @@ events==0.5
     # via opensearch-py
 exceptiongroup==1.3.0
     # via fastmcp-slim
+execnet==2.1.2
+    # via pytest-xdist
 fastmcp==3.4.7
     # via apache-superset
 fastmcp-slim==3.4.7
@@ -890,6 +892,7 @@ pytest==7.4.4
     #   pytest-asyncio
     #   pytest-cov
     #   pytest-mock
+    #   pytest-xdist
 pytest-asyncio==0.23.8
     # via apache-superset
 pytest-cov==6.0.0
@@ -900,6 +903,8 @@ pytest-mock==3.10.0
     # via
     #   apache-superset
     #   apache-superset-extensions-cli
+pytest-xdist==3.8.0
+    # via apache-superset
 python-calamine==0.8.2
     # via
     #   -c requirements/base-constraint.txt

tests/unit_tests/commands/report/base_test.py

@@ -33,27 +33,33 @@
 )
 from superset.reports.models import ReportScheduleType
 
-REPORT_TYPES = {
+# Tuples, not sets: these feed ``@pytest.mark.parametrize``, and pytest-xdist
+# requires every worker to collect the same test ids in the same order. A set
+# of strings iterates in per-process hash order (PYTHONHASHSEED), so a set here
+# makes workers disagree and xdist aborts with "Different tests were collected".
+REPORT_TYPES: tuple[ReportScheduleType, ...] = (
     ReportScheduleType.ALERT,
     ReportScheduleType.REPORT,
-}
+)
 
-TEST_SCHEDULES_EVERY_MINUTE = {
+TEST_SCHEDULES_EVERY_MINUTE: tuple[str, ...] = (
     "* * * * *",
     "1-5 * * * *",
     "10-20 * * * *",
     "0,45,10-20 * * * *",
     "23,45,50,51 * * * *",
     "10,20,30,40-45 * * * *",
-}
+)
 
-TEST_SCHEDULES_SINGLE_MINUTES = {
+TEST_SCHEDULES_SINGLE_MINUTES: tuple[str, ...] = (
     "1,5,8,10,12 * * * *",
     "10 1 * * *",
     "27,2 1-5 * * *",
-}
+)
 
-TEST_SCHEDULES = TEST_SCHEDULES_EVERY_MINUTE.union(TEST_SCHEDULES_SINGLE_MINUTES)
+TEST_SCHEDULES: tuple[str, ...] = (
+    TEST_SCHEDULES_EVERY_MINUTE + TEST_SCHEDULES_SINGLE_MINUTES
+)
 
 
 def dynamic_alert_minimum_interval(**kwargs) -> int:

tests/unit_tests/conftest.py

@@ -73,6 +73,38 @@ def session(get_session) -> Iterator[Session]:
     return get_session()
 
 
+@pytest.fixture(scope="session", autouse=True)
+def _preload_clock_reading_drivers() -> None:
+    """Import third-party drivers that read the clock at import, before any test.
+
+    ``clickhouse_connect`` normalises the local timezone through ``dateutil``
+    at module import time, which raises ``AttributeError`` under a frozen
+    ``freeze_time`` clock. Superset's ClickHouse engine spec imports it
+    lazily, on the first ``load_engine_specs()`` call, so whichever test
+    first resolves an engine spec pays that import -- and if that test runs
+    under ``freeze_time`` (seven unit-test files combine ``freeze_time`` with
+    engine-spec resolution) the import fails. Serially some earlier test
+    always paid it outside a frozen clock; under pytest-xdist each worker
+    starts cold, so the failure moves with worker assignment.
+
+    Only the third-party module is imported here, deliberately: importing
+    Superset's own engine-spec modules this early would also cache
+    app-context-dependent values (the ClickHouse spec resolves its
+    ``product_name`` from ``current_app`` on import), and running full
+    ``load_engine_specs()`` discovery would execute every registered
+    third-party entry point in every worker. Engine-spec discovery itself
+    stays lazy, so tests that exercise it still observe a cold state.
+    """
+    # Even a guarded module-top import would tie the driver to collection on
+    # every invocation, including --collect-only and partial runs. Session setup
+    # imports it once per worker, after conftest/plugin loading and before any
+    # test can enter freeze_time. Guard ImportError because the driver is optional.
+    try:
+        import clickhouse_connect  # noqa: F401
+    except ImportError:
+        pass
+
+
 @pytest.fixture(scope="module")
 def app(request: SubRequest) -> Iterator[SupersetApp]:
     """

tests/unit_tests/mcp_service/conftest.py

@@ -22,8 +22,35 @@
 RBAC logic is tested directly in test_auth_rbac.py.
 """
 
+from collections.abc import Iterator
+from contextvars import Token
+
 import pytest
 
+from superset.mcp_service.auth import _mcp_user_id_var
+
+
+@pytest.fixture(autouse=True)
+def isolate_mcp_user_id_var() -> Iterator[None]:
+    """Reset the ``_mcp_user_id_var`` ContextVar around every MCP test.
+
+    ``_setup_user_context()`` sets the var and relies on ``on_call_tool``'s
+    ``finally`` to clear it. Tests that call ``_setup_user_context()`` (or set
+    the var) directly, without going through the middleware, leave it set in
+    the thread's base context, and pytest-asyncio copies that context into
+    every later async test's task -- so a resolved user id leaks into
+    unrelated tests. In a serial run a middleware-exercising test file that
+    sorts between the leaking file and its victim happened to clear the var;
+    under pytest-xdist the files land on different workers and the leak
+    surfaces as a flake. Scoping the var to each test removes the ordering
+    dependence.
+    """
+    token: Token[int | None] = _mcp_user_id_var.set(None)
+    try:
+        yield
+    finally:
+        _mcp_user_id_var.reset(token)
+
 
 @pytest.fixture(autouse=True)
 def disable_mcp_rbac(app):

tests/unit_tests/test_clock_reading_driver_preload.py

@@ -0,0 +1,57 @@
+# 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.
+"""Pin the harness guarantee behind ``_preload_clock_reading_drivers``.
+
+``clickhouse_connect`` reads the local timezone at import time and fails
+under a ``freeze_time`` clock, so the unit-test session imports it before any
+test runs. A test can therefore resolve the ClickHouse engine spec under a
+frozen clock on any worker, regardless of what ran before it. This test
+asserts that guarantee directly; if the preload fixture is removed, it fails
+on any worker where nothing else happened to import the driver first.
+"""
+
+import importlib.util
+import sys
+
+import pytest
+from freezegun import freeze_time
+
+
+@pytest.mark.skipif(
+    importlib.util.find_spec("clickhouse_connect") is None,
+    reason="clickhouse_connect is not installed",
+)
+def test_clock_reading_driver_is_preloaded_before_tests() -> None:
+    """Verify the session fixture imports the driver before tests run."""
+    assert "clickhouse_connect" in sys.modules
+
+
+@pytest.mark.skipif(
+    importlib.util.find_spec("clickhouse_connect") is None,
+    reason="clickhouse_connect is not installed",
+)
+@freeze_time("2021-04-01T00:00:00Z")
+def test_clickhouse_engine_spec_resolves_under_frozen_clock() -> None:
+    """Verify the ClickHouse Connect engine spec resolves under a frozen clock."""
+    # Defer the engine-spec package import until after the session preload;
+    # a module-top import would execute during collection, before the fixture.
+    from superset.db_engine_specs import get_engine_spec
+
+    assert (
+        get_engine_spec("clickhousedb", "connect").__name__
+        == "ClickHouseConnectEngineSpec"
+    )