-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Update the hardcoded netcoreapp3.0 to the current framework #24979
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
Conversation
|
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
| var outputDirectory = buildCommand.GetOutputDirectory(testProject.TargetFrameworks); | ||
| outputDirectory.Should().HaveFiles(new[] { | ||
| runtimeConfigName, | ||
| $"{testProject.Name}.runtimeconfig.dev.json" |
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.
was this file renamed from 3.0 to now?
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.
No, we stopped generating it: #17014
| TargetFrameworkMoniker = $".NETCoreApp,Version=v{ToolsetInfo.CurrentTargetFrameworkVersion}", | ||
| RuntimeConfigPath = _runtimeConfigPath, | ||
| RuntimeConfigDevPath = _runtimeConfigDevPath, | ||
| RuntimeFrameworks = new[] |
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.
The mock items below this are all 3.1.0 frameworks. I believe those should get updated too.
| TargetFrameworkMoniker = $".NETCoreApp,Version=v{ToolsetInfo.CurrentTargetFrameworkVersion}", | ||
| RuntimeConfigPath = _runtimeConfigPath, | ||
| RuntimeConfigDevPath = _runtimeConfigDevPath, | ||
| RuntimeFrameworks = new[] |
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.
Same comment as above with the below mocktaskitems.
| [InlineData("latestMinor", "netcoreapp3.0", true)] | ||
| [InlineData("Invalid", "netcoreapp3.0", false)] | ||
| [InlineData("latestMinor", ToolsetInfo.CurrentTargetFramework, true)] | ||
| [InlineData("Invalid", ToolsetInfo.CurrentTargetFramework, false)] |
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.
Reading this test, I think it's trying to rollforward to the latest minor. Since 5.0 and 6.0 don't have minors, it probably makes sense to keep this on 3.0.
| TargetFrameworkMoniker = $".NETCoreApp,Version=v{ToolsetInfo.CurrentTargetFrameworkVersion}", | ||
| RuntimeConfigPath = _runtimeConfigPath, | ||
| RuntimeConfigDevPath = _runtimeConfigDevPath, | ||
| RuntimeFrameworks = new[] |
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.
I think the 3.0.0-preview1.100 should be a 6.0 version since it appears to be trying to roll forward on the latest minor if I'm reading this right.
| new Dictionary<string, string> | ||
| { | ||
| {"FrameworkName", "Microsoft.NETCore.App"}, {"Version", "3.1.0"} | ||
| {"FrameworkName", "Microsoft.NETCore.App"}, {"Version", "6.0.5"} |
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.
It probably makers sense to make all of these 6.0.0 versions. See how before it was 3.1.0. I think it's simpler if we're specifying a specific version of 6.0 to go with the first one as otherwise, different tests might end up doing different ones and anyone reading the code might wonder why we picked .5.
|
Done |
No description provided.