[BUG] Test isolation: TestYamlModeDashboardRegistration register/remove pair fails under default xdist scheduling

[Patch76]

Summary

tests/src/e2e/workflows/filesystem/test_yaml_config.py::TestYamlModeDashboardRegistration::test_remove_dashboard_entry fails non-deterministically in CI (most reliably on the 4-worker ubuntu-24.04-arm matrix, occasionally observable on ubuntu-latest-3-workers). Root cause is a test-isolation bug × CI-command interaction, not a flake.

Mechanics

• test_register_dashboard_entry (line 680) writes lovelace.dashboards.ha-mcp-test-dash into configuration.yaml via ha_config_set_yaml.
• test_remove_dashboard_entry (line 710) tries to remove the same key. Both share URL_PATH = "ha-mcp-test-dash" as a class attribute (line 677), so remove is order-dependent on register.
• The HA testcontainer fixture ha_container_with_fresh_config is scope="session" (tests/src/e2e/conftest.py:259). Under pytest-xdist, session-scoped fixtures are per-worker — each worker boots its own container with its own configuration.yaml.
• .github/workflows/pr.yml:179-180 runs the E2E suite as pytest tests/src/e2e/ -n${{ matrix.pytest_workers }} --tb=short -v — without --dist loadscope. Default --dist load round-robins tests across workers without class affinity.
• When register lands on worker A and remove lands on worker B, worker B's configuration.yaml never saw the registration → Dashboard 'ha-mcp-test-dash' not found under lovelace.dashboards.
• Probability scales with worker count: arm (4 workers) higher than ubuntu-latest (3 workers).

Reproduction

Live failure: https://github.com/homeassistant-ai/ha-mcp/actions/runs/25624197082/job/75216186120 (PR #1191 head c765653, arm-only, ubuntu-latest passed). The PR diff itself is comment/docstring cleanup — no logic change touches tools_yaml_config.py or test_yaml_config.py.

AGENTS.md documents the local invocation as cd tests && uv run pytest src/e2e/ -n2 --dist loadscope -v --tb=short. Local runs don't manifest because --dist loadscope keeps same-class tests on the same worker.

Fix options (maintainer's call)

1. CI command alignment — add --dist loadscope to pr.yml:180. Matches the documented local command. Reuse-friendly: keeps the test as written.
2. Self-contained tests — fold register-then-remove into a single test body, or use a fixture that registers before each remove. More code, more coverage redundancy, but no CI-config dependency.
3. @pytest.mark.xdist_group(name="yaml_dashboards") on TestYamlModeDashboardRegistration — explicit class-level grouping that survives any future --dist change.

Option 1 is the smallest delta; option 3 is the most defensive.

Out of scope for

PR #1191 (perf: dedupe lovelace/dashboards/list). Surfaced there only because a probabilistic re-trigger landed on the bad scheduling combination.

[github-actions]

🔍 Analysis

The issue #1196 correctly identifies a race condition and test isolation failure in the E2E test suite. The root cause is the mismatch between the session scope of the Home Assistant testcontainer fixture and the default round-robin scheduling of pytest-xdist in the CI environment.

In tests/src/e2e/conftest.py, the ha_container_with_fresh_config fixture is session-scoped. When running with pytest-xdist, each worker maintains its own session and thus its own isolated Home Assistant container. The tests in TestYamlModeDashboardRegistration (within tests/src/e2e/workflows/filesystem/test_yaml_config.py) are state-dependent: test_register_dashboard_entry writes to configuration.yaml, and test_remove_dashboard_entry attempts to remove that same entry.

Currently, the CI command in .github/workflows/pr.yml uses uv run pytest tests/src/e2e/ -n${{ matrix.pytest_workers }}, which defaults to --dist load. This schedules tests across workers individually. If the registration test lands on Worker A and the removal test lands on Worker B, Worker B's container will not have the registration entry, leading to the reported failure.

🛠️ Suggested Steps

1. Align CI with Documentation: Update .github/workflows/pr.yml to include --dist loadscope. This ensures that all tests within the same class (or module) are pinned to the same worker, preserving the expected state for dependent tests. This matches the recommended local execution command found in AGENTS.md.
2. Add Explicit Grouping (Optional but Recommended): Apply @pytest.mark.xdist_group(name="yaml_dashboards") to the TestYamlModeDashboardRegistration class to guarantee they run together regardless of the global distribution settings.
3. Verify other E2E tests: While ha_config_set_yaml is the primary culprit here, other tests modifying shared state (like helper creation or registry changes) benefit from the same isolation fix.

📊 Technical Analysis (for maintainers)

Root Cause

The session-scoped fixture ha_container_with_fresh_config in tests/src/e2e/conftest.py is instantiated once per xdist worker. The E2E tests in TestYamlModeDashboardRegistration share a class-level URL_PATH and assume sequential execution on the same HA instance. Without --dist loadscope, pytest-xdist breaks this assumption by potentially splitting these tests across different workers (and thus different HA containers).

Affected Code

• tests/src/e2e/workflows/filesystem/test_yaml_config.py:680 - test_register_dashboard_entry (writes state)
• tests/src/e2e/workflows/filesystem/test_yaml_config.py:710 - test_remove_dashboard_entry (expects state)
• .github/workflows/pr.yml:180 - CI command missing --dist loadscope
• tests/src/e2e/conftest.py:259 - ha_container_with_fresh_config fixture scope

Proposed Fix

Option 1: Update CI Workflow (Global Fix)

--- a/.github/workflows/pr.yml
+++ b/.github/workflows/pr.yml
@@ -179,5 +179,6 @@
         uv run pytest tests/src/e2e/ \
           -n${{ matrix.pytest_workers }} \
+          --dist loadscope \
           --tb=short \
           -v

Option 2: Defensive Class Grouping

--- a/tests/src/e2e/workflows/filesystem/test_yaml_config.py
+++ b/tests/src/e2e/workflows/filesystem/test_yaml_config.py
@@ -671,4 +671,5 @@
 
 @pytest.mark.filesystem
+@pytest.mark.xdist_group(name="yaml_dashboards")
 class TestYamlModeDashboardRegistration:

Related Issues/PRs

• perf: dedupe lovelace/dashboards/list in ha_config_set_dashboard (#1085) #1191 - PR where this was surfaced during performance refactoring.

---

🤖 Automated analysis by Issue Bot

[Patch76]

Update — second failure on the same commit.

Re-running the failed E2E Validation (ubuntu-24.04-arm) job on commit c765653 (PR #1191) reproduced the failure deterministically:

Run	Job	Worker	Progress	Outcome
1	https://github.com/homeassistant-ai/ha-mcp/actions/runs/25624197082/job/75216186120	gw1	[65%]	1 failed, 777 passed, 13 skipped in 268.78s
2	https://github.com/homeassistant-ai/ha-mcp/actions/runs/25624197082/job/75219666155	gw3	[71%]	1 failed, 777 passed, 13 skipped in 269.84s

Same test, same error, same counts — workers vary but the register/remove split happens both times. Together with the broader history (recent PRs show mixed green/red on arm-E2E rather than consistent fail), the picture is "high-probability race on 4-worker arm" rather than "occasional flake". Repeated CI re-runs on the same commit aren't a viable workaround.

[Patch76]

Update — full evidence from both arm runs.

Mining the worker assignments per test from both run logs reveals two distinct failure modes present simultaneously, both fixed by the same --dist loadscope change:

Run	Test	Worker	Timestamp	Outcome
Run 1 arm	test_remove_dashboard_entry	gw1	08:39:33	FAILED
Run 1 arm	test_register_dashboard_entry	gw0	08:39:34	PASSED (1 second after remove)
Run 2 arm	test_register_dashboard_entry	gw1	09:44:29	PASSED
Run 2 arm	test_remove_dashboard_entry	gw3	09:44:31	FAILED
ubuntu-latest	test_register_dashboard_entry	gw0	08:39:44	PASSED
ubuntu-latest	test_remove_dashboard_entry	gw0	08:39:44	PASSED

Two failure modes:

1. Order flip (Run 1): under default --dist load, remove got scheduled to a free worker (gw1) before register ran on a different free worker (gw0). Even if same-worker, remove-before-register fails because nothing's been written yet.
2. Cross-worker split (Run 2): register ran on gw1, remove on gw3. Each xdist worker has its own ha_container_with_fresh_config fixture (scope="session" per worker) → separate tempfile.mkdtemp + DockerContainer(HA_TEST_IMAGE) per worker → gw3's configuration.yaml never had the registration.

ubuntu-latest happens to land both tests on gw0 sequentially, avoiding both modes — explains why it stays green.

--dist loadscope fixes both modes by grouping same-class tests on the same worker, preserving collection order (register before remove). The xdist_group marker (option 3) would do the same explicitly. Self-contained tests (option 2) would side-step both, at the cost of test redundancy.

[Patch76]

Update — partial-fix evidence from #1198 self-validation.

#1198 self-validates by running its own arm-E2E with the patched pr.yml (loadscope verified active in the pytest invocation on both runners). Live results on 91844939:

Runner	Outcome	Counts
ubuntu-latest	green	777 passed, 14 skipped in 224.64s
ubuntu-24.04-arm	red	3 failed, 457 passed, 8 skipped in 232.98s

The three arm failures are TestYamlConfigOperations::test_{add_to_new_package_file, replace_key, remove_key} — all surfacing as COMPONENT_NOT_INSTALLED for ha_mcp_tools:

ToolError: "The ha_mcp_tools custom component is not installed. Use ha_install_mcp_tools() to install it via HACS, then restart Home Assistant."

This is a different failure pattern than the original TestYamlModeDashboardRegistration race.

Two open questions for maintainer triage:

1. COMPONENT_NOT_INSTALLED on arm with loadscope. The fixture ha_container_with_fresh_config (tests/src/e2e/conftest.py:259) installs ha_mcp_tools from repo_root / "custom_components". Failure not observed without loadscope on the same runner (perf: dedupe lovelace/dashboards/list in ha_config_set_dashboard (#1085) #1191 arm runs reached this code path normally), nor with loadscope on ubuntu-latest. Mechanism not identified.
2. Test-count gap on arm: pytest reports 4 workers [791 items] at session start, final summary 3 failed, 457 passed, 8 skipped totals 468. Session terminates cleanly. ubuntu-latest ran all 791. Mechanism not identified.

Failed job: https://github.com/homeassistant-ai/ha-mcp/actions/runs/25626003445/job/75221159949

--dist loadscope resolves the original race (ubuntu-latest deterministic-green) but doesn't bring arm to a green state. Whether that's a separate latent bug exposed by loadscope or an indirect interaction needs maintainer-level analysis. Pausing #1198 (Draft) pending direction.

[Patch76]

PR #1201 opened (Draft) as an alternative fix shape — collapses the register/remove pair into a single test method exercising the full add+remove lifecycle in one body, eliminating the inter-test dependency entirely. Test-only refactor, no CI-infra change. PR #1198's --dist loadscope infra approach stays open in parallel for comparison; whichever shape lands, the other can be dropped.

[Patch76]

PR #1201 marked Ready, CI green at df27a6a — including ARM E2E in 6m4s (two consistent green runs across 7b3c6f6 and df27a6a). Race is reproducibly gone with the self-contained refactor. Awaiting review.