Bug: Remove SlowLayout test category — tests hang due to ForceRenderTimerTick deadlock and over-broad tagging, not inherent headless limitations

[JoshuaRowePhantom]

Summary

Tests tagged [Trait("Category", "SlowLayout")] are excluded from the fast test suite with the comment "can hang in headless environments." Investigation shows the hangs are caused by specific fixable bugs, not fundamental headless limitations. ~60–70% of SlowLayout tests don't touch the visual tree at all and were swept into the category by bulk class-level tagging. The category and its filter should be eliminated.

Root Cause 1 — ForceRenderTimerTick() Self-Deadlock

The primary hang is in MainWindowIntegrationTests.cs:

window.Show();
for (var i = 0; i < 10; i++)
{
    Dispatcher.UIThread.RunJobs();
    AvaloniaHeadlessPlatform.ForceRenderTimerTick(); // ← deadlock
}

[AvaloniaFact] tests run on the UIThread. ForceRenderTimerTick() posts a synthetic render tick and synchronously waits for the UIThread to process it — but the UIThread is already blocked inside that call. Classic re-entrant deadlock with no nested message pump to escape it.

Fix: Write a WaitForLayoutAsync(Window) helper (alongside the existing WaitForWorkspaceTabAsync / WaitForAgentReadyAsync helpers) and replace every ForceRenderTimerTick loop with a call to it:

internal static Task WaitForLayoutAsync(Window window, TimeSpan? timeout = null)
{
    var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
    EventHandler? handler = null;
    handler = (_, _) => { window.LayoutUpdated -= handler; tcs.TrySetResult(); };
    window.LayoutUpdated += handler;
    var wait = tcs.Task;
    if (timeout is { } t) wait = wait.WaitAsync(t);
    return wait;
}

Root Cause 2 — Over-Broad Class-Level Tagging

A single bulk commit tagged entire test classes with [Trait("Category", "SlowLayout")], sweeping in ViewModel-only tests that have no visual-tree dependency and run fine without Avalonia at all. Examples:

• CloneEntityShortcutHandlerTests — no visual tree
• WebViewModelTests — no visual tree
• ReviewWorktreeShortcutHandlerTests — no visual tree
• WorkspaceGuiContextProviderTests — no visual tree
• AgentChatOutputControlTests — uses HeadlessControllableBrowser stub, no real window

These tests use [AvaloniaFact] only because the class was bulk-tagged, not because they need it. Downgrading them to [Fact] removes them from the PerAssembly Avalonia domain and cuts startup pressure.

Three Categories of Tests Currently in SlowLayout

Class	Examples	Genuinely needs visual tree?
Genuine layout tests	MainWindowIntegrationTests, AgentGuiMainWindowIntegrationTests	✅ window.Show()
ViewModel-only (swept in by bulk tagging)	CloneEntityShortcutHandlerTests, WebViewModelTests	❌
Control tests with stub browser	AgentChatOutputControlTests	❌

Required Fixes

Fix A — Replace ForceRenderTimerTick() with WaitForLayoutAsync

Write the WaitForLayoutAsync(Window) helper and replace all ForceRenderTimerTick loop call sites with it.

Fix B — Downgrade ViewModel-only tests from [AvaloniaFact] to [Fact]

For any SlowLayout-tagged test that does not call window.Show() or manipulate the visual tree, remove [Trait("Category", "SlowLayout")] and change [AvaloniaFact] → [Fact] / [AvaloniaTheory] → [Theory].

End State

After fixes A and B, the genuine layout tests no longer hang and run correctly in the headless environment. The Category!=SlowLayout filter in run-tests.ps1 fast mode should be deleted — no separate mode is needed. The SlowLayout category is removed from all tests.

Affected Files

• Phantom.Workspaces.Tests\MainWindowIntegrationTests.cs — add WaitForLayoutAsync helper, replace ForceRenderTimerTick loop
• Phantom.Workspaces.Tests\AgentGuiMainWindowIntegrationTests.cs — review for same pattern
• All test files with bulk class-level [Trait("Category", "SlowLayout")] on ViewModel-only tests — Fix B
• scripts\run-tests.ps1 — delete Category!=SlowLayout filter from fast mode

[JoshuaRowePhantom]

For #1, look for or write a helper to do this. Don't copy / paste this code everywhere.

Why is Fix C still necessary?

[JoshuaRowePhantom]

Responses

Fix A — helper, not inline code

The LayoutUpdated await pattern should live in a shared HeadlessTestHelpers (or similar) static class alongside the existing WaitForWorkspaceTabAsync / WaitForAgentReadyAsync helpers in MainWindowIntegrationTests.cs. Something like:

/// <summary>
/// Waits for the window to complete at least one layout pass after <see cref="Window.Show"/>.
/// Use instead of <c>ForceRenderTimerTick()</c>, which deadlocks on the UIThread.
/// </summary>
internal static Task WaitForLayoutAsync(Window window, TimeSpan? timeout = null)
{
    var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
    EventHandler? handler = null;
    handler = (_, _) =>
    {
        window.LayoutUpdated -= handler;
        tcs.TrySetResult();
    };
    window.LayoutUpdated += handler;
    var wait = tcs.Task;
    if (timeout is { } t) wait = wait.WaitAsync(t);
    return wait;
}

Every call site that currently does the ForceRenderTimerTick loop is replaced with await WaitForLayoutAsync(window).

Fix C — not necessary

Fix C is not necessary. After Fix A (deadlock removed) and Fix B (ViewModel-only tests converted to [Fact]), the remaining genuine layout tests no longer hang — they just work in the headless environment. The Category!=SlowLayout filter in fast mode should simply be deleted once A and B are done; there is no reason to run them separately. Fix C was a safety valve for tests that were still broken; once they're fixed the category and its filter go away entirely.

[JoshuaRowePhantom]

Won't WaitForLayoutAsync hang when there is no need to update layout?

[JoshuaRowePhantom]

Good catch. If layout is already complete when WaitForLayoutAsync is called — or if LayoutUpdated fires before the subscription is established — the task would never resolve.

The fix is to guard with IsMeasureValid && IsArrangeValid. Additionally, LayoutUpdated can fire multiple times during a single layout cycle (one pass per invalidated subtree), so the handler should re-check before resolving:

internal static Task WaitForLayoutAsync(Window window, TimeSpan? timeout = null)
{
    // Layout already complete — nothing to wait for.
    if (window.IsMeasureValid && window.IsArrangeValid)
        return Task.CompletedTask;

    var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
    EventHandler? handler = null;
    handler = (_, _) =>
    {
        if (!window.IsMeasureValid || !window.IsArrangeValid)
            return; // more passes still pending
        window.LayoutUpdated -= handler;
        tcs.TrySetResult();
    };
    window.LayoutUpdated += handler;
    var wait = tcs.Task;
    if (timeout is { } t) wait = wait.WaitAsync(t);
    return wait;
}

This is safe in both directions: returns immediately if layout is already valid, and waits for a stable layout if it isn't yet.

[JoshuaRowePhantom]

Isn't "timeout" just asking for non-determinstic, timing oriented tests?

[JoshuaRowePhantom]

Agreed — the timeout parameter is inconsistent with the deterministic-test principle (see #242) and should be removed. If layout never fires, the test hangs visibly, which correctly points to a real bug rather than masking it behind a timer.

The correct timeout guard is --blame-hang-timeout at the dotnet test level (already present in run-tests.ps1), not inside the helper itself.

Updated helper — no timeout parameter:

internal static Task WaitForLayoutAsync(Window window)
{
    if (window.IsMeasureValid && window.IsArrangeValid)
        return Task.CompletedTask;

    var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
    EventHandler? handler = null;
    handler = (_, _) =>
    {
        if (!window.IsMeasureValid || !window.IsArrangeValid)
            return;
        window.LayoutUpdated -= handler;
        tcs.TrySetResult();
    };
    window.LayoutUpdated += handler;
    return tcs.Task;
}

[JoshuaRowePhantom]

Fixed in commit dede25e on branch fix/remove-slow-layout-238:

• Replaced ForceRenderTimerTick loop with WaitForLayoutAsync helper
• Removed all SlowLayout trait annotations (24 files)
• Converted pure ViewModel tests to [Fact], kept [AvaloniaFact] where Avalonia UI thread or platform services are needed
• Removed SlowLayout filter from run-tests.ps1