stevedlawrence commented on a change in pull request #759:
URL: https://github.com/apache/daffodil/pull/759#discussion_r811913090
##########
File path:
daffodil-cli/src/it/scala/org/apache/daffodil/parsing/TestCLIParsing.scala
##########
@@ -1319,4 +1319,34 @@ class TestCLIparsing {
}
+ @Test def test_2575_DFDLX_Trace_output(): Unit = {
+
+ val schemaFile =
Util.daffodilPath("daffodil-cli/src/it/resources/org/apache/daffodil/CLI/trace_input.dfdl.xsd")
+ val inputFile =
Util.daffodilPath("daffodil-cli/src/it/resources/org/apache/daffodil/CLI/input/trace_input.txt")
+ val log4jConfig =
Util.daffodilPath("daffodil-cli/src/it/resources/org/apache/daffodil/CLI/conf/log4j2_traceOutput.xml")
+ val (testSchemaFile, testInputFile, testLog4jConfig) = if (Util.isWindows)
(Util.cmdConvert(schemaFile), Util.cmdConvert(inputFile),
Util.cmdConvert(log4jConfig)) else (schemaFile, inputFile, log4jConfig)
+
+ val DAFFODIL_JAVA_OPTS = Map("DAFFODIL_JAVA_OPTS" ->
("-Dlog4j.configurationFile=" + testLog4jConfig + " -Xms256m -Xmx2048m
-Dfile.encoding=UTF-8"))
+
+ val shell = Util.start("", envp = DAFFODIL_JAVA_OPTS)
+
+ try{
+
+ val cmd = String.format("%s -vvvv parse -r output -s %s %s",
Util.binPath, testSchemaFile, testInputFile)
+
+ shell.sendLine(cmd)
+
+ //show stderr has no output
+ shell.expectIn(1,contains(""))
Review comment:
Why have the log output go to stdout instead of stderr. Reduces the
possibility of ever having a case where the actual output of daffodil is
confused with the actual log output.
##########
File path:
daffodil-cli/src/it/scala/org/apache/daffodil/parsing/TestCLIParsing.scala
##########
@@ -1319,4 +1319,34 @@ class TestCLIparsing {
}
+ @Test def test_2575_DFDLX_Trace_output(): Unit = {
+
+ val schemaFile =
Util.daffodilPath("daffodil-cli/src/it/resources/org/apache/daffodil/CLI/trace_input.dfdl.xsd")
+ val inputFile =
Util.daffodilPath("daffodil-cli/src/it/resources/org/apache/daffodil/CLI/input/trace_input.txt")
+ val log4jConfig =
Util.daffodilPath("daffodil-cli/src/it/resources/org/apache/daffodil/CLI/conf/log4j2_traceOutput.xml")
+ val (testSchemaFile, testInputFile, testLog4jConfig) = if (Util.isWindows)
(Util.cmdConvert(schemaFile), Util.cmdConvert(inputFile),
Util.cmdConvert(log4jConfig)) else (schemaFile, inputFile, log4jConfig)
+
+ val DAFFODIL_JAVA_OPTS = Map("DAFFODIL_JAVA_OPTS" ->
("-Dlog4j.configurationFile=" + testLog4jConfig + " -Xms256m -Xmx2048m
-Dfile.encoding=UTF-8"))
Review comment:
I think if you change the log level to not trace, you probably shouldn't
need to change the log4j configuration, so this can maybe go way. I assume this
is just needed to deal with the crazy verbosity of the trace log level?
##########
File path:
daffodil-cli/src/it/resources/org/apache/daffodil/CLI/input/trace_input.txt
##########
@@ -0,0 +1 @@
+0
Review comment:
I would suggest just making the test use the `Util.echoN()` function so
that input data comes is read from stdin rather than a file. It simplifies the
test a bit, avoids a file with just a zero in it, and avoids having to update
the Rat.scala file.
##########
File path:
daffodil-runtime1/src/main/scala/org/apache/daffodil/dpath/DFDLXFunctions.scala
##########
@@ -185,7 +186,7 @@ case class DFDLXTrace(recipe: CompiledDPath, msg: String)
}
case other: DINode => other.namedQName.toString
}
- System.err.println("trace " + msg + ":" + nodeString)
+ Logger.log.trace(s"dfdlx:trace ${msg} : ${nodeString}")
Review comment:
Even though the function is called `dfdlx:trace` I'm not sure we want to
actually log the message at a the `trace()` log level. The trace log level is
incredibly verbose, and should really only ever be used when a developer is
debugging Daffodil.
Unlike the trace log level, the `dfdlx:trace` function is used when someone
wants to debug a schema, which rarely requires a deap level of daffodil debug
level logging. I would argue we want to log at either the error, warning, or
info log level. Those are normal log levels that won't overwhelm the user with
log messages. Though, I'm not sure which is more appropriate. Info feels more
correct to me, but it means the user would need to bump the verbosity to get
any output since we default to the warning log level, so that might be
intuitive. This would also simply the test a bit since you would really need
the complex log4j2 configuration.
Maybe another option would be to use an SDW instead of the logging
infrastructure. This always shows up regardless of the log level, and it can be
easily suppressed. Though, there might be issue with backtracking--I'm not sure
if backtracking also removes SDWs, which we definitely don't want
--
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]
