LanguageDev / Yoakke

A collection of libraries for implementing compilers in .NET.
https://languagedev.github.io/Yoakke/
Apache License 2.0
143 stars 7 forks source link

Unit tests fail depending on the line terminator style the source code is saved with #46

Closed skneko closed 3 years ago

skneko commented 3 years ago

Describe the bug Unit tests in Yoakke.Reporting.Tests (in file TextPresenterTests.cs) will not pass if the source code file is saved with a certain line ending style (e.g. CRLF vs LF) that does not match the style returned by the code under test. This happens because Assert.AreEqual compares strings exactly character by character, without ignoring the line endings, and the expected texts use @-strings, taking whatever line terminators the source code file is using. For example, if the source code file is saved using LF line terminators (Linux style) but the returned strings use CRLF (Windows style) the tests will fail even if the content of said strings except for the line terminators is identical.

I think it would be better to compare by ignoring differences in line terminators (maybe using StringComparison?). In case the line endings must be CRLF or LF explicitly, I think they should be encoded explicitly using escape secuences such as \r\n or \n to make it clear we do not expect any other line termination style.

Which libraries does it affect? All unit test projects using @-strings, such as Yoakke.Reporting.Tests.

To Reproduce

  1. Save all source code files of the project with line terminator style LF.
  2. Pass tests in Yoakke.Reporting.Tests. They should succeed.
  3. Save the source code file TextPresenterTests.cs with line terminator style CRLF.
  4. The tests will now fail even though no code has been changed semantically.

Expected behavior The tests should pass or fail in the same way, depending only on the semantics of the code, and regardless of the line terminators of the source code files involved.

Environment:

Additional context Discussion about this issue started in #9.

LPeter1997 commented 3 years ago

Indeed, this is a big issue. Initially I would have thought that explicit newline characters would be the best option, but then I realized that most code depends on what the underlying TextWriter does.

So I agree, that comparing ignoring newline differences would be the best. Sadly, I didn't find any existing functionality, neither in the string.Compare functions, nor in the MSTest Assert helpers (which just seems to copy the interface for string.Compare).

We could introduce something like so:

public class AssertHelpers
{
    private static string NormalizeNewlines(string s) => s.Replace("\r\n", "\n").Replace('\r', '\n');

    public static void AreEqualIgnoreNewlineDifferences(string a, string b) =>
        Asssert.AreEqual(NormalizeNewlines(a), NormalizeNewlines(b));
}

Of course this is just an idea of what we could do, I'm open to better implementations and better names if someone wants to grab this issue.

LPeter1997 commented 3 years ago

Oops, forgot a few things ;) . @all-contributors please add @skneko for tests, userTesting, bug, question

allcontributors[bot] commented 3 years ago

@LPeter1997

I've put up a pull request to add @skneko! :tada: