-
-
Notifications
You must be signed in to change notification settings - Fork 741
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
Added VS2022 to MSTest, VSTest #3775
Conversation
Also, for completeness, I added the "BuildTools" edition, though I feel MSTest is not deployed in that edition. Additionally I added a setting "AllowPreviewVersion" to the MSTestSettings which, if enabled, allows searching the "Preview" edition for an installed MSTest.exe
Also, I added the "BuildTools" edition to VS2019 and 2017 to be searched. Additionally I added a setting "AllowPreviewVersion" to the VSTestSettings which, if enabled, allows searching the "Preview" edition for an installed vstest.console.exe
which, if enabled, allows searching the "Preview" edition for an installed msbuild.exe. This syncronizes the behaviour between MSTest, VSTest and MSBuild.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM overall, just a question on the search order - I think we'd only run a Preview
version as a last resort, if the none of the other editions are present.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general LGTM 👍
But See a lot of duplication, we could probably add a helper class
internal static class VisualStudioEditions
{
internal static ICollection<string> Preview { get; } = new [] {
"Preview"
};
internal static ICollection<string> Stable { get; } = new [] {
"Enterprise",
"Professional",
"Community",
"BuildTools"
};
internal static ICollection<string> All { get; } = Preview
.Concat(Stable)
.ToArray();
}
then we don't need to create a new list or insert but just do something like
foreach(edition in settings.AllowPreviewVersion
? VisualStudioEditions.All
: VisualStudioEditions.Stable)
...
to find/lookup Visual Studio installations. Those were previously duplicated among MSBuild, MSTest and VSTest
@devlead I added a helper to group some common functionality. The change got a little bigger, sorry. 🤷♂️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
@nils-a your changes have been merged, thanks for your contribution 👍 |
Can someone (when possible) confirm that this change set will not break VS2022 Build tools only install resolution? From what I understand #3678 Fixed Cake 2.x to also look for VS2022 build tools in x86 ProgFiles where it seems to install the build tools, and from the change to test Should_Get_Correct_Path_To_MSBuild_Version_17_When_Only_Build_Tools_Are_Installed, Cake now makes the assumptions it is in x64 ProgFiles again ? |
Yes someone could use the pre-release version already todayand verify in such an environment A NuGet.config for pre-release feed could look something like below ( full instructions here) <?xml version="1.0" encoding="utf-8"?>
<configuration>
<packageSources>
<clear />
<add key="cake@Local" value="https://pkgs.dev.azure.com/cake-build/Cake/_packaging/cake%40Local/nuget/v3/index.json" />
</packageSources>
</configuration> Probably someone could create such an environment using a Windows container with only build tools installed in dockerfile i.e. using Chocolatey. If so probably best to raise a new issue so someone could tackle adding such an integration test, and if such a test encounters an issue raise an bug so it's addressed. Easy for to miss things when commenting on closed or merged issues/prs. |
@WGroenestein, it seems you are correct. (Though I could have sworn that I had checked that...) BuildTools is the only edition of VS2022 that is installed in the I have created #3794 to track that. |
This also synchronized the behavior between
MSTest
,VSTest
andMSBuild
:AllowPreviewVersion
that, if set totrue
enables searching the Preview edition.fixes #3772