Skip to content

Fix web root for WebApplication #32604

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

Merged
merged 2 commits into from
May 13, 2021
Merged

Fix web root for WebApplication #32604

merged 2 commits into from
May 13, 2021

Conversation

halter73
Copy link
Member

  • Use WebRootKey instead of ContentRootKey to populate WebRootPath

Fixes #32415


using System.Runtime.CompilerServices;

[assembly: InternalsVisibleTo("Microsoft.AspNetCore.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we have a better way to do this from the csproj now?

@davidfowl
Copy link
Member

@SteveSandersonMS @javiercn @captainsafia @pranavkm the blazor performance tests are broken


namespace Microsoft.AspNetCore.Tests
{
public class WebHostEnvironmentTests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a test making sure static files works?


webApp.MapGet("/", (Func<string>)(() => "Hello, World!"));
app.UseStaticFiles();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you manually tested this?

Copy link
Member Author

@halter73 halter73 May 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I can write a manual automated test. There aren't any e2e tests in DefaultBuilder that don't hit the network stack so I was a little lazy and just went with the unit tests and the sample.

@SteveSandersonMS
Copy link
Member

@davidfowl

the blazor performance tests are broken

Can you clarify which ones? The WebAssembly benchmarks still seem to be working fine. And do you mean they are broken already, or just after this PR? The build on CI doesn't seem to indicate that this PR breaks any perf tests.

@pranavkm
Copy link
Contributor

pranavkm commented May 13, 2021

It’s the razor compiler issue with generic constraints.

@davidfowl
Copy link
Member

It’s the razor compiler issue with generic constraints.

Is it being fixed? Can these tests be skipped? Is it flaky? We want to merge this PR 😄

@SteveSandersonMS
Copy link
Member

Yes, it was quarantined yesterday by @captainsafia: #32593

@davidfowl
Copy link
Member

/azp run aspnetcore-ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@davidfowl davidfowl merged commit 44116a9 into main May 13, 2021
@davidfowl davidfowl deleted the halter73/32415 branch May 13, 2021 22:19
@ghost ghost added this to the 6.0-preview5 milestone May 13, 2021
@DamianEdwards
Copy link
Member

🥳

@ghost
Copy link

ghost commented May 13, 2021

Hi @DamianEdwards. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@amcasey amcasey added area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed area-runtime labels Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-minimal-hosting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Web root is incorrect when using new minimal hosting API (WebApplication.CreateBuilder)
6 participants