Skip to content

Port https://github.com/dotnet/aspnetcore/pull/38814 to 6.0 #40101

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Feb 11, 2022
Merged

Port https://github.com/dotnet/aspnetcore/pull/38814 to 6.0 #40101

merged 3 commits into from
Feb 11, 2022

Conversation

pranavkm
Copy link
Contributor

@pranavkm pranavkm commented Feb 9, 2022

No description provided.

@ghost ghost added this to the 6.0.x milestone Feb 9, 2022
@ghost
Copy link

ghost commented Feb 9, 2022

Hi @pranavkm. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document.
Otherwise, please add tell-mode label.

@pranavkm pranavkm mentioned this pull request Feb 9, 2022
Merged
@Pilchie Pilchie added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Feb 9, 2022
@Pilchie
Copy link
Member

Pilchie commented Feb 9, 2022

The goal is to increase coverage in servicing? Sounds good to me.

@Pilchie Pilchie added the tell-mode Indicates a PR which is being merged during tell-mode label Feb 9, 2022
@pranavkm pranavkm requested a review from dougbu February 9, 2022 18:47
@pranavkm
Copy link
Contributor Author

pranavkm commented Feb 9, 2022

@dougbu had attempted to port this back to 6.0 yesterday: #38814 (comment). I'm not super sure of the background on this though.

dougbu
dougbu reviewed Feb 9, 2022
View reviewed changes
Copy link
Contributor

@dougbu dougbu left a comment

Choose a reason for hiding this comment

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

We need the following in this PR or the branch is going to get more flaky
https://github.com/dotnet/aspnetcore/blob/main/eng/test-configuration.json#L11

That was visible in #38814 but not 9c29067 for some reason

dougbu
dougbu reviewed Feb 9, 2022
View reviewed changes
src/Middleware/Diagnostics/test/FunctionalTests/Diagnostics.FunctionalTests.csproj
Comment on lines +8 to +10

<!-- https://github.com/dotnet/aspnetcore/issues/38819 LocalDb sometimes hangs on win11 helix queue -->
<BuildHelixPayload>false</BuildHelixPayload>
Copy link
Contributor

Choose a reason for hiding this comment

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

@dougbu had attempted to port this back to 6.0 yesterday: #38814 (comment). I'm not super sure of the background on this though.

The background is we need this addition in release/6.0 to get PRs passing because LocalDB isn't reliable where we need it to be reliable.

The un-quarantining is also fine as long as the tests have been just as reliable in this branch. If the test or test infrastructure isn't up to the task, we should backport only this change. I'm not familiar enough w/ the tests to know either way and am asking the gathered masses (as I would have if the backport bot could work)…

Copy link
Contributor

Choose a reason for hiding this comment

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

/btw this lack is also holding up #40073

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not familiar enough w/ the tests to know either way and am asking the gathered masses (as I would have if the backport bot could work)…

Checked the analytics and the tests have been passing at 100% in this branch too 🎉

@wtgodbe
Copy link
Member

wtgodbe commented Feb 9, 2022

We need the following in this PR or the branch is going to get more flaky
https://github.com/dotnet/aspnetcore/blob/main/eng/test-configuration.json#L11

There is no test-configuration.json in this branch, is there more needed than just dropping it in?

@pranavkm
Copy link
Contributor Author

@dougbu / @wtgodbe good to merge?

@dougbu
Copy link
Contributor

dougbu commented Feb 10, 2022

@dougbu / @wtgodbe good to merge?

Please add the missing eng/test-configuration.json file to allow un-quarantining in src/Servers/Kestrel/test/FunctionalTests/MaxRequestBufferSizeTests.cs. Fine to just copy the file from 'main' and un-quarantine the other test methods mentioned there later. Also fine to narrow things down to the one Helix-run retry.

@pranavkm pranavkm requested a review from a team as a code owner February 10, 2022 20:36
wtgodbe
wtgodbe approved these changes Feb 10, 2022
View reviewed changes
Copy link
Member

@wtgodbe wtgodbe left a comment

Choose a reason for hiding this comment

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

Looks good, but we should probably wait for next month at this point

@dougbu
Copy link
Contributor

dougbu commented Feb 10, 2022

Tests here are now failing because Blazor template tests are still running on Helix. Some differences between this PR and #38814 remain because src/ProjectTemplates/BlazorTemplates.Tests/BlazorTemplates.Tests.csproj doesn't appear to change here.

@pranavkm pranavkm merged commit 00d34c3 into dotnet:release/6.0 Feb 11, 2022
@pranavkm pranavkm deleted the port-tests branch February 11, 2022 01:09
@wtgodbe
Copy link
Member

wtgodbe commented Feb 11, 2022

@pranavkm please check with the CQBs before merging into release branches this far into the build window, changes were supposed to be in by EOD Monday

@pranavkm
Copy link
Contributor Author

Sorry about that. I missed reading your previous comment and thought the approval meant we were good to get this in.

@dougbu
Copy link
Contributor

dougbu commented Feb 11, 2022

Was the problem addressed in this PR blocking 6.0 dependency update PRs❔ Or, were the CQBs ignoring the problem (or problems) and merging on red❔

@wtgodbe
Copy link
Member

wtgodbe commented Feb 11, 2022

Looks like the current 6.0 aspnetcore dependency PR has been red for a few days - I think CQBs aren't focused on that yet because of the ongoing ref pack issues in WindowsDesktop.

@dougbu
Copy link
Contributor

dougbu commented Feb 11, 2022

Unfortunately things didn't turn green w/ this fix. The CG failures (which just turn the build orange fortunately) on Linux MUSL indicate we need either #39898 backported or another Arcade update (if @mmitche's continueOnError fix is now available for 6.0 branches). Should we take the time to do one or the other❔

@wtgodbe
Copy link
Member

wtgodbe commented Feb 11, 2022

Let's backport #39898 for now

@dougbu
Copy link
Contributor

dougbu commented Feb 11, 2022

Realized we have an Arcade build containing continueOnError. So, the orangeness is mostly aesthetic but I prefer we get to green if we have time. Thanks for starting the backport.

@wtgodbe
Copy link
Member

wtgodbe commented Feb 11, 2022

#40158

@dougbu dougbu modified the milestones: 6.0.x, 6.0.3 Mar 7, 2022
Elanis pushed a commit to Elanis/portfolio that referenced this pull request Dec 13, 2022
Bump Microsoft.AspNetCore.TestHost from 6.0.2 to 6.0.3 (#29)
b238200 
Bumps [Microsoft.AspNetCore.TestHost](https://github.com/dotnet/aspnetcore) from 6.0.2 to 6.0.3.
<details>
<summary>Release notes</summary>

*Sourced from [Microsoft.AspNetCore.TestHost's releases](https://github.com/dotnet/aspnetcore/releases).*

> ## .NET 6.0.3
> [Release](https://github.com/dotnet/core/tree/v6.0.3)
</details>
<details>
<summary>Commits</summary>

- [`c911002`](dotnet/aspnetcore@c911002) Merged PR 21440: [internal/release/6.0] Update dependencies from dnceng/inter...
- [`5049143`](dotnet/aspnetcore@5049143) [internal/release/6.0] Update dependencies from dnceng/internal/dotnet-runtime
- [`04bc56b`](dotnet/aspnetcore@04bc56b) [internal/release/6.0] Update dependencies from dnceng/internal/dotnet-efcore...
- [`f01a14d`](dotnet/aspnetcore@f01a14d) Merge in 'release/6.0' changes
- [`d5d2e4d`](dotnet/aspnetcore@d5d2e4d) [release/6.0] Extend disable component governance for linux ([#40158](dotnet/aspnetcore#40158))
- [`e7bfbce`](dotnet/aspnetcore@e7bfbce) Merge in 'release/6.0' changes
- [`00d34c3`](dotnet/aspnetcore@00d34c3) Port dotnet/aspnetcore#38814 to 6.0 ([#40101](dotnet/aspnetcore#40101))
- [`8dfa6ac`](dotnet/aspnetcore@8dfa6ac) Merge in 'release/6.0' changes
- [`006f1b8`](dotnet/aspnetcore@006f1b8) Fix 5.0 SiteExtension Version ([#40121](dotnet/aspnetcore#40121))
- [`372292b`](dotnet/aspnetcore@372292b) Merge in 'release/6.0' changes
- Additional commits viewable in [compare view](dotnet/aspnetcore@v6.0.2...v6.0.3)
</details>

<br />

Reviewed-on: https://gitea.dysnomia.studio/elanis/portfolio/pulls/29
Co-authored-by: elanis <[email protected]>
Co-committed-by: elanis <[email protected]>
Elanis added a commit to Dysnomia-Studio/dysnomia-website that referenced this pull request Jul 14, 2023
@Elanis
Bump Microsoft.AspNetCore.TestHost from 6.0.2 to 6.0.3 (#34)
876ecd0 
Bumps [Microsoft.AspNetCore.TestHost](https://github.com/dotnet/aspnetcore) from 6.0.2 to 6.0.3.
<details>
<summary>Release notes</summary>

*Sourced from [Microsoft.AspNetCore.TestHost's releases](https://github.com/dotnet/aspnetcore/releases).*

> ## .NET 6.0.3
> [Release](https://github.com/dotnet/core/tree/v6.0.3)
</details>
<details>
<summary>Commits</summary>

- [`c911002`](dotnet/aspnetcore@c911002) Merged PR 21440: [internal/release/6.0] Update dependencies from dnceng/inter...
- [`5049143`](dotnet/aspnetcore@5049143) [internal/release/6.0] Update dependencies from dnceng/internal/dotnet-runtime
- [`04bc56b`](dotnet/aspnetcore@04bc56b) [internal/release/6.0] Update dependencies from dnceng/internal/dotnet-efcore...
- [`f01a14d`](dotnet/aspnetcore@f01a14d) Merge in 'release/6.0' changes
- [`d5d2e4d`](dotnet/aspnetcore@d5d2e4d) [release/6.0] Extend disable component governance for linux ([#40158](dotnet/aspnetcore#40158))
- [`e7bfbce`](dotnet/aspnetcore@e7bfbce) Merge in 'release/6.0' changes
- [`00d34c3`](dotnet/aspnetcore@00d34c3) Port dotnet/aspnetcore#38814 to 6.0 ([#40101](dotnet/aspnetcore#40101))
- [`8dfa6ac`](dotnet/aspnetcore@8dfa6ac) Merge in 'release/6.0' changes
- [`006f1b8`](dotnet/aspnetcore@006f1b8) Fix 5.0 SiteExtension Version ([#40121](dotnet/aspnetcore#40121))
- [`372292b`](dotnet/aspnetcore@372292b) Merge in 'release/6.0' changes
- Additional commits viewable in [compare view](dotnet/aspnetcore@v6.0.2...v6.0.3)
</details>

<br />

Co-authored-by: Elanis <[email protected]>
Reviewed-on: https://gitea.dysnomia.studio/elanis/dysnomia-website/pulls/34
Co-authored-by: elanis <[email protected]>
Co-committed-by: elanis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dougbu dougbu dougbu approved these changes

@wtgodbe wtgodbe wtgodbe approved these changes

@BrennanConroy BrennanConroy Awaiting requested review from BrennanConroy

@halter73 halter73 Awaiting requested review from halter73

@Pilchie Pilchie Awaiting requested review from Pilchie

@Tratcher Tratcher Awaiting requested review from Tratcher

Assignees
No one assigned
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework tell-mode Indicates a PR which is being merged during tell-mode
Projects
None yet
Milestone
6.0.3
Development

Successfully merging this pull request may close these issues.
4 participants
@pranavkm @Pilchie @wtgodbe @dougbu