Reenable terminal tests on CI

[Blaisorblade]

The issue is discussed at #4952 (comment):
console tries to open a terminal, on CI we can only open dumb ones, but we configured JLine to fail then. If we don't, the tests pass.

Since opening dumb terminals is necessary in real usecases where the terminal emulator is limited, this might be good to go; otherwise we can only enable dumb terminals in CI.

/cc @SzymonPajzert: is this fix sufficient for your testing goals? We aren't doing that much testing yet...

[allanrenucci]

The test is only run on nightly. You need to uncomment:
https://github.com/lampepfl/dotty/blob/0d7826c8a77bfa0da727ac21213475739faa9428/.drone.yml#L45-L47

[Blaisorblade]

Thanks.

[Blaisorblade]

Oh, the last failure is spurious (#5021) but the actual issue is fixed!
Issue shown at: http://dotty-ci.epfl.ch/lampepfl/dotty/7070/6
And fixed in: http://dotty-ci.epfl.ch/lampepfl/dotty/7072/6

[allanrenucci]

This need to be reverted

[allanrenucci]

I am against enabling this by default. Last time I tried, the dumb terminal was very limited and I don't want to expose it to users. E.g.

• no syntax highlighting
• no history
• no auto-completion
• no multi-line editing

You can experiment with dumb terminal with these changes:

diff --git a/compiler/src/dotty/tools/repl/JLineTerminal.scala b/compiler/src/dotty/tools/repl/JLineTerminal.scala
index 22e44e4a5..9c33a7b87 100644
--- a/compiler/src/dotty/tools/repl/JLineTerminal.scala
+++ b/compiler/src/dotty/tools/repl/JLineTerminal.scala
@@ -17,9 +17,12 @@ final class JLineTerminal extends java.io.Closeable {
   // import java.util.logging.{Logger, Level}
   // Logger.getLogger("org.jline").setLevel(Level.FINEST)
 
-  private val terminal = TerminalBuilder.builder()
-    .dumb(false) // fail early if not able to create a terminal
-    .build()
+  // private val terminal = TerminalBuilder.builder()
+  //   .dumb(false) // fail early if not able to create a terminal
+  //   .build()
+  import java.lang.System
+  private val terminal =
+    new org.jline.terminal.impl.DumbTerminal(System.in, System.out)
   private val history = new DefaultHistory
 
   private def blue(str: String) = Console.BLUE + str + Console.RESET

I would be Ok to make this configurable in order to use dumb terminal in tests. However, we clearly need to document what can be tested

[Blaisorblade]

Oh, I just wanted to enable it as fallback if there is no alternative. Even a dumb REPL seems better than no REPL at all? Tho we should probably warn the user to fix their terminal.

Anyway, supporting dumb terminals for users is a separate question.

[allanrenucci]

A more extensive REPL test would be nice. I don't know to what extent we can test the REPL with scripted tests. E.g. can we test something like #3007?

[Blaisorblade]

IIUC, this already tests that starting the console succeeds, which seems why @SzymonPajzert was interested for #4933.

If we want to connect something to the actual terminal, give commands and wait for responses, one could use expect-like programs.
If we bypass the terminal logic, it seems we'd have to add a consoleTest script command which combines console with ReplTest: it'd be a ConsoleInterface creating, instead of ReplDriver, a ScriptedTests on the script. EDIT: but that, and interfacing to sbt for that, seems much more than I plan for this PR or I can take on.

[SzymonPajzert]

Hi, yes, that would be probably enough at the beginning, probably more tests will be added with resolution of #5069, but for now it seems sufficient for me.

[odersky]

@Blaisorblade Inactive for 4 months. What should we do with this?

[odersky]

/cc @allanrenucci We should reach a decision what to do with this.

[Blaisorblade]

I'm afraid I won't get back to this one any time soon.