Skip to content
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

[release/6.0.1xx] Migrate to 1ES hosted pools #5508

Closed

Conversation

lpatalas
Copy link

@lpatalas lpatalas commented Sep 20, 2021

We need to migrate pools to 1ES hosted pools. Only the yamls have to be changed. The supported functionality stays the same.

Tracking issue: https://github.com/dotnet/core-eng/issues/14276
/CC: @jonfortescue

@lpatalas lpatalas requested a review from a team as a code owner September 20, 2021 14:19
@lpatalas
Copy link
Author

Some tests failed but the logs does not contain the actual error. I can't see "Test execution summary" in the logs so it looks as if the runner crashed halfway. We had some problems in dotnet/roslyn related to the interactive login setting: dotnet/roslyn#56186 (comment). I wonder if this may be related?

xUnit.net Console Runner v2.4.1 (64-bit Desktop .NET 4.7.2, runtime: 4.0.30319.42000)
  Discovering: Microsoft.CodeAnalysis.NetAnalyzers.UnitTests
  Discovered:  Microsoft.CodeAnalysis.NetAnalyzers.UnitTests
  Starting:    Microsoft.CodeAnalysis.NetAnalyzers.UnitTests
    Microsoft.CodeQuality.Analyzers.ApiDesignGuidelines.UnitTests.AvoidEmptyInterfacesTests.TestConflictingAnalyzerOptionsForPartialsAsync [SKIP]
      https://github.com/dotnet/roslyn-analyzers/issues/3494
    Microsoft.NetCore.Analyzers.Performance.UnitTests.PreferIsEmptyOverCountTests.CSharpTestIsEmptyGetter_NoDiagnosisAsync [SKIP]
      Removed default support for all types but this scenario can be useful for .editorconfig
    Microsoft.NetCore.Analyzers.Performance.UnitTests.PreferIsEmptyOverCountTests.BasicTestIsEmptyGetter_NoDiagnosisAsync [SKIP]
      Removed default support for all types but this scenario can be useful for .editorconfig
    Microsoft.NetCore.Analyzers.Performance.UnitTests.CSharpUsePropertyInsteadOfCountMethodWhenAvailableOverlapTests_Concurrent.NonZeroEqualsCount_WithoutPredicate_FixedAsync [SKIP]
      https://github.com/dotnet/roslyn-analyzers/issues/3700
    Microsoft.CodeQuality.Analyzers.Maintainability.UnitTests.DoNotIgnoreMethodResultsTests.UnusedComImportPreserveSigAsync [SKIP]
      https://github.com/dotnet/roslyn-analyzers/issues/746
=== COMMAND LINE ===
"D:\a\_work\1\s\.packages\xunit.runner.console\2.4.1\tools\net472\xunit.console.exe" "D:\a\_work\1\s\artifacts\bin\Microsoft.CodeAnalysis.NetAnalyzers.UnitTests\Debug\net472\Microsoft.CodeAnalysis.NetAnalyzers.UnitTests.dll" -noshadow -xml "D:\a\_work\1\s\artifacts\TestResults\Debug\Microsoft.CodeAnalysis.NetAnalyzers.UnitTests_net472_x64.xml" -html "D:\a\_work\1\s\artifacts\TestResults\Debug\Microsoft.CodeAnalysis.NetAnalyzers.UnitTests_net472_x64.html"  > "D:\a\_work\1\s\artifacts\log\Debug\Microsoft.CodeAnalysis.NetAnalyzers.UnitTests_net472_x64.log" 2>&1

@jonfortescue
Copy link
Contributor

/azp run

1 similar comment
@jonfortescue
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 5508 in repo dotnet/roslyn-analyzers

@jonfortescue
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 5508 in repo dotnet/roslyn-analyzers

1 similar comment
@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 5508 in repo dotnet/roslyn-analyzers

@jmarolf
Copy link
Contributor

jmarolf commented Oct 22, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@lpatalas
Copy link
Author

I've merged the branch with the latest release/6.0.1xx but some tests are still failing. We have already enabled interactive login on the machines so it's probably not connected to this. The images should be basically the same between the pools so it's surprising but I will try to investigate it some more.

@lpatalas lpatalas force-pushed the migrate-6.0.1xx-to-1es branch from 805b4ba to b97ad21 Compare October 26, 2021 17:10
@lpatalas
Copy link
Author

I was trying to reproduce the error but I had no success. The tests pass successfully both on my local machine as well as on standalone VM created from the same image as we have on the 1ES pool. I'm a bit lost what could be the reason of the failure. We also see similar failures on main branch: #5674.

@jmarolf Do you have any suspicion what could be the reason for the test failures in this PR? I would really appreciate some tips on how to solve it.

@codecov
Copy link

codecov bot commented Dec 8, 2021

Codecov Report

Merging #5508 (a9203ac) into release/6.0.1xx (e4bb418) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@                 Coverage Diff                 @@
##           release/6.0.1xx    #5508      +/-   ##
===================================================
- Coverage            95.53%   95.52%   -0.01%     
===================================================
  Files                 1275     1275              
  Lines               292697   292697              
  Branches             17701    17701              
===================================================
- Hits                279619   279613       -6     
- Misses               10652    10662      +10     
+ Partials              2426     2422       -4     

@jmarolf
Copy link
Contributor

jmarolf commented Dec 17, 2021

@lpatalas can you add this to the pipeline: https://github.com/dotnet/roslyn-sdk/blob/9e0d174c165bccdc18f8231c4268b1a19f2be420/.vsts-pr.yaml#L52-L55

so we can get dumps of what is happening?

@mavasani
Copy link
Contributor

@lpatalas Is this PR still relevant?

@lpatalas
Copy link
Author

I still see two places where old pools are used so I think it's still relevant. @jmarolf and @JoeRobich were doing some investigation of build failures in #5772 and #5674. I'm not sure if those failures are still blocking the merge?

Just as a reminder we are decommissioning old pools tomorrow (February 1, 2022) so any jobs using old pools won't run after this date.

@JoeRobich
Copy link
Member

@lpatalas #5772 has merged. Closing this out.

@JoeRobich JoeRobich closed this Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants