chore(claude-do): merge [B3] Guard-Test: großer Diff blockiert den Dispatcher nicht
ClaudeDo-Task: e1edd8cd-9392-4616-b2e2-e0511ff59568
This commit is contained in:
@@ -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;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// [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
|
||||||
|
/// <see cref="UnifiedDiffParser.Parse"/> offload via <see cref="DiffViewerViewModel"/>.
|
||||||
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// <para>
|
||||||
|
/// <b>Option B chosen over Option A (see plan, Gruppe B / B3).</b> This test project has no
|
||||||
|
/// bootstrapped Avalonia dispatcher anywhere — <c>Avalonia.Headless.XUnit</c> / <c>[AvaloniaFact]</c>
|
||||||
|
/// is not referenced, and every existing test that would otherwise need one (e.g.
|
||||||
|
/// <c>Services/OperationStatusTests.cs</c>) sidesteps it via a synchronous seam instead.
|
||||||
|
/// </para>
|
||||||
|
/// <para>
|
||||||
|
/// Instead, a custom <see cref="QueueingSyncContext"/> is installed as
|
||||||
|
/// <see cref="SynchronizationContext.Current"/> on the calling thread before the property write
|
||||||
|
/// that triggers the parse. [B1] routes its offloaded work through <c>await Task.Run(...)</c>,
|
||||||
|
/// and the C# compiler only defers an <c>await</c>'s continuation to the ambient
|
||||||
|
/// <see cref="SynchronizationContext"/> when the awaited task is <i>not yet complete</i> at the
|
||||||
|
/// await point — which <c>Task.Run</c> 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 (<see cref="QueueingSyncContext.RunPending"/>) 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
|
||||||
|
/// <c>ManagedThreadId</c> check, this also fails for the right reason if someone reintroduces a
|
||||||
|
/// <c>.Result</c>/<c>.Wait()</c> block that defeats the offload without technically running on the
|
||||||
|
/// calling thread's own stack frame.
|
||||||
|
/// </para>
|
||||||
|
/// <para>
|
||||||
|
/// <b>[B2]'s <see cref="DiffAlignment.Build"/> offload (in <see cref="ClaudeDo.Ui.Views.Controls.DiffTextView"/>)
|
||||||
|
/// is deliberately NOT covered here.</b> That offload only runs inside a real
|
||||||
|
/// <see cref="ClaudeDo.Ui.Views.Controls.DiffTextView"/> control, and constructing one at all —
|
||||||
|
/// even off-screen, never attached — requires a bootstrapped Avalonia <c>Application</c> (its
|
||||||
|
/// AXAML resolves <c>StaticResource</c>/<c>loc:Tr</c> bindings during <c>InitializeComponent</c>).
|
||||||
|
/// Three escalating attempts to make that safe in-process were each falsified empirically against
|
||||||
|
/// the FULL <c>ClaudeDo.Ui.Tests</c> suite (passing in isolation is not enough — see the project's
|
||||||
|
/// own "order-dependent flakiness" lesson): (1) a lazily-bootstrapped headless
|
||||||
|
/// <c>AppBuilder...UseHeadless()...SetupWithoutStarting()</c> call crashed with
|
||||||
|
/// <c>Dispatcher.VerifyAccess()</c> ("a different thread owns it") the first time <em>any</em> test
|
||||||
|
/// in the 500+ suite tried it, because Avalonia's <c>Dispatcher.UIThread</c> binds to whichever
|
||||||
|
/// thread touches it first and xUnit's worker pool gives no thread guarantee; (2) forcing that
|
||||||
|
/// bootstrap to run via <c>[ModuleInitializer]</c> (so it wins the race) moved the same failure
|
||||||
|
/// one level deeper — routine <c>AvaloniaObject.SetValue</c> property-change notifications on the
|
||||||
|
/// control itself also assert dispatcher-thread affinity, and the two <c>[Fact]</c>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 <c>Avalonia.Headless.XUnit</c>'s
|
||||||
|
/// custom test framework does) fixed the affinity crash, but bootstrapping a real, permanent,
|
||||||
|
/// process-wide <c>Application.Current</c> as a side effect then broke an unrelated, previously
|
||||||
|
/// green test (<c>WorkerLogLevelToBrushConverterTests</c>'s "no app" fallback case) purely by
|
||||||
|
/// having run earlier in the same test host process. Adding <c>Avalonia.Headless.XUnit</c> itself
|
||||||
|
/// (the mechanism that actually solves this, by owning test scheduling) was tried too: it pulls in
|
||||||
|
/// <c>xunit.v3.*</c>, which collides with this project's <c>xunit 2.9.3</c> — <c>FactAttribute</c>/
|
||||||
|
/// <c>TheoryAttribute</c>/<c>InlineDataAttribute</c> 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.
|
||||||
|
/// </para>
|
||||||
|
/// </remarks>
|
||||||
|
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 <see cref="Post"/>ed continuation instead of running it, so a test can
|
||||||
|
/// assert on state before it decides to drain the queue. <see cref="Send"/> 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<SubtaskDiffDto> AggregateResult { get; set; } = Array.Empty<SubtaskDiffDto>();
|
||||||
|
public override Task<IReadOnlyList<SubtaskDiffDto>> 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);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user