-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[wasm] Fix wbt AppsettingsTests #122356
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[wasm] Fix wbt AppsettingsTests #122356
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes the AppsettingsTests test by refactoring how target frameworks are accessed in WASM build tests. Instead of duplicating logic to determine the correct target framework based on template type, the code now delegates to the WasmSdkBasedProjectProvider which already knows the appropriate target framework.
- Adds
GetDefaultTargetFramework()method toWasmSdkBasedProjectProviderto expose its configured target framework - Removes template-based conditional logic for framework selection in
CreateWasmTemplateProject - Replaces static
DefaultTargetFrameworkreferences with calls to_provider.GetDefaultTargetFramework()
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| WasmSdkBasedProjectProvider.cs | Adds public accessor method GetDefaultTargetFramework() to expose the private _defaultTargetFramework field |
| WasmTemplateTestsBase.cs | Removes template-based framework selection logic and uses provider's framework consistently across multiple methods |
| BlazorWasmTestBase.cs | Updates GetBlazorBinFrameworkDir to use provider's framework instead of static DefaultTargetFrameworkForBlazor |
| AppsettingsTests.cs | Removes ActiveIssue attribute to re-enable the previously failing test |
| BuildWasmAppsJobsList.txt | Re-adds Wasm.Build.Tests.Blazor.AppsettingsTests to the test jobs list |
|
|
The issue is that the runtime pack comparison logic is broken when the tfm is not NetCoreAppCurrent. The cause is that the workload check is for net11.0 and that is set to true in this case, but the test is targeting net10.0 because of the template limitation so the pack resolution logic expects to resolve to the 11 workload not the 10 nuget and fails. I'll either fix this runtime pack lookup or skip the test in this case. |
I guess skipping the test if the versions don't match is legit. |
|
It passed the build but It looks like it may be hitting something related to dotnet/aspnetcore#63507 now when starting the app |
fixes #122338 (maybe)