Fix #5158: Take into account Windows line endings

[melekhove]

Fixes #5158

[dottybot]

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Have an awesome day! ☀️

[allanrenucci]

I would move this into ReplCompilerTests.scala.

Also, what about?

def lines() = storedOutput().lines.toList

[melekhove]

I would move this into ReplCompilerTests.scala.

I thought it might be used in other derived classes, but OK, will move.

Also, what about?

def lines() = storedOutput().lines.toList

Thanks for hint!
It's not exactly the solution but this one works:

def lines() = storedOutput().trim().lines.toList

I will update commit in a few minutes

[allanrenucci]

Can you explain why this is needed?

[melekhove]

The problem is the same as in ReplCompilerTests. The string has \r\n Windows ending while expected only \n

[allanrenucci]

Would you need to do this if we used System.lineSeparator consistently across the file? I.e.

diff --git a/compiler/test/dotty/tools/repl/ScriptedTests.scala b/compiler/test/dotty/tools/repl/ScriptedTests.scala
index 090d49949..6882b2057 100644
--- a/compiler/test/dotty/tools/repl/ScriptedTests.scala
+++ b/compiler/test/dotty/tools/repl/ScriptedTests.scala
@@ -44,7 +44,7 @@ class ScriptedTests extends ReplTest with MessageRendering {
     def evaluate(state: State, input: String, prompt: String) =
       try {
         val nstate = run(input.drop(prompt.length))(state)
-        val out = input + "\n" + storedOutput()
+        val out = input + System.lineSeparator + storedOutput()
         (out, nstate)
       }
       catch {
@@ -60,7 +60,7 @@ class ScriptedTests extends ReplTest with MessageRendering {
       }
 
     val expectedOutput =
-      Source.fromFile(f).getLines().flatMap(filterEmpties).mkString("\n")
+      Source.fromFile(f).getLines().flatMap(filterEmpties).mkString(System.lineSeparator)
     val actualOutput = {
       resetToInitial()
       val inputRes = extractInputs(prompt)
@@ -70,7 +70,7 @@ class ScriptedTests extends ReplTest with MessageRendering {
         buf.append(out)
         nstate
       }
-      buf.flatMap(filterEmpties).mkString("\n")
+      buf.flatMap(filterEmpties).mkString(System.lineSeparator)
     }

[melekhove]

Well, this makes the whole test more consistent, one small change is required for this to work

diff --git a/compiler/test/dotty/tools/repl/ScriptedTests.scala b/compiler/test/dotty/tools/repl/ScriptedTests.scala
index 78ba60aba8d..3936702e449 100644
--- a/compiler/test/dotty/tools/repl/ScriptedTests.scala
+++ b/compiler/test/dotty/tools/repl/ScriptedTests.scala
@@ -44,7 +44,7 @@ class ScriptedTests extends ReplTest with MessageRendering {
     def evaluate(state: State, input: String, prompt: String) =
       try {
         val nstate = run(input.drop(prompt.length))(state)
-        val out = input + "\n" + storedOutput()
+        val out = (input :: storedOutput().trim().lines.toList).mkString(System.lineSeparator())
         (out, nstate)
       }
       catch {

Which looks is a bit artificial, but I don't see more succinct/clear way to do that.
So, in case if you don't have any objections I will commit this variant. What do you think?

[allanrenucci]

I don't understand why storedOutput needs to be trimmed and modified. To me doing something like:

def f(s: String) = s.lines.mkString(System.lineSeparator)

is the identity transformation.

Maybe, you could explain by showing what doesn't work in one of the failing tests

[melekhove]

trimming may not be necessary (just to be safe) But it is not the identity transformation.

s.lines.mkString(System.lineSeparator) converts line containing 0x0A or 0x0D, 0x0A as separators into a string containing System.lineSeparator since we decided to use it in other parts of the test,

Right now storedOutput() contains 0x0A but it's better not to rely on that and use .lines to convert it properly (the same result can be achieved using regexp as it was done in my first implementation as far as I remember)

[allanrenucci]

Closing this PR. REPL tests now work on Windows. It was fixed in eb175cb.

Thanks @melekhove!