Stabilize VirtualizationTest.CancelsOutdatedRefreshes_Async flaky test

[irfanajaffer]

Stabilize VirtualizationTest.CancelsOutdatedRefreshes_Async

Description

This PR addresses flakiness in
VirtualizationTest.CancelsOutdatedRefreshes_Async.

The test could intermittently fail with StaleElementReferenceException
or with Expected: 1000, Actual: 0 during the scroll-driven async
refresh of a Virtualize component rendered in server-side execution
mode.

To make the test more resilient, the assertions have been updated to
re-query the DOM inside the assertion delegate so a fresh element
reference is obtained on every retry.

Validation / Investigation

1. The failure was analyzed and determined to be caused by an
   IWebElement capture of #cancellation-count.
2. The pre-fix per-iteration Browser.Equal(y, () => scrollTop) was a
   strict equality check on a possibly-converging value.
3. The final int.Parse(cancellationCount.Text, ...) could throw
   FormatException on a transient empty/non-numeric value.
4. The solution aligns with an existing stabilization pattern.

Changes

1. Removed the captured cancellationCount element and re-query it on every retry.
2. Replaced Browser.Equal(y, () => scrollTop) with
   Browser.True(() => scrollTop == targetY).
3. Changed the final cancellation assertion to use int.TryParse.

Fixes #66120.

[ilonatommy]

This change looks too complex for just test stabilization purpose, you're basically introducing a new component with new logic. We don't want to edit the test so drastically, it's not just "mirroring the structure already in used in other stabilization PRs". From what I see, we had 2 failing lines:

1. Browser.Equal(y, () => (long)js.ExecuteScript(...)) -with Expected: 1000, Actual: 0 failure.
2. cancellationCount.Text with StaleElementReferenceException
   I believe both can be fixed by moving the checks to Browser.True/Equals e.g.

- Browser.Equal("0", () => cancellationCount.Text);
+ Browser.Equal("0", () => Browser.FindElement(By.Id("cancellation-count")).Text);

-     js.ExecuteScript($"document.getElementById('async-container').scrollTo({{ top: {y} }})");
-     Browser.Equal(y, () => (long)js.ExecuteScript("return document.getElementById('async-container').scrollTop"));
+    var target = y;
+    Browser.True(() =>
+    {
+         js.ExecuteScript($"document.getElementById('async-container').scrollTo({{ top: {target} }})");
+         return (long)js.ExecuteScript("return document.getElementById('async-container').scrollTop") >= target;
+    }

or similar. By changing too much we're at risk of loosing the test coverage. Try to limit your fix to as few updates as possible.

[ilonatommy]

#68079 (comment)

[ilonatommy]

If it passes I'm fine with the changes.

[irfanajaffer]

This change looks too complex for just test stabilization purpose, you're basically introducing a new component with new logic. We don't want to edit the test so drastically, it's not just "mirroring the structure already in used in other stabilization PRs". From what I see, we had 2 failing lines:

1. Browser.Equal(y, () => (long)js.ExecuteScript(...)) -with Expected: 1000, Actual: 0 failure.
2. cancellationCount.Text with StaleElementReferenceException
   I believe both can be fixed by moving the checks to Browser.True/Equals e.g.

- Browser.Equal("0", () => cancellationCount.Text);
+ Browser.Equal("0", () => Browser.FindElement(By.Id("cancellation-count")).Text);

-     js.ExecuteScript($"document.getElementById('async-container').scrollTo({{ top: {y} }})");
-     Browser.Equal(y, () => (long)js.ExecuteScript("return document.getElementById('async-container').scrollTop"));
+    var target = y;
+    Browser.True(() =>
+    {
+         js.ExecuteScript($"document.getElementById('async-container').scrollTo({{ top: {target} }})");
+         return (long)js.ExecuteScript("return document.getElementById('async-container').scrollTop") >= target;
+    }

or similar. By changing too much we're at risk of loosing the test coverage. Try to limit your fix to as few updates as possible.

Thanks for the review — you're right that the previous draft was too
broad. I reduced the PR to just the two in-place changes you described,
all in the body of CancelsOutdatedRefreshes_Async:

1. Re-query #cancellation-count per retry instead of capturing an
   IWebElement outside the assertion delegate. This avoids both
   StaleElementReferenceException and a stale "0" read when a
   server-side rerender replaces the <span> node.
2. Wrap the per-iteration scrollTop check in Browser.True(...) with a
   targetY = y capture, so a Browser.Equal against a possibly-
   converging scrollTop is replaced by a single-attempt scrollTo
   followed by a poll. (Embedding scrollTo inside the retry delegate
   would mutate state on every retry and weaken the test's semantics,
   so the scroll command is issued once per iteration outside the
   assertion.)

VirtualizationComponent.razor and WaitAssert.cs are unchanged. The
existing #async-container / #cancellation-count / GetItemsAsync
test scenario is preserved exactly, so the production cancellation path
the test was written to cover is still exercised.