stevedlawrence commented on code in PR #781:
URL: https://github.com/apache/daffodil/pull/781#discussion_r875849921
##########
project/Dependencies.scala:
##########
@@ -58,4 +58,8 @@ object Dependencies {
lazy val schematron = Seq(
"net.sf.saxon" % "Saxon-HE" % "11.2",
)
+
+ lazy val log4jcore = Seq(
Review Comment:
I'd suggest calling this `tdml_processor`. If we ever add new
`tdml_processor dependencies`, they can go in this variable.
##########
daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLRunner.scala:
##########
@@ -864,29 +876,42 @@ abstract class TestCase(testCaseXML: NodeSeq, val parent:
DFDLTestSuite) {
nBits,
optExpectedErrors,
optExpectedWarnings,
+ optExpectedLogs,
optExpectedValidationErrors,
validationMode,
roundTrip,
- implString)
+ implString,
+ parent.tdmlLogger)
}
+
+ if (parent.tdmlLogger ne null)
+ parent.tdmlLogger.clearDiagnostics()
Review Comment:
This `clearDiagnostics` thing is sort of an artifact of our logger and the
way it works (i.e. dealing with global state). It concerns me that other
logging systems might need very different abstractions that our basic appended
might not support. I wonder if there's a good way to make it so that
TDMLProcessor implementations do not need to be restricted to a specific logger
abstraction?
For example, what if creating/configuring the logging mechanism is handled
entirely within the TDMLProcessor implementation? And the TDML Runner has no
concept of that logging mechanism? Then TDML Processor can capture logs however
it wants and add them to the CompileResult/ParseResult/UnaprseResult. Then the
TDMLRunner can then access them with a new function (e.g. `getLogs`) and do the
comparisons you do. This way, the only thign the TDMLRunner needs to know is
there is a way to get logs from a result, and it's entirely up to the processor
how to best do that.
An added benefit is you don't need to pass around the tdmlLogger to a bunch
of functions, since it's all contained in the TDML Processor and the
Parse/UnparseResults, which we already pass around where needed.
##########
daffodil-tdml-processor/src/test/scala/org/apache/daffodil/tdml/TestTDMLRunnerLogs.scala:
##########
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.daffodil.tdml
+
+import org.junit.Test
+import org.junit.AfterClass
+import org.apache.daffodil.Implicits._
+import org.junit.Assert.fail
+
+import org.apache.logging.log4j.LogManager
+import org.apache.logging.log4j.core.{Logger => Log4jLogger}
+import org.apache.logging.log4j.Level
+import org.apache.logging.log4j.core.config.Configurator
+
+object TestTDMLRunnerLogs {
+ val runner = Runner("/test/tdml/", "testLogs.tdml")
+ val tdmlAppender = TDMLAppender.createAppender
+
+
if(LogManager.getRootLogger().asInstanceOf[Log4jLogger].getAppenders().get("org.apache.tdml")==null){
+ val loggers = LogManager.getRootLogger().asInstanceOf[Log4jLogger]
+ loggers.addAppender(tdmlAppender)
+ tdmlAppender.start()
+ Configurator.setLevel("org.apache.daffodil", Level.INFO)
+ }
+
+ @AfterClass def shutDown(): Unit = {
+ runner.reset
+ }
+}
+
+class TestTDMLRunnerLogs {
+ import TestTDMLRunnerLogs._
+
+ @Test def test_logWhenExpectingSuccess() = {
runner.runOneTest("logWhenExpectingSuccess", tdmlAppender) }
Review Comment:
Also, if we do as mentioned above, then we don't have to explicitly pass in
something to capture logs. It's just something that is done by default by the
TDML Processor and is always available to any TDML test.
##########
daffodil-tdml-processor/src/test/scala/org/apache/daffodil/tdml/TestTDMLRunnerLogs.scala:
##########
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.daffodil.tdml
+
+import org.junit.Test
+import org.junit.AfterClass
+import org.apache.daffodil.Implicits._
+import org.junit.Assert.fail
+
+import org.apache.logging.log4j.LogManager
+import org.apache.logging.log4j.core.{Logger => Log4jLogger}
+import org.apache.logging.log4j.Level
+import org.apache.logging.log4j.core.config.Configurator
+
+object TestTDMLRunnerLogs {
+ val runner = Runner("/test/tdml/", "testLogs.tdml")
+ val tdmlAppender = TDMLAppender.createAppender
+
+
if(LogManager.getRootLogger().asInstanceOf[Log4jLogger].getAppenders().get("org.apache.tdml")==null){
+ val loggers = LogManager.getRootLogger().asInstanceOf[Log4jLogger]
+ loggers.addAppender(tdmlAppender)
+ tdmlAppender.start()
+ Configurator.setLevel("org.apache.daffodil", Level.INFO)
+ }
Review Comment:
If we do as mentioned above, could this be moved into the TDML Processor?
Simplifies the logic in the test files a bit.
--
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]
