Code Review Bench PR #64973 - [release/10.0] Source code updates from dotnet/dotnet

[ketkarameya]

Benchmark PR for Gitar evaluation

Original PR: agentic-review-benchmarks#1

Source code updates from dotnet/dotnet repository for release/10.0 branch.

---

Summary by Gitar

• Dependency updates:
  • Updated 60+ package versions from build 26055.111 to 26056.115 in eng/Version.Details.props and eng/Version.Details.xml
• Test framework migration:
  • Changed NavigationException_InAsyncContext test from xUnit [Fact] to MSTest [TestMethod] with [QuarantinedTest] attribute
• Fixed test initialization:
  • Reordered element reference capture before navigation in InitializeAsync() to prevent StaleElementReferenceException
• Test logic corrections:
  • Changed navigation direction from Back() to Forward() in RedirectEnhancedGetToInternal test
  • Corrected AppContext.SetSwitch value from false to true for navigation exception suppression

_{This will update automatically on new commits.}

---

[gitar-bot-staging]

Code Review ⚠️ Changes requested 0 resolved / 3 findings

Three important bugs in test fixes: element capture before navigation creates stale references, [TestMethod] on xUnit class makes test undiscoverable, and Back()→Forward() change contradicts comment and all other tests in the class.

⚠️ Bug: InitializeAsync captures element before navigation — will be stale

📄 src/Components/test/E2ETest/ServerRenderingTests/RedirectionTest.cs:32

The reordering of InitializeAsync captures _originalH1Element via Browser.Exists(By.TagName("h1")) before navigating to the /redirect page. This means the element is captured from whatever default page the browser is on at that point — not the "Redirections" page.

After Navigate($"{ServerPathBase}/redirect") runs, the browser navigates to a new page, making the previously-captured element reference stale. The subsequent assertion Browser.Equal("Redirections", () => _originalH1Element.Text) will then attempt to read from a stale element, causing a StaleElementReferenceException — which is ironically the exact issue this change claims to fix.

The original order (navigate first, then capture) was correct: it navigates to /redirect, waits for the <h1> element to exist, captures the reference, and then asserts its text. The _originalH1Element is then used in subsequent tests to detect when the page has changed (via enhanced navigation that reuses the DOM).

Fix: Revert to the original order — navigate first, then capture the element.

Suggested fix

        _originalH1Element = Browser.Exists(By.TagName("h1"));
        Navigate($"{ServerPathBase}/redirect");

⚠️ Bug: [TestMethod] on xUnit class makes test undiscoverable

📄 src/Components/test/E2ETest/ServerRenderingTests/RedirectionTest.cs:287

The NavigationException_InAsyncContext_DoesNotBecomeUnobservedTaskException test method now uses [TestMethod] (MSTest attribute) instead of [Fact] (xUnit attribute). However, RedirectionTest inherits from ServerTestBase which is an xUnit test class (uses IClassFixture<T>, xUnit's ITestOutputHelper, etc.).

xUnit discovers tests via [Fact] or [Theory] attributes — it does not recognize [TestMethod]. MSTest also won't discover this test because the class isn't decorated with [TestClass] and doesn't follow MSTest conventions.

As a result, this test becomes dead code — it will never be executed by any test runner.

Additionally, [QuarantinedTest] is an xUnit trait attribute (implements ITraitAttribute from Xunit.Sdk), so it requires an xUnit test attribute to function. Every other [QuarantinedTest] usage in this test project is paired with [Fact], not [TestMethod].

Fix: Replace [TestMethod] with [Fact] and remove the using Microsoft.VisualStudio.TestTools.UnitTesting; import.

Suggested fix

    [Fact]
    [QuarantinedTest("https://github.com/dotnet/aspnetcore/pull/63708/")]

⚠️ Bug: Back() → Forward() contradicts test intent and comment

📄 src/Components/test/E2ETest/ServerRenderingTests/RedirectionTest.cs:113

In RedirectEnhancedGetToInternal, the navigation call was changed from Browser.Navigate().Back() to Browser.Navigate().Forward(), but the comment directly above it still reads:

// See that 'back' takes you to the place from before the redirection
Browser.Navigate().Forward();

This is contradictory. Furthermore, every other similar redirection test in this class (e.g., RedirectStreamingGetToInternal, RedirectStreamingPostToInternal, RedirectEnhancedPostToInternal, RedirectStreamingEnhancedGetToInternal, RedirectEnhancedNonBlazorGetToInternal, RedirectEnhancedGetToInternalWithErrorBoundary) all use Browser.Navigate().Back() for the same pattern of "go back to the page from before the redirection."

Using Forward() after a click-redirect-arrival sequence makes no semantic sense — there's no forward history entry at that point. The test will likely fail or produce undefined behavior.

Fix: Revert to Browser.Navigate().Back() to match the comment and the pattern used by all other tests in this class.

Suggested fix

        Browser.Navigate().Back();

Rules 🎸 1 action taken

Gitar Rules

🎸 Summary Enhancement: Appended technical summary covering dependency updates, test framework migration, initialization fixes, and logic corrections

_{5 rules not applicable. Show all rules by commenting gitar display:verbose.}

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[gitar-bot-staging]

⚠️ Bug: InitializeAsync captures element before navigation — will be stale

The reordering of InitializeAsync captures _originalH1Element via Browser.Exists(By.TagName("h1")) before navigating to the /redirect page. This means the element is captured from whatever default page the browser is on at that point — not the "Redirections" page.

After Navigate($"{ServerPathBase}/redirect") runs, the browser navigates to a new page, making the previously-captured element reference stale. The subsequent assertion Browser.Equal("Redirections", () => _originalH1Element.Text) will then attempt to read from a stale element, causing a StaleElementReferenceException — which is ironically the exact issue this change claims to fix.

The original order (navigate first, then capture) was correct: it navigates to /redirect, waits for the <h1> element to exist, captures the reference, and then asserts its text. The _originalH1Element is then used in subsequent tests to detect when the page has changed (via enhanced navigation that reuses the DOM).

Fix: Revert to the original order — navigate first, then capture the element.

_{Was this helpful? React with 👍 / 👎}

Suggested change

	_originalH1Element = Browser.Exists(By.TagName("h1"));
	_originalH1Element = Browser.Exists(By.TagName("h1"));
	Navigate($"{ServerPathBase}/redirect");

• Apply suggested fix

[gitar-bot-staging]

⚠️ Bug: [TestMethod] on xUnit class makes test undiscoverable

The NavigationException_InAsyncContext_DoesNotBecomeUnobservedTaskException test method now uses [TestMethod] (MSTest attribute) instead of [Fact] (xUnit attribute). However, RedirectionTest inherits from ServerTestBase which is an xUnit test class (uses IClassFixture<T>, xUnit's ITestOutputHelper, etc.).

xUnit discovers tests via [Fact] or [Theory] attributes — it does not recognize [TestMethod]. MSTest also won't discover this test because the class isn't decorated with [TestClass] and doesn't follow MSTest conventions.

As a result, this test becomes dead code — it will never be executed by any test runner.

Additionally, [QuarantinedTest] is an xUnit trait attribute (implements ITraitAttribute from Xunit.Sdk), so it requires an xUnit test attribute to function. Every other [QuarantinedTest] usage in this test project is paired with [Fact], not [TestMethod].

Fix: Replace [TestMethod] with [Fact] and remove the using Microsoft.VisualStudio.TestTools.UnitTesting; import.

_{Was this helpful? React with 👍 / 👎}

Suggested change

	[TestMethod]
	[Fact]
	[QuarantinedTest("https://github.com/dotnet/aspnetcore/pull/63708/")]

• Apply suggested fix

[gitar-bot-staging]

⚠️ Bug: Back() → Forward() contradicts test intent and comment

In RedirectEnhancedGetToInternal, the navigation call was changed from Browser.Navigate().Back() to Browser.Navigate().Forward(), but the comment directly above it still reads:

// See that 'back' takes you to the place from before the redirection
Browser.Navigate().Forward();

This is contradictory. Furthermore, every other similar redirection test in this class (e.g., RedirectStreamingGetToInternal, RedirectStreamingPostToInternal, RedirectEnhancedPostToInternal, RedirectStreamingEnhancedGetToInternal, RedirectEnhancedNonBlazorGetToInternal, RedirectEnhancedGetToInternalWithErrorBoundary) all use Browser.Navigate().Back() for the same pattern of "go back to the page from before the redirection."

Using Forward() after a click-redirect-arrival sequence makes no semantic sense — there's no forward history entry at that point. The test will likely fail or produce undefined behavior.

Fix: Revert to Browser.Navigate().Back() to match the comment and the pattern used by all other tests in this class.

_{Was this helpful? React with 👍 / 👎}

Suggested change

	Browser.Navigate().Forward();
	Browser.Navigate().Back();

• Apply suggested fix

[gitar-bot-staging]

⚠️ This PR is blocked due to unresolved code review findings.

_{Configure merge blocking · Maintainers can dismiss this review. Gitar never approves changes.}