Repository navigation
Fix version comparison for dotnet tool install - #22684
Merged
Merged
Conversation
ghost
added
the 
sfoslund
approved these changes
Nov 19, 2021
sfoslund
left a comment
sfoslund
left a comment
Member
There was a problem hiding this comment.
LGTM, but can we add any tests?
Member
Author
I've been thinking about it. It seems none of the tests we have today cover that path because it relies on the .version file of the installed SDK. I'll see if we can add anything |
Member
Author
|
@sfoslund I added a unit test (the change looks wors since I Ctrl-K+D, R+G the test code). |
Member
Author
|
@sfoslund I'm resetting to before trying to add tests. It appears that we had code in the test project for generating the .version file, but that got removed/commented out. It seems that once we generate the file, it breaks a number of msbuild tests and without it we can't test this fix. I'll follow up with Daniel to figure this out, but I need to unblock ASP.NET |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
.version file can contain semantic versions causing Version.Parse to throw an exception. Since we're only comparing against the package framework versions, we'll parse and then discard the semantic elements.