stevedlawrence commented on code in PR #865:
URL: https://github.com/apache/daffodil/pull/865#discussion_r1015424895
##########
daffodil-cli/src/it/scala/org/apache/daffodil/CLI/Util.scala:
##########
@@ -17,183 +17,501 @@
package org.apache.daffodil.CLI
-import org.apache.daffodil.util.Misc
-import net.sf.expectit.ExpectBuilder
+import java.io.File
+import java.io.InputStream
+import java.io.OutputStream
+import java.io.PipedInputStream
+import java.io.PipedOutputStream
+import java.io.PrintStream
+import java.lang.ProcessBuilder
+import java.math.BigInteger
+import java.nio.file.Files
+import java.nio.file.Path
+import java.nio.file.Paths
+import java.security.MessageDigest
+import java.util.concurrent.TimeUnit
+
+import scala.collection.JavaConverters._
+import scala.collection.mutable
+
+import com.fasterxml.jackson.core.io.JsonStringEncoder
+
import net.sf.expectit.Expect
+import net.sf.expectit.ExpectBuilder
+import net.sf.expectit.Result
import net.sf.expectit.filter.Filters.replaceInString
+import net.sf.expectit.matcher.Matcher
import net.sf.expectit.matcher.Matchers.contains
-import org.apache.daffodil.Main.ExitCode
-import java.nio.file.Paths
-import java.io.{File, PrintWriter}
-import java.util.concurrent.TimeUnit
-import org.apache.daffodil.xml.XMLUtils
-import org.junit.Assert.fail
+import org.apache.commons.io.FileUtils
-object Util {
+import org.apache.logging.log4j.Level
+import org.apache.logging.log4j.core.appender.OutputStreamAppender
+import org.apache.logging.log4j.core.config.AbstractConfiguration
+import org.apache.logging.log4j.core.config.ConfigurationSource
+import org.apache.logging.log4j.core.config.Configurator
+import org.apache.logging.log4j.core.layout.PatternLayout
- //val testDir = "daffodil-cli/src/it/resources/org/apache/daffodil/CLI/"
- val testDir = "/org/apache/daffodil/CLI/"
- val outputDir = testDir + "output/"
+import org.junit.Assert.assertEquals
- val isWindows =
System.getProperty("os.name").toLowerCase().startsWith("windows")
+import org.apache.daffodil.Main
+import org.apache.daffodil.Main.ExitCode
- val dafRoot = sys.env.getOrElse("DAFFODIL_HOME", ".")
+object Util {
- def daffodilPath(dafRelativePath: String): String = {
- XMLUtils.slashify(dafRoot) + dafRelativePath
- }
+ private val isWindows =
System.getProperty("os.name").toLowerCase().startsWith("windows")
- val binPath = Paths.get(dafRoot, "daffodil-cli", "target", "universal",
"stage", "bin", String.format("daffodil%s", (if (isWindows) ".bat" else
""))).toString()
+ private val daffodilRoot = sys.env.getOrElse("DAFFODIL_HOME", ".")
- def getExpectedString(filename: String, convertToDos: Boolean = false):
String = {
- val rsrc = Misc.getRequiredResource(outputDir + filename)
- val is = rsrc.toURL.openStream()
- val source = scala.io.Source.fromInputStream(is)
- val lines = source.mkString.trim()
- source.close()
- fileConvert(lines)
+ private val daffodilBinPath = {
+ val ext = if (isWindows) ".bat" else ""
+ Paths.get(daffodilRoot,
s"daffodil-cli/target/universal/stage/bin/daffodil$ext")
}
- def start(cmd: String, envp: Map[String, String] = Map.empty[String,
String], timeout: Long = 30): Expect = {
- val spawnCmd = if (isWindows) {
- "cmd /k" + cmdConvert(cmd)
- } else {
- "/bin/bash"
- }
-
- getShell(cmd, spawnCmd, envp, timeout)
+ /**
+ * Convert the daffodilRoot + parameter to a java Path. The string
+ * parameter should contain unix path sparators and it will be interpreted
+ * correctly regardless of operating system. When converted to a string to
+ * send to the CLI, it will use the correct line separator for the
+ * operating system
+ */
+ def path(string: String): Path = {
+ Paths.get(daffodilRoot, string)
}
- // This function will be used if you are providing two separate commands
- // and doing the os check on the 'front end' (not within this utility class)
- def startNoConvert(cmd: String, envp: Map[String, String] =
Map.empty[String, String], timeout: Long = 30): Expect = {
- val spawnCmd = if (isWindows) {
- "cmd /k" + cmd
- } else {
- "/bin/bash"
- }
+ def devNull(): String = if (isWindows) "NUL" else "/dev/null"
- return getShell(cmd, spawnCmd, envp = envp, timeout = timeout)
- }
-
- // Return a shell object with two streams
- // The inputStream will be at index 0
- // The errorStream will be at index 1
- def getShell(cmd: String, spawnCmd: String, envp: Map[String, String] =
Map.empty[String, String], timeout: Long): Expect = {
- val newEnv = sys.env ++ envp
-
- val envAsArray = newEnv.toArray.map { case (k, v) => k + "=" + v }
- val process = Runtime.getRuntime().exec(spawnCmd, envAsArray)
- val shell = new ExpectBuilder()
- .withInputs(process.getInputStream(), process.getErrorStream())
- .withInputFilters(replaceInString("\r\n", "\n"))
- .withOutput(process.getOutputStream())
- .withEchoOutput(System.out)
- .withEchoInput(System.out)
- .withTimeout(timeout, TimeUnit.SECONDS)
- .withExceptionOnFailure()
- .build();
- if (!isWindows) {
- shell.send(cmd)
+ def md5sum(path: Path): String = {
+ val md = MessageDigest.getInstance("MD5")
+ val buffer = new Array[Byte](8192)
+ val stream = Files.newInputStream(path)
+ var read = 0
+ while ({read = stream.read(buffer); read} > 0) {
+ md.update(buffer, 0, read)
}
- return shell
+ val md5sum = md.digest()
+ val bigInt = new BigInteger(1, md5sum)
+ bigInt.toString(16)
}
- def cmdConvert(str: String): String = {
- if (isWindows)
- str.replaceAll("/", "\\\\")
- else
- str
+ /**
+ * Create a temporary file in /tmp/daffodil/, call a user provided function
+ * passing in the Path to that new file, and delete the file when the
+ * function returns.
+ */
+ def withTempFile(f: (Path) => Unit) : Unit = withTempFile(null, f)
+
+ /**
+ * Create a temporary file in /tmp/daffodil/ with a givin suffix, call a user
+ * provided function passing in the Path to that new file, and delete the
+ * file when the function returns.
+ */
+ def withTempFile(suffix: String, f: (Path) => Unit): Unit = {
+ val tempRoot = Paths.get(System.getProperty("java.io.tmpdir"), "daffodil")
+ Files.createDirectories(tempRoot)
+ val tempFile = Files.createTempFile(tempRoot, "daffodil-", suffix)
+ try {
+ f(tempFile)
+ } finally {
+ tempFile.toFile.delete()
+ }
}
- def fileConvert(str: String): String = {
- val newstr = str.replaceAll("\r\n", "\n")
- return newstr
+ /**
+ * Create a temporary directory in /tmp/daffodil/, call a user provided
+ * function passing in the Path to that new directory, and delete the
+ * directory and all of its contents when the function returns
+ */
+ def withTempDir(f: (Path) => Unit): Unit = {
+ val tempRoot = Paths.get(System.getProperty("java.io.tmpdir"), "daffodil")
+ Files.createDirectories(tempRoot)
+ val tempDir = Files.createTempDirectory(tempRoot, "daffodil-")
+ try {
+ f(tempDir)
+ } finally {
+ FileUtils.deleteDirectory(tempDir.toFile)
+ }
}
- def echoN(str: String): String = {
- if (isWindows) {
- "echo|set /p=" + str
- } else {
- "echo -n " + str
+ /**
+ * Set a system property using a provided key, value tuple, call a user
+ * provided function, and reset or clear the property when the function
+ * returns.
+ */
+ def withSysProp(keyVal: (String, String))(f: => Unit): Unit = {
+ val key = keyVal._1
+ val newVal = keyVal._2
+ val oldVal = System.setProperty(key, newVal)
+ try {
+ f
+ } finally {
+ if (oldVal == null) {
+ System.clearProperty(key)
+ } else {
+ System.setProperty(key, oldVal)
+ }
}
}
- def devNull(): String = {
- if (isWindows) {
- "NUL"
- } else {
- "/dev/null"
+ /**
+ * Run a CLI test.
+ *
+ * Runs CLI logic using the provided arguments and classpath, creates a
+ * CLITester so that the user can send input and validate output, and
+ * verifies the expected exit code.
+ *
+ * For performance reasons, this defaults to running the CLI in a new thread
+ * unless the classpaths parameter is nonempty or he fork parameter is set to
+ * true. Otherwise a new process is spawned.
+ *
+ * @param args arguments to pass to the CLI. This should not include the
+ * daffodil binary
+ * @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 fork if true, forces the the CLI in a new process
+ * @param timeout how long to wait, in seconds, for the CLI to exit after the
+ * testFunc has returned. Also how long to wait for individual expect
+ * operations in the CLITester
+ * @param debug if true, prints arguments and classpath information to
+ * stdout. Also echos all CLITester input and output to stdout.
+ * @param testFunc function to call to send input to the CLI and validate
+ * output from CLI stdout/stderr.
+ * @param expectedExitCode the expected exit code of the CLI. In the actual
+ * exit code does not match
+ *
+ * @throws AssertionError if the actual exit code does not match the
expected exit code
+ * @throws ExpectIOException if the an CLITester expect validation operation
fails
+ */
+ def runCLI
+ (args: Array[String], classpaths: Seq[Path] = Seq(), fork: Boolean =
false, timeout: Int = 10, debug: Boolean = false)
+ (testFunc: (CLITester) => Unit)
+ (expectedExitCode: ExitCode.Value): Unit = {
+
+ val (toIn, fromOut, fromErr, threadOrProc: Either[CLIThread, Process]) =
+ if (classpaths.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()
+
+ if (classpaths.nonEmpty) {
+ val classpath = classpaths.mkString(File.pathSeparator)
+ if (debug) System.out.println(s"DAFFODIL_CLASSPATH=$classpath")
+ processBuilder.environment().put("DAFFODIL_CLASSPATH", classpath)
+ }
+
+ val cmd = daffodilBinPath.toString +: args
+ if (debug) System.out.println(cmd.mkString(" "))
+ processBuilder.command(cmd.toList.asJava)
+
+ val process = processBuilder.start()
+
+ val toIn = process.getOutputStream()
+ val fromOut = process.getInputStream()
+ val fromErr = process.getErrorStream()
+ (toIn, fromOut, fromErr, Right(process))
+ } else {
+ // create a new thread for the CLI test to run, using piped
+ // input/output streams to connected the thread and the CLItester
+ val in = new PipedInputStream()
+ val toIn = new PipedOutputStream(in)
+
+ val out = new PipedOutputStream()
+ val fromOut = new PipedInputStream(out)
+
+ val err = new PipedOutputStream()
+ val fromErr = new PipedInputStream(err)
+
+ if (debug) System.out.println("daffodil " + args.mkString(" "))
+
+ val thread = new CLIThread(args, in, out, err)
+ thread.start()
+ (toIn, fromOut, fromErr, Left(thread))
+ }
+
+ val eb = new ExpectBuilder()
+ eb.withOutput(toIn)
+ eb.withInputs(fromOut, fromErr)
+ eb.withInputFilters(replaceInString("\r\n", "\n"))
+ eb.withTimeout(timeout, TimeUnit.SECONDS)
+ eb.withExceptionOnFailure()
+ if (debug) {
+ eb.withEchoOutput(System.out)
+ eb.withEchoInput(System.out)
+ }
+ val expect = eb.build()
+ val tester = new CLITester(expect, toIn)
+
+ try {
+ testFunc(tester)
+ } finally {
+ threadOrProc match {
+ case Left(thread) => thread.join(timeout * 1000)
+ case Right(process) => process.waitFor(timeout, TimeUnit.SECONDS)
+ }
+ expect.close()
+ toIn.close()
+ fromOut.close()
+ fromErr.close()
}
- }
- def makeMultipleCmds(cmds: Array[String]): String = {
- if (isWindows) {
- cmds.mkString(" & ")
- } else {
- cmds.mkString("; ")
+ val actualExitCode = threadOrProc match {
+ case Left(thread) => thread.exitCode
+ case Right(process) => ExitCode(process.exitValue)
}
+ assertEquals("Incorrect exit code,", expectedExitCode, actualExitCode)
}
- def md5sum(blob_path: String): String = {
- if (isWindows) {
- String.format("certutil -hashfile %s MD5", blob_path)
- } else {
- String.format("md5sum %s", blob_path)
+ /**
+ * A class to run the CLI in a thread instead of a new process, given the
+ * arguments to use (excluded the daffodil binary) and streams to use for
+ * stdin/out/err.
+ */
+ private class CLIThread(args: Array[String], in: InputStream, out:
OutputStream, err: OutputStream) extends Thread {
+ var exitCode = ExitCode.Failure
+
+ override def run(): Unit = {
+ val psOut = new PrintStream(out)
+ val psErr = new PrintStream(err)
+
+ // configure the CLI and log4j to use our custom streams, nothing should
+ // not actually use stdin/stdout/stderr
Review Comment:
I used this command to find non-test related println/printf:
grep -R --exclude-dir=test -e '\(println\|printf\)' *
This found the following:
* Main/CLIDebuggerRunner/TraceDebuggerRunner - these all use new logic to
output to a specified stream
* DirectOrBufferedDataOutputStream.scala - this println occurs in the
`dumpChain()` function, which is used for debugging suspensions
* PropertyGenerator.scala - run by sbt to generate code--needs to always
output to stdout for sbt
* CodeGenerator.scala - run by sbt to update example files--needs to be
output to stdout for sbt
* Tak.scala - used for timing TDML tests/performance related
All other instances are in commented out code. So I think all instances that
related to the CLI have been fixed, or are intended to output to stdout. We
might want to go through all the tests and commented out code and inspect those
println's to see if they are really needed, but I think that's out of scope for
this ticket.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
