Skip to content

Commit 21fb115

Browse files
authored
test(wpf): stop the TransitioningContentControl tests waiting on WPF's render loop (#4499)
## Summary **The `TransitioningContentControl` tests no longer wait on WPF's render loop, so they cannot time out on a busy runner.** - **`CompletingTransition_Completes_AbortsTransitionAndRaisesCompleted` signals completion directly.** It calls the control's completion handler with the storyboard's clock instead of starting the storyboard and pumping the dispatcher until WPF ticks it. - **The double-completion regression test does the same.** `OnTransitionCompleted_RepeatedForSameClock_RaisesTransitionCompletedOnce` runs a real content change, signals completion twice with the same clock as WPF does, then runs a second transition, and expects started, completed, started, completed. ## Why **The test failed intermittently in CI, blocking unrelated PRs.** - It waited for WPF's render loop to tick the animation clock. On a busy runner the loop can stall, and the test failed after its full 10 second wait with `completed == false` while other runs of the same test passed in under a second. ## Breaking changes **None.** - `TransitioningContentControl.OnTransitionCompleted` becomes `internal` (was `private`) so the tests can signal completion, matching the other internal members the tests already use. ## How this was verified **The test class and the whole WPF test project were run repeatedly on real Windows across net8, net9 and net10 with no failures; the regression test still fails with the once-per-transition guard removed.** ## Notes for the reviewer **The control's logic is unchanged; the change is in how the tests drive completion.** - No test in `ReactiveUI.Wpf.Tests` waits on an animation clock any more. - Trade-off: no test now proves that the control subscribes its handler to the storyboard's `Completed` event, since that needs WPF to tick the clock. ## Checklist - [x] I have read the [Contribute guide](https://www.reactiveui.net/contribute/index.html) - [x] The PR title follows [Conventional Commits](https://www.conventionalcommits.org/) - [x] Tests cover this change, or the summary says why they do not - [x] New or changed public API has XML documentation
1 parent 76926d8 commit 21fb115

2 files changed

Lines changed: 47 additions & 74 deletions

File tree

‎src/ReactiveUI.Wpf.Shared/TransitioningContentControl.cs‎

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -665,6 +665,29 @@ internal void SetTransitionDefaultValues()
665665
}
666666
}
667667

668+
/// Handles the completion of a transition and raises the TransitionCompleted event.
669+
/// The source of the event. This is typically the object that initiated the transition.
670+
/// An EventArgs object that contains the event data.
671+
/// Internal so tests can drive completion without depending on WPF's animation clock ticking.
672+
internal void OnTransitionCompleted(object? sender, EventArgs e)
673+
{
674+
// Returning to Normal from inside this handler makes WPF raise Completed a second time for the same clock.
675+
// Only the first one ends the transition.
676+
if (sender is Clock clock)
677+
{
678+
if (ReferenceEquals(clock, _completedClock))
679+
{
680+
return;
681+
}
682+
683+
_completedClock = clock;
684+
}
685+
686+
AbortTransition();
687+
688+
TransitionCompleted?.Invoke(this, new());
689+
}
690+
668691
/// Called when the value of the property changes.
669692
/// The previous content value.
670693
/// The new content value.
@@ -701,28 +724,6 @@ private void AbortTransition()
701724
PreviousImageSite.UpdateLayout();
702725
}
703726

704-
/// Handles the completion of a transition and raises the TransitionCompleted event.
705-
/// The source of the event. This is typically the object that initiated the transition.
706-
/// An EventArgs object that contains the event data.
707-
private void OnTransitionCompleted(object? sender, EventArgs e)
708-
{
709-
// Returning to Normal from inside this handler makes WPF raise Completed a second time for the same clock.
710-
// Only the first one ends the transition.
711-
if (sender is Clock clock)
712-
{
713-
if (ReferenceEquals(clock, _completedClock))
714-
{
715-
return;
716-
}
717-
718-
_completedClock = clock;
719-
}
720-
721-
AbortTransition();
722-
723-
TransitionCompleted?.Invoke(this, new());
724-
}
725-
726727
/// Raises the TransitionStarted event to signal that a transition has begun.
727728
[MethodImpl(MethodImplOptions.AggressiveInlining)]
728729
private void RaiseTransitionStarted() => TransitionStarted?.Invoke(this, new());

‎src/tests/ReactiveUI.Wpf.Tests/Wpf/TransitioningContentControlTest.cs‎

Lines changed: 24 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -66,18 +66,9 @@ public class TransitioningContentControlTest
6666
/// An out-of-range value used to exercise the default arms of the transition switch expressions.
6767
private const int InvalidEnumValue = 999;
6868

69-
/// The longest time, in seconds, to pump the dispatcher while waiting for a storyboard to complete.
70-
private const int StoryboardTimeoutSeconds = 10;
71-
72-
/// The delay, in milliseconds, between dispatcher pump iterations.
73-
private const int PumpDelayMs = 10;
74-
7569
/// The duration, in milliseconds, of the transition driven through the visual state manager.
7670
private const int TransitionDurationMs = 50;
7771

78-
/// How long, in milliseconds, to keep pumping after a transition completes to catch a repeated event.
79-
private const int SettleTimeMs = 300;
80-
8172
/// The label recorded when TransitionStarted fires.
8273
private const string StartedEvent = "started";
8374

@@ -960,10 +951,15 @@ public async Task OnContentChanged_WithoutTemplate_DoesNotThrow()
960951
}
961952

962953
///
963-
/// When the completing storyboard runs to completion, the control aborts the transition (returns to Normal, clears
964-
/// the previous-image snapshot) and raises TransitionCompleted.
954+
/// When the completing storyboard's clock reports completion, the control aborts the transition (returns to Normal,
955+
/// clears the previous-image snapshot) and raises TransitionCompleted.
965956
///
966957
/// A representing the asynchronous operation.
958+
///
959+
/// The test raises the completion itself rather than running the storyboard. A running storyboard only completes
960+
/// when WPF's per-thread render loop ticks its clock, and that loop depends on the composition engine answering;
961+
/// on CI runners it sometimes never ticks, which made a pumped wait fail after its full deadline.
962+
///
967963
[Test]
968964
public async Task CompletingTransition_Completes_AbortsTransitionAndRaisesCompleted()
969965
{
@@ -973,48 +969,33 @@ public async Task CompletingTransition_Completes_AbortsTransitionAndRaisesComple
973969
// Seed a previous-image snapshot so AbortTransition exercises its render-target clearing branch.
974970
control.PrepareTransitionImages(new TextBlock { Text = "snapshot" });
975971

976-
// A real, zero-duration storyboard targeting an existing element so it actually completes and fires Completed.
977972
// Two children are required because the Fade defaults read Children[0] and Children[1].
978973
var storyboard = new Storyboard();
979-
var opacityAnimation = new DoubleAnimation(1.0, 1.0, new Duration(TimeSpan.Zero));
980-
Storyboard.SetTarget(opacityAnimation, control);
981-
Storyboard.SetTargetProperty(opacityAnimation, new(UIElement.OpacityProperty));
982-
storyboard.Children.Add(opacityAnimation);
983-
984-
var secondAnimation = new DoubleAnimation(1.0, 1.0, new Duration(TimeSpan.Zero));
985-
Storyboard.SetTarget(secondAnimation, control);
986-
Storyboard.SetTargetProperty(secondAnimation, new(UIElement.OpacityProperty));
987-
storyboard.Children.Add(secondAnimation);
974+
storyboard.Children.Add(CreateOpacityAnimation(control));
975+
storyboard.Children.Add(CreateOpacityAnimation(control));
988976

989977
var completed = false;
990978
control.TransitionCompleted += (_, _) => completed = true;
991979

992-
// Assigning CompletingTransition hooks OnTransitionCompleted onto the storyboard's Completed event.
993980
control.CompletingTransition = storyboard;
994981

995-
storyboard.Begin(control, true);
996-
997-
// Pump the dispatcher so the zero-length animation completes and fires its Completed callback. A slow runner can
998-
// need more than a fixed number of pumps, so keep pumping until it completes or the deadline passes.
999-
var started = System.Diagnostics.Stopwatch.GetTimestamp();
1000-
while (!completed && System.Diagnostics.Stopwatch.GetElapsedTime(started) < TimeSpan.FromSeconds(StoryboardTimeoutSeconds))
1001-
{
1002-
Tests.Xaml.Utilities.DispatcherUtilities.DoEvents();
1003-
await Task.Delay(PumpDelayMs);
1004-
}
982+
control.OnTransitionCompleted(storyboard.CreateClock(), EventArgs.Empty);
1005983

1006984
await Assert.That(completed).IsTrue();
1007985
}
1008986

1009987
///
1010-
/// A content change that runs a real visual-state transition raises TransitionStarted once and
1011-
/// TransitionCompleted once. WPF raises the storyboard's Completed event a second time for the same
1012-
/// clock when the handler moves the control back to the Normal state, and that repeat must not surface as a
1013-
/// second TransitionCompleted.
988+
/// A transition raises TransitionStarted once and TransitionCompleted once, even though WPF raises the
989+
/// storyboard's Completed event a second time for the same clock when the handler moves the control back to
990+
/// the Normal state. The next transition runs on a new clock and completes again.
1014991
///
1015992
/// A representing the asynchronous operation.
993+
///
994+
/// The test raises the clock's completion itself, twice for the same clock as WPF does, rather than waiting for
995+
/// WPF's render loop to tick the storyboard; see .
996+
///
1016997
[Test]
1017-
public async Task OnContentChanged_RealTransition_RaisesTransitionCompletedOnce()
998+
public async Task OnTransitionCompleted_RepeatedForSameClock_RaisesTransitionCompletedOnce()
1018999
{
10191000
var control = CreateRealizedControl();
10201001
control.Transition = TransitioningContentControl.TransitionType.Fade;
@@ -1026,23 +1007,14 @@ public async Task OnContentChanged_RealTransition_RaisesTransitionCompletedOnce(
10261007
control.TransitionCompleted += (_, _) => events.Add(CompletedEvent);
10271008

10281009
control.Content = new TextBlock { Text = NewContentText };
1010+
var firstClock = control.CompletingTransition!.CreateClock();
1011+
control.OnTransitionCompleted(firstClock, EventArgs.Empty);
1012+
control.OnTransitionCompleted(firstClock, EventArgs.Empty);
10291013

1030-
// Pump until the transition completes, then keep pumping so a repeated Completed has time to arrive.
1031-
var started = System.Diagnostics.Stopwatch.GetTimestamp();
1032-
while (!events.Contains(CompletedEvent) && System.Diagnostics.Stopwatch.GetElapsedTime(started) < TimeSpan.FromSeconds(StoryboardTimeoutSeconds))
1033-
{
1034-
Tests.Xaml.Utilities.DispatcherUtilities.DoEvents();
1035-
await Task.Delay(PumpDelayMs);
1036-
}
1037-
1038-
var settled = System.Diagnostics.Stopwatch.GetTimestamp();
1039-
while (System.Diagnostics.Stopwatch.GetElapsedTime(settled) < TimeSpan.FromMilliseconds(SettleTimeMs))
1040-
{
1041-
Tests.Xaml.Utilities.DispatcherUtilities.DoEvents();
1042-
await Task.Delay(PumpDelayMs);
1043-
}
1014+
control.Content = new TextBlock { Text = "second" };
1015+
control.OnTransitionCompleted(control.CompletingTransition!.CreateClock(), EventArgs.Empty);
10441016

1045-
await Assert.That(events).IsEquivalentTo([StartedEvent, CompletedEvent]);
1017+
await Assert.That(events).IsEquivalentTo([StartedEvent, CompletedEvent, StartedEvent, CompletedEvent]);
10461018
}
10471019

10481020
///

0 commit comments

Comments
 (0)