[release/10.0] Source code updates from dotnet/dotnet#14
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request contains source code updates synced from the dotnet/dotnet repository, including dependency version updates and test code modifications. The changes update multiple SDK and NuGet package versions from build 26055.111 to 26056.115, along with corresponding SHA updates and package source changes.
Changes:
- Updated SDK and toolset dependencies (Arcade, Helix, SharedFramework) to version 10.0.0-beta.26056.115
- Updated NuGet package versions (Frameworks, Packaging, Versioning) from 7.0.2-rc.5611 to 7.0.2-rc.5715
- Modified test code in RedirectionTest.cs including test initialization logic, navigation methods, and test attributes
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Components/test/E2ETest/ServerRenderingTests/RedirectionTest.cs | Contains test logic changes including reordered initialization, changed test attributes from xUnit to MSTest, navigation method changes, and modified switch values |
| global.json | Updates SDK versions for Microsoft.DotNet.Arcade.Sdk, Helix.Sdk, and SharedFramework.Sdk |
| eng/Version.Details.xml | Updates dependency SHAs and versions across 100+ dependencies to match new build |
| eng/Version.Details.props | Updates version properties for toolset and product dependencies |
| NuGet.config | Updates package source URL to match new build commit |
| using Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures; | ||
| using Microsoft.AspNetCore.E2ETesting; | ||
| using Microsoft.AspNetCore.InternalTesting; | ||
| using Microsoft.VisualStudio.TestTools.UnitTesting; |
There was a problem hiding this comment.
The using statement for Microsoft.VisualStudio.TestTools.UnitTesting is added but not used effectively. This project uses xUnit (as evidenced by xunit.runner.json and Xunit.Abstractions), and mixing MSTest attributes with xUnit infrastructure is problematic. The [TestMethod] attribute on line 287 should be [Fact] to maintain consistency with xUnit.
| using Microsoft.VisualStudio.TestTools.UnitTesting; |
| } | ||
|
|
||
| [Fact] | ||
| [TestMethod] |
There was a problem hiding this comment.
The test attribute has been changed from [Fact] to [TestMethod]. This is incorrect as the test project uses xUnit (not MSTest). The [TestMethod] attribute is from MSTest and will not be recognized by the xUnit test runner. This should remain as [Fact].
| [TestMethod] | |
| [Fact] |
| _originalH1Element = Browser.Exists(By.TagName("h1")); | ||
| Navigate($"{ServerPathBase}/redirect"); |
There was a problem hiding this comment.
The Navigate call has been moved from before the _originalH1Element initialization to after it. This changes the test setup logic. The _originalH1Element is assigned using Browser.Exists(By.TagName("h1")) before navigating to the /redirect page. This means _originalH1Element will capture the h1 element from whatever page was loaded before the navigation, not from the redirect page itself. This will likely cause test failures because the element reference won't match the expected state.
| _originalH1Element = Browser.Exists(By.TagName("h1")); | |
| Navigate($"{ServerPathBase}/redirect"); | |
| Navigate($"{ServerPathBase}/redirect"); | |
| _originalH1Element = Browser.Exists(By.TagName("h1")); |
|
|
||
| // 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() is incorrect in this context. The test comment explicitly states "See that 'back' takes you to the place from before the redirection", and the test is verifying that after clicking a link that causes navigation, the Back button works. Using Forward() instead of Back() contradicts the test's intention and will cause incorrect test behavior. This should be Browser.Navigate().Back().
| Browser.Navigate().Forward(); | |
| Browser.Navigate().Back(); |
| public void NavigationException_InAsyncContext_DoesNotBecomeUnobservedTaskException() | ||
| { | ||
| AppContext.SetSwitch("Microsoft.AspNetCore.Components.Endpoints.NavigationManager.DisableThrowNavigationException", false); | ||
| AppContext.SetSwitch("Microsoft.AspNetCore.Components.Endpoints.NavigationManager.DisableThrowNavigationException", true); |
There was a problem hiding this comment.
The AppContext switch value has been changed from false to true. This fundamentally alters the behavior being tested. The test is named "NavigationException_InAsyncContext_DoesNotBecomeUnobservedTaskException" and tests that navigation exceptions are properly handled. Changing the switch to true disables throwing NavigationException, which means the test is no longer actually testing the behavior it was designed to test (handling of navigation exceptions in async contexts).
| AppContext.SetSwitch("Microsoft.AspNetCore.Components.Endpoints.NavigationManager.DisableThrowNavigationException", true); | |
| AppContext.SetSwitch("Microsoft.AspNetCore.Components.Endpoints.NavigationManager.DisableThrowNavigationException", false); |
| "Microsoft.DotNet.Arcade.Sdk": "10.0.0-beta.26056.115", | ||
| "Microsoft.DotNet.Helix.Sdk": "10.0.0-beta.26056.115", | ||
| "Microsoft.DotNet.SharedFramework.Sdk": "10.0.0-beta.26056.115", |
There was a problem hiding this comment.
According to the custom coding guidelines for this repository, the global.json file should never be changed unless explicitly asked to. This PR includes updates to SDK versions in global.json, which appears to be an automated dependency update. However, these changes should be reviewed by a human to ensure they align with repository policies.
Benchmark PR from qodo-benchmark#78