Skip to content

Commit

Permalink
Update jline to 3.24.1
Browse files Browse the repository at this point in the history
Due to dependency conflicts with the version of jline that SBT ships
with and broken ClassLoader created by SBT, we must fork any tests that
use jline. Fortunately, this is a small number of CLI debug related
tests. Because these tests must fork, they are moved to the
daffodil-test-integration project, where daffodil is staged for fork
execution and parallel test execution is disabled to reduce memory
usage. This causes a pretty large increase in execution time for
integration tests, but unfortunately there are not many good
alternatives.

Additionally, because these tests now fork they use jlines normal
TermainalBuilder instead of the DumbTerminal, which fails the
integration tests on windows. To fix this, runCLI adds a new option to
provide environment variables, and these tests provide set options that
force jline to use a dumb terminal.

This also adds a test that should fail if sbt or jline ever change to
fix this issue, allowing us to revert this change so integration tests
do not take as long.

This also found an issue in the CLI where if a bug is found we print the
stack trace to stdout instead of to whatever PrintStream the CLI is
configured to use for stdout. In most cases these are going to be the
same so doesn't matter, but in CLI testing that isn't the case.
  • Loading branch information
scala-steward authored and stevedlawrence committed Nov 30, 2023
1 parent 3b57060 commit 89b7567
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1858,7 +1858,7 @@ class Main(
| to this bug.
|
|""".stripMargin)
e.printStackTrace
e.printStackTrace(STDERR)
1
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,18 +123,6 @@ class TestCLItdml {
}(ExitCode.Success)
}

@Test def test_CLI_Tdml_Debug_singleTest(): Unit = {
val tdml = path(
"daffodil-test/src/test/resources/org/apache/daffodil/section06/entities/Entities.tdml",
)

runCLI(args"-d test $tdml byte_entities_6_08") { cli =>
cli.expect("(debug)")
cli.sendLine("continue")
cli.expect("[Pass] byte_entities_6_08")
}(ExitCode.Success)
}

@Test def test_CLI_Tdml_DebugFile_singleTest(): Unit = {
val tdml = path(
"daffodil-test/src/test/resources/org/apache/daffodil/section06/entities/Entities.tdml",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,9 @@ object Util {
* @param classpaths sequence of paths to add to the classpath. If non-empty,
* runs the CLI in a new process instead of a thread and will likely decrease
* performance
* @param envs map of environment varibles to set. If non-empty,
* runs the CLI in a new process instead of a thread and will likely decrease
* performance
* @param fork if true, forces the the CLI to run in a new process
* @param timeout how long to wait, in seconds, for the testFunc to finish after
* the CLI has completed. Test testFunc is interrupted if this timeout is reached
Expand All @@ -174,13 +177,14 @@ object Util {
def runCLI(
args: Array[String],
classpaths: Seq[Path] = Seq(),
envs: Map[String, String] = Map(),
fork: Boolean = false,
timeout: Int = 10,
debug: Boolean = false,
)(testFunc: (CLITester) => Unit)(expectedExitCode: ExitCode.Value): Unit = {

val (toIn, fromOut, fromErr, cliThreadOrProc: Either[CLIThread, Process]) =
if (classpaths.nonEmpty || fork) {
if (classpaths.nonEmpty || envs.nonEmpty || fork) {
// spawn a new process to run Daffodil, needed if a custom classpath is
// defined or if the caller explicitly wants to fork
val processBuilder = new ProcessBuilder()
Expand All @@ -191,6 +195,10 @@ object Util {
processBuilder.environment().put("DAFFODIL_CLASSPATH", classpath)
}

if (envs.nonEmpty) {
processBuilder.environment().putAll(envs.asJava)
}

val cmd = daffodilBinPath.toString +: args
if (debug) System.out.println(cmd.mkString(" "))
processBuilder.command(cmd.toList.asJava)
Expand Down
Loading

0 comments on commit 89b7567

Please sign in to comment.