-
Notifications
You must be signed in to change notification settings - Fork 133
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
VMR smoke test project hardcodes package versions and TFM #4289
Comments
Here's a draft PR that shows the changes that I have in mind: dotnet/installer#19290 |
I agree that those hardcoded versions should be removed. However, it looks like this file was added to support the test infra building independently of the VMR? If that's the case, I want to ensure that these changes will still allow us to run the smoke tests without having to do a full build of the VMR. I often run the smoke tests locally, and it'd be a pain to have to depend on a build of the VMR in order to run them. |
Yes, using the repo infrastructure won't mean that the VMR needs to be built. With my draft PR, running the smoke tests will be easier actually. |
Sounds good! I just wanted to clarify since your PR says "This allows the test project to just be invoked with dotnet test or dotnet build /t:Test after the VMR is built." I'm good with these changes as long as the VMR does not need to be built. Thanks, Viktor. |
In general, I'm on board with these proposed changes. It provides better alignment and better structure of the tests. |
Agreed. |
See https://github.com/dotnet/installer/blob/d28a790713cc31b2293715bd075814df5b5f78de/src/SourceBuild/content/test/Microsoft.DotNet.SourceBuild.SmokeTests/Microsoft.DotNet.SourceBuild.SmokeTests.csproj#L11-L14
Having these hardcodes increases the chance of CG warnings. In addition, the hardcoded xunit version means that this test project won't use the latest xunit as the rest of the stack.
Now that we have another test project in the VMR orchestrator which fully uses the VMR infrastructure, it might make sense to change smoke tests to use the repo infrastructure as well. Currently this isn't happening because of this file: https://github.com/dotnet/installer/blob/main/src/SourceBuild/content/test/Microsoft.DotNet.SourceBuild.SmokeTests/Directory.Build.props
@ellahathaway @MichaelSimons
Let me know if that sounds right to you as I would be happy to submit a PR to make that happen and as part of that also clean-up how the smoke test project is invoked so that is in more in line with the other test project.
The text was updated successfully, but these errors were encountered: