tuxji commented on code in PR #800:
URL: https://github.com/apache/daffodil/pull/800#discussion_r892378574
##########
daffodil-tdml-processor/src/test/java/org/apache/daffodil/tdml/TestRunnerFactory.java:
##########
@@ -39,6 +40,7 @@ public void testPrimaryConstructor() {
Right<scala.xml.Elem, String> rightURI = new Right<>(tdmlUri.toString());
Runner runner = new Runner(
rightURI,
+ Option.apply(null),
Review Comment:
Yes, this is the closest Java equivalent to passing `None` (for some reason,
TestRunnerFactory is a Java source file).
##########
daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLRunner.scala:
##########
@@ -180,27 +180,6 @@ class DFDLTestSuite private[tdml] (
val shouldDoWarningComparisonOnCrossTests: Boolean)
extends HasSetDebugger {
- // Uncomment to force conversion of all test suites to use Runner(...)
instead.
- // That avoids creating the test suites repeatedly, but also leaks memory
unless
- // you have an @AfterClass shutdown method in the object that calls
runner.reset() at end.
- @deprecated("Use Runner(...) instead.", "3.2.0")
Review Comment:
I realized that removing this constructor might break existing tests so
that's why I removed the first parameter (__nl) from the first constructor to
make it a replacement for the second constructor. The only additional step we
might need is to remove the `private[tdml]` from the first constructor, but the
original comments make it clear that all test suites should use Runner(...)
instead of new DFDLTestSuite(...). I also suspect that if existing tests are
written in Java, they'll be able to call a private constructor anyway.
##########
daffodil-cli/src/main/scala/org/apache/daffodil/Main.scala:
##########
@@ -1330,7 +1345,8 @@ object Main {
case s: scala.util.control.ControlThrowable => throw s
case u: UnsuppressableException => throw u
case e: TDMLTestNotCompatibleException => {
- println("[Skipped] %s (Not compatible
implementation.)".format(name))
+ println("[Skipped] %s (not compatible with implementation
%s)"
+ .format(name, e.implementation.getOrElse("<none>")))
Review Comment:
This e.implementation is a field of TDMLException with type Option[String].
There are plenty of places which throw a TDMLImplementation with None as the
implementation, so I get the feeling this is a planned feature, not a problem
that a beginner could fix.
```scala
daffodil-tdml-processor/src/main/scala/org/apache/daffodil/tdml/processor/DaffodilTDMLDFDLProcessor.scala
347: throw TDMLException("Unexpected error during SAX Unparse:" + e,
None)
daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLRunner.scala
222: throw TDMLException(s"$envName=$value does not give a valid TDML
implementation", None)
255: case e => throw TDMLException(e, None)
343: throw TDMLException(loadingExceptions.toSeq, None)
406: throw TDMLException(s"TDML file ${tsURI} is not valid.", None)
440: case None => throw TDMLException("test " + testName + " was not
found.", None)
483: throw TDMLException("Duplicate definitions found for " + name + ":
" + dups.keys.mkString(", "), None)
701: case (None, None) => throw TDMLException("Model '" + model + "'
was not passed, found embedded in the TDML file, nor as a schema file.", None)
702: case (Some(_), Some(_)) => throw TDMLException("Model '" + model +
"' is ambiguous. There is an embedded model with that name, AND a file with
that name.", None)
728: throw TDMLException(parent.loadingExceptions.toSeq, None)
733: throw TDMLException("The " + attrName + " '" + cfgName + "' was
not found either as a embedded config, nor as a file.", None)
735: throw TDMLException("The " + attrName + " '" + cfgName + "' is
ambiguous. There is an embedded config with that name, AND a file with that
name.", None)
2413: testCase.toss(TDMLException(newExceptions, None), None)
```
--
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]
