search

microsoft / python-language-server

Microsoft Language Server for Python
Apache License 2.0
912 stars 133 forks source link

Test all newline combinations in round trip tests #248

Open jakebailey opened 6 years ago

jakebailey commented 6 years ago

Refocusing this to be about testing both LF and CRLF with CodeFormattingOptions, since #256 will effectively "fix" the tests not passing.

The round trip tests assume that the C# file they're written in (thanks to @"" strings) is the same as Environment.NewLine. The file's line endings are \r\n since the file was created on Windows, so the tests fail on platforms other than Windows.

Test cases should be normalized, and potentially tested against both \n and \r\n.

MikhailArkhipov commented 6 years ago

We should just assume \n

AlexanderSher commented 6 years ago

Won't work. We have just missed a bug in LineFormatter because of that assumption in tests.

jakebailey commented 6 years ago

In this case, I'm specifically referring to the round-trip tests, which do a character-for-character check to see that the tokenizer and parser will give back exactly what they were given (so long as they were configured to be in verbatim mode).

Honestly, reading over ParserRoundTripTest makes me want to just trim the whole thing down; there's a decent amount of code there that's just for pretty printing string differences that would be better written as a single line of FluentAssertions.

MikhailArkhipov commented 6 years ago

Ah, OK. I thought about baselines, I believe RTVS normalized baseline files to just LF.

jakebailey commented 6 years ago

They could also be converted to DataTests at the same time, though there are so many test cases that it might look really silly in the test explorer. 😛

jakebailey commented 6 years ago

Refocusing this to be about testing both LF and CRLF with CodeFormattingOptions, since #256 will effectively "fix" the tests not passing.

MikhailArkhipov commented 6 years ago

Doesn't have to be done this sprint