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.

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

Run multiple target frameworks and architectures in single vstest.console #3412

Merged
merged 142 commits into from
May 23, 2022
Merged

Run multiple target frameworks and architectures in single vstest.console #3412

merged 142 commits into from
May 23, 2022

Conversation

nohwnd
Copy link
Member

@nohwnd nohwnd commented Feb 23, 2022

Description

The idea here is to grab each source, get tfm and architecture from it, and not care about mixing them.

I am avoiding change to ITestRuntimeProvider, and also insider knowledge in our runtime providers.

Far from done, but the basic flow of running testhosts works.

Shared hosts seem to be broken by the code in the past, and need more investigation if I did not break anything myself.

@nohwnd
Copy link
Member Author

nohwnd commented Feb 23, 2022 •
edited
Loading

Draft. Give me your highlevel concerns. Not every single misplaced space :)

  • Unit tests don't build.
  • Tie loose end in proxy operation manager - done, and added tests
  • Add compat mode where we write all determined frameworks back, and add programmer test
  • TestSessions are not hooked up.
  • Discovery does not send updated runsettings.
  • test/Microsoft.TestPlatform.AcceptanceTests/TranslationLayerTests/CustomTestHostTests.cs remove justrow=1 and fix tests
  • test/Microsoft.TestPlatform.AcceptanceTests/Properties/AssemblyInfo.cs - revert back to ClassLevel
  • fix two tests with NOMERGE

@nohwnd nohwnd changed the title Multi tfm run in vstest.console Run multiple target frameworks and architectures in single vstest.console Mar 1, 2022
@nohwnd nohwnd mentioned this pull request Mar 2, 2022
Closed
Evangelink
Evangelink reviewed Mar 3, 2022
View reviewed changes
src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelOperationManager.cs Outdated Show resolved Hide resolved
src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelOperationManager.cs Outdated Show resolved Hide resolved
src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelOperationManager.cs Outdated Show resolved Hide resolved
src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelOperationManager.cs Show resolved Hide resolved
src/AttachVS/AttachVs.cs Outdated
@@ -143,6 +143,14 @@ private static bool AttachVs(Process vs, int pid)
Trace($"ComException: Retrying in 250ms.\n{ex}");
Thread.Sleep(250);
}
catch (TargetInvocationException ex)
Copy link
Member

Choose a reason for hiding this comment

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

[minor] You could merge with previous catch with something like catch (Exception ex) when (ex is COMException || ex is TargetInvocationException tie && tie.InnerException is COMException).

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you really like the proposed code better than what is there now?

Copy link
Member

Choose a reason for hiding this comment

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

I just like the fact that if we change one block we won't forget the other one :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok changed.

test/vstest.ProgrammerTests/MultiTFMRunAndDiscovery.cs Outdated Show resolved Hide resolved
test/vstest.ProgrammerTests/MultiTFMRunAndDiscovery.cs Outdated Show resolved Hide resolved
test/vstest.ProgrammerTests/MultiTFMRunAndDiscovery.cs Outdated Show resolved Hide resolved
test/vstest.ProgrammerTests/MultiTFMRunAndDiscovery.cs Outdated Show resolved Hide resolved
test/vstest.ProgrammerTests/MultiTFMRunAndDiscovery.cs Outdated Show resolved Hide resolved
Evangelink
Evangelink reviewed May 16, 2022
View reviewed changes
src/Microsoft.TestPlatform.Client/TestPlatform.cs Outdated Show resolved Hide resolved
src/Microsoft.TestPlatform.Common/PublicAPI/PublicAPI.Shipped.txt Show resolved Hide resolved
src/Microsoft.TestPlatform.CrossPlatEngine/Client/Parallel/ParallelProxyDiscoveryManager.cs Outdated Show resolved Hide resolved
src/Microsoft.TestPlatform.CrossPlatEngine/TestSession/ProxyTestSessionManager.cs Outdated Show resolved Hide resolved
src/Microsoft.TestPlatform.ObjectModel/PublicAPI/PublicAPI.Shipped.txt Show resolved Hide resolved
test/Microsoft.TestPlatform.AcceptanceTests/Extension/RunnerCompatibilityDataSource.cs Show resolved Hide resolved
test/Microsoft.TestPlatform.AcceptanceTests/Properties/AssemblyInfo.cs Outdated Show resolved Hide resolved
test/Microsoft.TestPlatform.AcceptanceTests/TranslationLayerTests/CustomTestHostTests.cs Show resolved Hide resolved
test/TestAssets/NuGet.config Outdated Show resolved Hide resolved
src/Microsoft.TestPlatform.CrossPlatEngine/TestEngine.cs Outdated Show resolved Hide resolved
nohwnd
nohwnd commented May 17, 2022
View reviewed changes
test/TestAssets/NuGet.config Outdated Show resolved Hide resolved
@nohwnd
Copy link
Member Author

nohwnd commented May 18, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@nohwnd
Copy link
Member Author

nohwnd commented May 18, 2022

Only failed on ThreadSafeStringWriter.GetOrAddStringBuilder, which is related to mstest not vstest.

@nohwnd
Copy link
Member Author

nohwnd commented May 20, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@nohwnd
Copy link
Member Author

nohwnd commented May 23, 2022 •
edited
Loading

Failed on CUIT test only, merging.

@nohwnd nohwnd merged commit 9802fc0 into microsoft:main May 23, 2022
@nohwnd nohwnd deleted the multi-tfm-run-in-vstestconsole branch May 23, 2022 11:19
@waf waf mentioned this pull request Aug 11, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MarcoRossignoli MarcoRossignoli MarcoRossignoli left review comments

@Evangelink Evangelink Evangelink approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nohwnd @Evangelink @MarcoRossignoli