[automated] Merge branch 'release/3.1' => 'master'

[dotnet-maestro-bot]

I detected changes in the release/3.1 branch which have not been merged yet to master. I'm a robot and am configured to help you automatically keep master up to date, so I've opened this PR.

This PR merges commits made on release/3.1 by the following committers:

• @javiercn
• @jkotalik
• @dougbu
• @SteveSandersonMS
• @dotnet-maestro[bot]
• @mkArtakMSFT
• @JunTaoLuo
• @mmitche
• @danroth27
• @pranavkm
• @shirhatti
• @joeloff
• @JamesNK
• @BrennanConroy
• @HaoK
• @ryanbrandenburg
• @Pilchie
• @Tratcher
• @halter73
• @mikaelm12
• @dotnet-bot

Instructions for merging from UI

This PR will not be auto-merged. When pull request checks pass, complete this PR by creating a merge commit, not a squash or rebase commit.

If this repo does not allow creating merge commits from the GitHub UI, use command line instructions.

Instructions for merging via command line

Run these commands to merge this pull request from the command line.

git fetch
git checkout release/3.1
git pull --ff-only
git checkout master
git pull --ff-only
git merge --no-ff release/3.1

# If there are merge conflicts, resolve them and then run git merge --continue to complete the merge
# Pushing the changes to the PR branch will re-trigger PR validation.
git push https://github.com/dotnet-maestro-bot/AspNetCore HEAD:merge/release/3.1-to-master

or if you are using SSH

git push git@github.com:dotnet-maestro-bot/AspNetCore HEAD:merge/release/3.1-to-master

After PR checks are complete push the branch

git push

Instructions for resolving conflicts

⚠️ If there are merge conflicts, you will need to resolve them manually before merging. You can do this using GitHub or using the command line.

Instructions for updating this pull request

Contributors to this repo have permission update this pull request by pushing to the branch 'merge/release/3.1-to-master'. This can be done to resolve conflicts or make other changes to this pull request before it is merged.

git checkout -b merge/release/3.1-to-master master
git pull https://github.com/dotnet-maestro-bot/AspNetCore merge/release/3.1-to-master
(make changes)
git commit -m "Updated PR with my changes"
git push https://github.com/dotnet-maestro-bot/AspNetCore HEAD:merge/release/3.1-to-master

or if you are using SSH

git checkout -b merge/release/3.1-to-master master
git pull git@github.com:dotnet-maestro-bot/AspNetCore merge/release/3.1-to-master
(make changes)
git commit -m "Updated PR with my changes"
git push git@github.com:dotnet-maestro-bot/AspNetCore HEAD:merge/release/3.1-to-master

Contact .NET Core Engineering if you have questions or issues.
Also, if this PR was generated incorrectly, help us fix it. See https://github.com/dotnet/arcade/blob/master/scripts/GitHubMergeBranches.ps1.

[javiercn]

@dougbu Seems like the template tests are hanging.

I've updated the PR by merging release/3.1 into this branch again to pick the latest commits that were missing on the PR.

Among those commits is one that separates the dump upload into its own artifact and uploads it before any other artifact. This is done to prevent the task from failing to upload the dump when a log file is locked (as it was the case before).

I'm not sure what changed in the infrastructure between .NET 3.1 and .NET 5.0 but the tests hang in the 5.0 branches (and they've suffered no meaningful change). So we should get the dump from the CI and use it to investigate.

If we find that we don't have enough information there are a couple of things we can do.

• List the current processes we think might be alive and hung other than dotnet. (This can easily be done by adding them to the list passed to the StartDumpCollection.ps1 script)
• Capture all the processes that are running on the machine and dump them into a file so that we can validate that we didn't miss capturing the dump for any process that might be causing the hang.

[dotnet-maestro-bot]

❌ Uh oh, this pull request could not be updated automatically. New commits were pushed to release/3.1, but I could not automatically push those to merge/release/3.1-to-master to update this PR.
You may need to fix this problem by merging branches with this PR. Contact .NET Core Engineering if you are not sure what to do about this.

[javiercn]

There was an issue with npm test on the angular template causing the template tests runs to hang. I've filed #14216 to track it.

[javiercn]

The fix for #14216 was actually trivial, so I've done it inline.

[dougbu]

Thanks @javiercn❕ Glad the change was trivial. (As mentioned in #13497, I was coming at the problem from another angle. We ended up looking at the same code 😺)

[dougbu]

Separately, thoughts on adding node.exe to the list of processes killed in eng/scripts/KillProcesses.ps1 and node to …/KillProcesses.sh?

[dougbu]

Pity the template tests remain flaky. F# variant of MvcTemplate_NoAuthImplAsync(...) failed again. I've skipped it and will do the same in 'release/3.1'

• @mkArtakMSFT why is that test enabled with only a 65% pass rate in 'release/3.1'?
• @JunTaoLuo suggest we should have skipped this variant when you filed Fix MvcTemplate_NoAuthImplAsync F# template tests in release/3.1  #14022
• @javiercn this test is unfortunately not reliable enough to retry ☹️

[javiercn]

@dougbu There are binlogs in Artifacts->Windows_Templates_Logs check those before skipping the tests. Make sure it’s flakiness and not that the merge is missing something.

[dotnet-maestro-bot]

❌ Uh oh, this pull request could not be updated automatically. New commits were pushed to release/3.1, but I could not automatically push those to merge/release/3.1-to-master to update this PR.
You may need to fix this problem by merging branches with this PR. Contact .NET Core Engineering if you are not sure what to do about this.

[dougbu]

@javiercn we know MvcTemplate_NoAuthImplAsync is flaky. It failed in both attempts before I skipped it. The test is also flaky in 'release/3.1'.

FYI the only difference between https://dev.azure.com/dnceng/public/_build/results?buildId=362543 and the earlier https://dev.azure.com/dnceng/public/_build/results?buildId=362349 is the new Skip property setting.

The newer run also failed due to template test flakiness but that's not as common as the MvcTemplate_NoAuthImplAsync problem. Further, the earlier run didn't collect any artifacts for the template tests, making it impossible to see what went on exactly.

[dotnet-maestro-bot]

❌ Uh oh, this pull request could not be updated automatically. New commits were pushed to release/3.1, but I could not automatically push those to merge/release/3.1-to-master to update this PR.
You may need to fix this problem by merging branches with this PR. Contact .NET Core Engineering if you are not sure what to do about this.

[javiercn]

@dougbu This is an example of a failing test (webapi F#) that is clearly not a flakyness issue.

Please do not dismiss things as flaky without investigating, as it hides away real issues.

[javiercn]

A similar issue can be found with the empty F# template.

[dotnet-maestro-bot]

❌ Uh oh, this pull request could not be updated automatically. New commits were pushed to release/3.1, but I could not automatically push those to merge/release/3.1-to-master to update this PR.
You may need to fix this problem by merging branches with this PR. Contact .NET Core Engineering if you are not sure what to do about this.

[dougbu]

This particular test passes ~65% of the time in 'release/3.1'. By definition that means it's flaky. And, when they fail, it's usually more like in https://dev.azure.com/dnceng/public/_build/results?buildId=362349:

F:\workspace\_work\1\s\.dotnet\sdk\5.0.100-alpha1-013788\FSharp\Microsoft.FSharp.Targets(284,9): error MSB6006: "dotnet.exe" exited with code -1073741819. [F:\workspace\_work\1\s\src\ProjectTemplates\test\bin\Release\netcoreapp5.0\TestTemplates\AspNet.mvcnoauthfsharp.qpnuuo32wuo\AspNet.mvcnoauthfsharp.qpnuuo32wuo.fsproj]

Please do not put investigation of failing tests on @aspnet/build. #14022 exists and is assigned to @ajaybhargavb.

[dougbu]

Fixing the test hang and skipping MvcTemplate_NoAuthImplAsync took us from one known-flaky test failing in https://dev.azure.com/dnceng/public/_build/results?buildId=362288 (which later hung), to two failures in https://dev.azure.com/dnceng/public/_build/results?buildId=362349 (which included the hang fix), then to 4 failures in two runs of https://dev.azure.com/dnceng/public/_build/results?buildId=362543

We need to get this in. I'm going to skip the other two F# tests and an obvious baseline issue. Not sure how the baseline tests were passing. Perhaps they didn't run for some reason?

@mkArtakMSFT 026b9df was merged when all project tests were skipped. Needed a reaction in https://github.com/aspnet/AspNetCore/blob/master/src/ProjectTemplates/test/template-baselines.json.

[dotnet-maestro-bot]

❌ Uh oh, this pull request could not be updated automatically. New commits were pushed to release/3.1, but I could not automatically push those to merge/release/3.1-to-master to update this PR.
You may need to fix this problem by merging branches with this PR. Contact .NET Core Engineering if you are not sure what to do about this.

[dotnet-maestro-bot]

❌ Uh oh, this pull request could not be updated automatically. New commits were pushed to release/3.1, but I could not automatically push those to merge/release/3.1-to-master to update this PR.
You may need to fix this problem by merging branches with this PR. Contact .NET Core Engineering if you are not sure what to do about this.

[javiercn]

@dougbu I realize now that this was merged. The issue that I mentioned is not the same you reference. If you see the image I added it comes from a separate point in the test and is a different exception.

In this case its an access violation exception in the F# compiler. I'm not trying to pin the investigations on the build team, but it is not ok to blindly skip or mark tests as flaky.

All I ask is that when we are doing these types of merges whomever is doing the merge takes a quick look at the test output (which we diligently capture) and makes a quick assessment to determine if its an actual product issue or a flaky test issue.

This is a fundamental step to avoid hiding real product issues under the flaky test umbrella. Marking a test as flaky implies that the person making the assessment has a basic understanding of why the test is failing and is confident to assert that there likely isn't a product issue.

For product issues we should be diligent in following up with the subject matter experts or the partner teams to make sure the issues are properly communicated and come to a conclusion.

For flaky test issues we should identity the root cause of the test flakiness and try to fix it in a way that is balanced with the rest of the product work.