stevedlawrence commented on code in PR #1204:
URL: https://github.com/apache/daffodil/pull/1204#discussion_r1759049464
##########
daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLRunner.scala:
##########
@@ -1199,37 +1259,26 @@ case class ParserTestCase(ptc: NodeSeq, parentArg:
DFDLTestSuite)
VerifyTestCase.verifyParserTestData(resultXmlNode, testInfoset, implString)
(shouldValidate, expectsValidationError) match {
- case (_, true) => {
- // Note that even if shouldValidate is false, we still need to check
- // for validation diagnostics because failed assertions with
- // failureType="recoverableError" are treated as validation errors
- VerifyTestCase.verifyAllDiagnosticsFound(
- actual.getDiagnostics,
- optExpectedValidationErrors,
- implString
- ) // verify all validation errors were found
- Assert.invariant(actual.isValidationError)
- }
case (true, false) => {
Review Comment:
Do we need this case anymore? You've updated checkDiagnosicts to check
validation errors, including whether or not to ignore unexpected errors. I
think this match case goes away completely and we just use the default case?
And verifyNoValidatioNErrorsFound goes away?
##########
daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/TDMLRunner.scala:
##########
@@ -1630,26 +1700,10 @@ case class UnparserTestCase(ptc: NodeSeq, parentArg:
DFDLTestSuite)
val xmlNode = parseActual.getResult
VerifyTestCase.verifyParserTestData(xmlNode, inputInfoset, implString)
- if (
- !isCrossTest(implString.get) ||
- parent.shouldDoWarningComparisonOnCrossTests
- )
- VerifyTestCase.verifyAllDiagnosticsFound(
- actual.getDiagnostics,
- optWarnings,
- implString
- )
(shouldValidate, expectsValidationError) match {
case (_, true) => {
- // Note that even if shouldValidate is false, we still need to check
- // for validation diagnostics because failed assertions with
- // failureType="recoverableError" are treated as validation errors
- VerifyTestCase.verifyAllDiagnosticsFound(
- actual.getDiagnostics,
- optExpectedValidationErrors,
- implString
- ) // verify all validation errors were found
+ // we've already checked validationErrors with the checkDiagnostics
call above
Assert.invariant(actual.isValidationError)
}
case (true, false) => {
Review Comment:
Same comment as above? Does this match case go away?
--
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]
