From c7958271628a472f65219e8bea3466057f2ee26d Mon Sep 17 00:00:00 2001 From: Mika Kuns Date: Wed, 12 Aug 2026 14:55:37 +0200 Subject: [PATCH] test(ui): [B3] guard against a synchronous diff parse blocking the dispatcher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds DiffDispatcherGuardTests, the one blockade-regression test in the operation-feedback effort. Uses a queueing SynchronizationContext (Option B — this project has no bootstrapped Avalonia dispatcher anywhere) to prove DiffViewerViewModel's UnifiedDiffParser.Parse offload ([B1]) truly defers past the calling thread rather than racing on timing. [B2]'s DiffAlignment.Build offload in DiffTextView is deliberately not covered — three escalating attempts to construct that control safely in-process each broke under the full test suite (dispatcher thread-affinity crashes, then process-wide Application state polluting an unrelated test), and Avalonia.Headless.XUnit collides with this project's xUnit v2 stack. Verified the test fails against the pre-[B1] code and passes with it; full suite green aside from a pre-existing, unrelated flaky test. --- .../ViewModels/DiffDispatcherGuardTests.cs | 167 ++++++++++++++++++ 1 file changed, 167 insertions(+) create mode 100644 tests/ClaudeDo.Ui.Tests/ViewModels/DiffDispatcherGuardTests.cs diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/DiffDispatcherGuardTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/DiffDispatcherGuardTests.cs new file mode 100644 index 00000000..81673cad --- /dev/null +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/DiffDispatcherGuardTests.cs @@ -0,0 +1,167 @@ +using System.Collections.Concurrent; +using System.IO; +using System.Text; +using System.Threading; +using ClaudeDo.Ui.Services; +using ClaudeDo.Ui.ViewModels.Modals; +using Xunit; + +namespace ClaudeDo.Ui.Tests.ViewModels; + +/// +/// [B3] The one blockade-regression test in the operation-feedback effort. Proves that a large +/// diff no longer runs its parse work inline on the calling ("UI") thread — [B1]'s +/// offload via . +/// +/// +/// +/// Option B chosen over Option A (see plan, Gruppe B / B3). This test project has no +/// bootstrapped Avalonia dispatcher anywhere — Avalonia.Headless.XUnit / [AvaloniaFact] +/// is not referenced, and every existing test that would otherwise need one (e.g. +/// Services/OperationStatusTests.cs) sidesteps it via a synchronous seam instead. +/// +/// +/// Instead, a custom is installed as +/// on the calling thread before the property write +/// that triggers the parse. [B1] routes its offloaded work through await Task.Run(...), +/// and the C# compiler only defers an await's continuation to the ambient +/// when the awaited task is not yet complete at the +/// await point — which Task.Run guarantees (its delegate never runs inline on the calling +/// thread). So immediately after the triggering property set returns, the parsed result must +/// still be the previous (empty) one — not because of a timing race that could flip either way, +/// but because the continuation that would write it is sitting, unpumped, in the queue. Draining +/// that queue () is what a real dispatcher's message +/// loop would do; this proves the calling thread stayed free to keep pumping while the work ran +/// elsewhere, which is the actual regression [B1] guards against. Unlike a bare +/// ManagedThreadId check, this also fails for the right reason if someone reintroduces a +/// .Result/.Wait() block that defeats the offload without technically running on the +/// calling thread's own stack frame. +/// +/// +/// [B2]'s offload (in ) +/// is deliberately NOT covered here. That offload only runs inside a real +/// control, and constructing one at all — +/// even off-screen, never attached — requires a bootstrapped Avalonia Application (its +/// AXAML resolves StaticResource/loc:Tr bindings during InitializeComponent). +/// Three escalating attempts to make that safe in-process were each falsified empirically against +/// the FULL ClaudeDo.Ui.Tests suite (passing in isolation is not enough — see the project's +/// own "order-dependent flakiness" lesson): (1) a lazily-bootstrapped headless +/// AppBuilder...UseHeadless()...SetupWithoutStarting() call crashed with +/// Dispatcher.VerifyAccess() ("a different thread owns it") the first time any test +/// in the 500+ suite tried it, because Avalonia's Dispatcher.UIThread binds to whichever +/// thread touches it first and xUnit's worker pool gives no thread guarantee; (2) forcing that +/// bootstrap to run via [ModuleInitializer] (so it wins the race) moved the same failure +/// one level deeper — routine AvaloniaObject.SetValue property-change notifications on the +/// control itself also assert dispatcher-thread affinity, and the two [Fact]s in this class +/// still landed on different xUnit worker threads; (3) marshalling the whole test body onto one +/// dedicated background thread (a hand-rolled stand-in for what Avalonia.Headless.XUnit's +/// custom test framework does) fixed the affinity crash, but bootstrapping a real, permanent, +/// process-wide Application.Current as a side effect then broke an unrelated, previously +/// green test (WorkerLogLevelToBrushConverterTests's "no app" fallback case) purely by +/// having run earlier in the same test host process. Adding Avalonia.Headless.XUnit itself +/// (the mechanism that actually solves this, by owning test scheduling) was tried too: it pulls in +/// xunit.v3.*, which collides with this project's xunit 2.9.3 — FactAttribute/ +/// TheoryAttribute/InlineDataAttribute became ambiguous across ~30 existing test +/// files (CS0433), i.e. it would require migrating the whole test project to xUnit v3, far outside +/// a guard test's scope. [B2] is left to the plan's existing "visuelle Prüfung offen" item; a +/// follow-up to actually cover it would need either that migration or a purpose-built, isolated +/// (e.g. separate test assembly/process) Avalonia UI test harness. +/// +/// +public class DiffDispatcherGuardTests +{ + // 2 000 changed line-pairs (4 000 raw diff lines) — long enough that a synchronous + // UnifiedDiffParser.Parse would be a visibly dropped frame (measured ~15ms on dev hardware for + // parsing alone), short enough that this test stays well under a second even without offload. + private const int LinePairCount = 2000; + + private static string BuildLargeDiff(int pairCount) + { + var sb = new StringBuilder(); + sb.Append("diff --git a/big.cs b/big.cs\n--- a/big.cs\n+++ b/big.cs\n"); + sb.Append($"@@ -1,{pairCount} +1,{pairCount} @@\n"); + for (var i = 0; i < pairCount; i++) + { + sb.Append($"- var value{i} = ComputeSomething(alpha{i}, beta{i}, gamma{i});\n"); + sb.Append($"+ var value{i} = ComputeSomethingElse(alpha{i}, beta{i}, gamma{i});\n"); + } + return sb.ToString(); + } + + /// Captures every ed continuation instead of running it, so a test can + /// assert on state before it decides to drain the queue. is intentionally + /// left unimplemented (base throws) — nothing under test uses synchronous marshalling. + private sealed class QueueingSyncContext : SynchronizationContext + { + private readonly ConcurrentQueue<(SendOrPostCallback Callback, object? State)> _queue = new(); + + public override void Post(SendOrPostCallback d, object? state) => _queue.Enqueue((d, state)); + + public bool RunPending() + { + var ran = false; + while (_queue.TryDequeue(out var item)) + { + item.Callback(item.State); + ran = true; + } + return ran; + } + } + + private sealed class GuardWorkerClient : StubWorkerClient + { + public IReadOnlyList AggregateResult { get; set; } = Array.Empty(); + public override Task> GetPlanningAggregateAsync(string planningTaskId) => + Task.FromResult(AggregateResult); + } + + [Fact] + public async Task PlanningParse_OfLargeDiff_DoesNotRunOnCallingThread() + { + var ctx = new QueueingSyncContext(); + var previous = SynchronizationContext.Current; + SynchronizationContext.SetSynchronizationContext(ctx); + try + { + var raw = BuildLargeDiff(LinePairCount); + var fake = new GuardWorkerClient + { + AggregateResult = new[] { new SubtaskDiffDto("s1", "First", "b1", "base1", "head1", null, raw) }, + }; + var vm = new DiffViewerViewModel(null!, fake, new AppSettings + { + ConfigPath = Path.Combine(Path.GetTempPath(), $"claudedo-uicfg-{Guid.NewGuid():N}.json"), + }); + vm.ConfigurePlanning("plan-1", "main"); + + // LoadAsync's own await chain only ever awaits already-completed tasks + // (Task.FromResult, then synchronous property setters), so it never suspends and + // never touches the sync context itself — only the fire-and-forget parse it + // triggers as a side effect does. Auto-selecting the first (only) subtask sets + // DisplayedDiff to the large diff, which kicks off that parse. + await vm.LoadAsync(); + + // Deterministic, not a race: the continuation that would populate PlanningFiles is + // parked in `ctx`, unpumped. This is exactly what must be false pre-[B1], where + // OnDisplayedDiffChanged parsed and wrote PlanningFiles synchronously and inline. + Assert.Empty(vm.PlanningFiles); + + var pumped = false; + var deadline = DateTime.UtcNow.AddSeconds(5); + while (DateTime.UtcNow < deadline && !pumped) + { + pumped = ctx.RunPending(); + if (!pumped) Thread.Sleep(5); + } + + Assert.True(pumped, "the offloaded parse never posted its completion back"); + var file = Assert.Single(vm.PlanningFiles); + Assert.Equal("big.cs", file.Path); + } + finally + { + SynchronizationContext.SetSynchronizationContext(previous); + } + } +}