Add regression test for untitled scripts in Windows PowerShell #1830
Conversation
Since we could theoretically receive any complicated string.
Specifically, just a raw script.
We were depending on the test run to succeed in order to create and overwrite the file; however, this was very confusing when debugging a single test while it was failing.
andyleejordan
added
Issue-Enhancement
|
@SeeminglyScience could you please re-review? |
| return Uri.IsWellFormedUriString(path, UriKind.RelativeOrAbsolute) && | ||
| !string.Equals(DocumentUri.From(path).Scheme, Uri.UriSchemeFile, StringComparison.OrdinalIgnoreCase); |
There was a problem hiding this comment.
Double-check me here please. As far as I can tell, this is expecting a URI?
| public bool TryGetFile(string filePath, out ScriptFile scriptFile) | ||
| { | ||
| // This might not have been given a file path, in which case the Uri constructor barfs. | ||
| try | ||
| { | ||
| return TryGetFile(new Uri(filePath), out scriptFile); | ||
| } | ||
| catch (UriFormatException) | ||
| { | ||
| scriptFile = null; | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
I had to change this one to catch the exception instead of using Uri.IsWellFormedUriString(path, UriKind.RelativeOrAbsolute) since the path we were being given was like \\foo\\bar, which makes me hesitant about the above too.
| @@ -651,6 +651,7 @@ public void DocumentUriReturnsCorrectStringForAbsolutePath() | |||
| [InlineData("vscode-notebook-cell:/Users/me/Documents/test.ps1#0001", true)] | |||
| [InlineData("https://microsoft.com", true)] | |||
| [InlineData("Untitled:Untitled-1", true)] | |||
| [InlineData("'a log statement' > 'c:\\Users\\me\\Documents\\test.txt'\r\n", false)] | |||
| public void IsUntitledFileIsCorrect(string path, bool expected) => Assert.Equal(expected, ScriptFile.IsUntitledPath(path)); | |||
There was a problem hiding this comment.
See, this test implies that the function is asking "is this path a URI that indicates we're untitled?"
There was a problem hiding this comment.
Perhaps we should expect this case to be true by flipping the logic with IsWellFormedUri. That is, if we're given something that is not a URI, assume it's untitled, but I'm not sure that would work for the first two cases.
There was a problem hiding this comment.
Yeah, doesn't work, since /Users/me/Documents/test.ps1 is not a URI.
|
Tests are passing, but I'm not super happy with it. |
Specifically targeting the edge case where the last line of the script is a comment. Covers PowerShell/vscode-powershell#3965