[release/10.0] Source code updates from dotnet/dotnet#11
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| } | ||
|
|
||
| [Fact] | ||
| [TestMethod] |
There was a problem hiding this comment.
🔴 Test uses [TestMethod] (MSTest) instead of [Fact] (xUnit) - test will not run
The test method NavigationException_InAsyncContext_DoesNotBecomeUnobservedTaskException was changed from [Fact] to [TestMethod]. This is incorrect because the test project uses xUnit (as evidenced by other tests using [Fact] and [Theory]), and [TestMethod] is an MSTest attribute.
Click to expand
Impact
- The test will not be discovered or executed by the xUnit test runner
- The
[QuarantinedTest]attribute (which implements xUnit'sITraitAttributeatsrc/Testing/src/xunit/QuarantinedTestAttribute.cs:40) will have no effect since it only works with xUnit tests - The
using Microsoft.VisualStudio.TestTools.UnitTesting;import was added but MSTest is not configured for this project
Expected
[Fact]
[QuarantinedTest("https://github.com/dotnet/aspnetcore/pull/63708/")]
public void NavigationException_InAsyncContext_DoesNotBecomeUnobservedTaskException()Actual
[TestMethod]
[QuarantinedTest("https://github.com/dotnet/aspnetcore/pull/63708/")]
public void NavigationException_InAsyncContext_DoesNotBecomeUnobservedTaskException()Recommendation: Change [TestMethod] back to [Fact] and remove the using Microsoft.VisualStudio.TestTools.UnitTesting; import.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| // See that 'back' takes you to the place from before the redirection | ||
| Browser.Navigate().Back(); | ||
| Browser.Navigate().Forward(); |
There was a problem hiding this comment.
🔴 Browser.Navigate().Forward() should be Back() - test logic is inverted
In RedirectEnhancedGetToInternal, Browser.Navigate().Back() was changed to Browser.Navigate().Forward(), but the comment on line 112 still says "See that 'back' takes you to the place from before the redirection".
Click to expand
Impact
After a redirect from /redirect to /nav/scroll-to-hash, calling Forward() would attempt to go forward in browser history (which has nothing ahead), rather than going Back() to the original /redirect page. This will cause the test assertion on line 114-115 to fail.
All other similar tests use Back()
Lines 51, 80, 141, 169, 199, 228, 257, 282 all use Browser.Navigate().Back() for the same pattern of "return to the page from before the redirection".
Expected behavior
// See that 'back' takes you to the place from before the redirection
Browser.Navigate().Back();Actual (buggy) behavior
// See that 'back' takes you to the place from before the redirection
Browser.Navigate().Forward();Recommendation: Change Browser.Navigate().Forward() back to Browser.Navigate().Back().
Was this helpful? React with 👍 or 👎 to provide feedback.
| _originalH1Element = Browser.Exists(By.TagName("h1")); | ||
| Navigate($"{ServerPathBase}/redirect"); |
There was a problem hiding this comment.
🔴 InitializeAsync captures h1 element before navigating to redirect page
The order of operations in InitializeAsync was swapped. Now _originalH1Element is captured before navigating to the redirect page, so it references an element from the wrong page.
Click to expand
Original (correct) order
await base.InitializeAsync();
Navigate($"{ServerPathBase}/redirect"); // First navigate
_originalH1Element = Browser.Exists(By.TagName("h1")); // Then capture element
Browser.Equal("Redirections", () => _originalH1Element.Text);New (buggy) order
await base.InitializeAsync();
_originalH1Element = Browser.Exists(By.TagName("h1")); // Captures from wrong page!
Navigate($"{ServerPathBase}/redirect"); // Navigates after capture
Browser.Equal("Redirections", () => _originalH1Element.Text); // May fail or be staleImpact
_originalH1Elementwill reference an h1 element from whatever pagebase.InitializeAsync()loads, not the/redirectpage- The assertion
Browser.Equal("Redirections", ...)may fail if the initial page has a different h1 text - In subsequent tests,
_originalH1Elementwill be a stale reference or reference the wrong element, causingStaleElementReferenceExceptionor incorrect test behavior
Recommendation: Restore the original order: first Navigate($"{ServerPathBase}/redirect"), then _originalH1Element = Browser.Exists(By.TagName("h1")).
Was this helpful? React with 👍 or 👎 to provide feedback.
3 participants
Benchmark PR from qodo-benchmark#78