stevedlawrence commented on a change in pull request #430:
URL: https://github.com/apache/incubator-daffodil/pull/430#discussion_r500432342
##########
File path:
daffodil-tdml-lib/src/main/scala/org/apache/daffodil/tdml/RunnerFactory.scala
##########
@@ -21,69 +21,94 @@ import org.apache.daffodil.util.Misc
import org.apache.daffodil.exceptions.Assert
/**
- * Creates the DFDLTestSuite object lazily, so the file isn't read into memory
- * and parsed unless you actually try to run a test using it.
- *
- * Creates the DFDLTestSuite only once.
- *
- * Provides a reset method to be called from @AfterClass to drop
- * the test suite object (and avoid memory leak).
- *
- * Note: I have verified that this does get called after each test suite has
been run.
- *
- * defaultRoundTripDefault if true the round trip default for the test suite
will be
- * this value, if the test suite does not specify defaultRoundTrip attribute.
- *
- * defaultRoundTripDefaultDefault
+ * A wrapper to contain the optional parameters that can be used by the Runner
+ * class, so that they don't need to be redefined in multiple constructors,
+ * and can be easily defaulted by calling functions
*/
-object Runner {
- def apply(dir: String, file: String,
- validateTDMLFile: Boolean = true,
+
+object RunnerOpts {
+ def apply(validateTDMLFile: Boolean = true,
validateDFDLSchemas: Boolean = true,
compileAllTopLevel: Boolean = false,
- defaultRoundTripDefault: RoundTrip = defaultRoundTripDefaultDefault,
- defaultValidationDefault: String = defaultValidationDefaultDefault,
- defaultImplementationsDefault: Seq[String] =
defaultImplementationsDefaultDefault): Runner =
- new Runner(elem = null, dir, file, validateTDMLFile, validateDFDLSchemas,
compileAllTopLevel,
- defaultRoundTripDefault, defaultValidationDefault,
defaultImplementationsDefault)
-
- def apply(elem: scala.xml.Elem): Runner =
- new Runner(elem, dir = null, file = null)
-
- def apply(elem: scala.xml.Elem, validateTDMLFile: Boolean): Runner =
- new Runner(elem, dir = null, file = null, validateTDMLFile)
+ defaultRoundTripDefault: RoundTrip =
RunnerOpts.defaultRoundTripDefaultDefault,
+ defaultValidationDefault: String =
RunnerOpts.defaultValidationDefaultDefault,
+ defaultImplementationsDefault: Seq[String] =
RunnerOpts.defaultImplementationsDefaultDefault,
+ shouldDoErrorComparisonOnCrossTests: Boolean =
RunnerOpts.defaultShouldDoErrorComparisonOnCrossTests,
+ shouldDoWarningComparisonOnCrossTests: Boolean =
RunnerOpts.defaultShouldDoWarningComparisonOnCrossTests): RunnerOpts =
+ new RunnerOpts(validateTDMLFile, validateDFDLSchemas,
compileAllTopLevel,
+ defaultRoundTripDefault, defaultValidationDefault,
defaultImplementationsDefault,
+ shouldDoErrorComparisonOnCrossTests,
shouldDoWarningComparisonOnCrossTests)
- // Yes, that's a lot of defaults.....
- // but really it is 3-tiers deep:
- // roundTrip - on test case
- // defaultRoundTrip - on test suite
- // defaultRoundTripDefault - on runner aka test suite factory
- // defaultRoundTripDefaultDefault - on runner factory
- //
def defaultRoundTripDefaultDefault: RoundTrip = NoRoundTrip
- def defaultValidationDefaultDefault = "off"
+ def defaultValidationDefaultDefault: String = "off"
/**
* Default for what DFDL implementations to run tests against.
*
* A test or test suite can override this to specify more or different
implementations
* that the test should pass for.
*/
- def defaultImplementationsDefaultDefault = Seq("daffodil", "ibm")
+ def defaultImplementationsDefaultDefault: Seq[String] = Seq("daffodil",
"ibm")
/**
* By default we don't run Daffodil negative TDML tests against
cross-testers.
* The error messages are simply too varied.
*
* Negative tests must fail, but error messages aren't compared.
*/
- def defaultShouldDoErrorComparisonOnCrossTests = false
+ def defaultShouldDoErrorComparisonOnCrossTests: Boolean = false
/**
* By default we don't cross test warning messages because they are too
varied.
*/
- def defaultShouldDoWarningComparisonOnCrossTests = false
+ val defaultShouldDoWarningComparisonOnCrossTests: Boolean = false
+}
+class RunnerOpts(
+ var validateTDMLFile: Boolean = true,
+ var validateDFDLSchemas: Boolean = true,
+ var compileAllTopLevel: Boolean = false,
+ var defaultRoundTripDefault: RoundTrip,
+ var defaultValidationDefault: String,
+ var defaultImplementationsDefault: Seq[String],
+ var shouldDoErrorComparisonOnCrossTests: Boolean,
+ var shouldDoWarningComparisonOnCrossTests: Boolean) {
+ // Yes, that's a lot of defaults.....
+ // but really it is 3-tiers deep:
+ // roundTrip - on test case
+ // defaultRoundTrip - on test suite
+ // defaultRoundTripDefault - on runner aka test suite factory
+ // defaultRoundTripDefaultDefault - on runner factory
+}
+
+/**
+ * Creates the DFDLTestSuite object lazily, so the file isn't read into memory
+ * and parsed unless you actually try to run a test using it.
+ *
+ * Creates the DFDLTestSuite only once.
+ *
+ * Provides a reset method to be called from @AfterClass to drop
+ * the test suite object (and avoid memory leak).
+ *
+ * Note: I have verified that this does get called after each test suite has
been run.
+ *
+ * defaultRoundTripDefault if true the round trip default for the test suite
will be
+ * this value, if the test suite does not specify defaultRoundTrip attribute.
+ *
+ * defaultRoundTripDefaultDefault
+ */
+object Runner {
+ def apply(dir: String, file: String): Runner =
+ new Runner(dir, file, RunnerOpts())
+
+ def apply(dir: String, file: String, options: RunnerOpts): Runner =
+ new Runner(dir, file, options)
+
+ def apply(elem: scala.xml.Elem): Runner =
+ new Runner(elem, RunnerOpts())
+
+ def apply(elem: scala.xml.Elem, options: RunnerOpts): Runner =
+ new Runner(elem, options)
}
Review comment:
I'm concerned that this could break existing Runners not in the daffodil
repo. Other schema repos use this ``Runner`` for testing and they could
currently provide options which would no longer compile once this change is
made. I'm wondering if it makes sense to keep an apply the old dir/file apply
method with all options defaulted and creates the new ``RunnerOpts`` using
those values? We can deprecate that ``apply`` method so other repos will still
work but get a message to fix them?
##########
File path:
daffodil-test-ibm1/src/test/scala/org/apache/daffodil/TresysTests.scala
##########
@@ -34,69 +33,72 @@ object TresysTests {
lazy val runnerDelimited = Runner(testDir, "delimTests.tdml")
- lazy val runnerMD = Runner(testDir, "multiple-diagnostics.tdml",
compileAllTopLevel = true)
- lazy val runnerMD_NV = Runner(testDir, "multiple-diagnostics.tdml",
compileAllTopLevel = true, validateDFDLSchemas = false)
+ lazy val runnerMD = Runner(testDir, "multiple-diagnostics.tdml",
RunnerOpts(compileAllTopLevel = true))
+ lazy val runnerMD_NV = Runner(testDir, "multiple-diagnostics.tdml",
RunnerOpts(compileAllTopLevel = true, validateDFDLSchemas = false))
- val bb = testDir + "BB.tdml"
- lazy val runnerBB = new DFDLTestSuite(Misc.getRequiredResource(bb))
- val be = testDir + "BE.tdml"
- lazy val runnerBE = new DFDLTestSuite(Misc.getRequiredResource(be))
- val bf = testDir + "BF.tdml"
- lazy val runnerBF1 = new DFDLTestSuite(Misc.getRequiredResource(bf))
- val bg = testDir + "BG.tdml"
- lazy val runnerBG = new DFDLTestSuite(Misc.getRequiredResource(bg))
- val mb = testDir + "mixed-binary-text.tdml"
- lazy val runnerMB = new DFDLTestSuite(Misc.getRequiredResource(mb))
-
- val ap = testDir + "AP.tdml"
- lazy val runnerAP = new DFDLTestSuite(Misc.getRequiredResource(ap))
- val ax = testDir + "AX.tdml"
- lazy val runnerAX = new DFDLTestSuite(Misc.getRequiredResource(ax))
- val av0 = testDir + "AV000.tdml"
- lazy val runnerAV000 = new DFDLTestSuite(Misc.getRequiredResource(av0))
- val av1 = testDir + "AV001.tdml"
- lazy val runnerAV001 = new DFDLTestSuite(Misc.getRequiredResource(av1))
- val av2 = testDir + "AV002.tdml"
- lazy val runnerAV002 = new DFDLTestSuite(Misc.getRequiredResource(av2))
- val av3 = testDir + "AV003.tdml"
- lazy val runnerAV003 = new DFDLTestSuite(Misc.getRequiredResource(av3))
- val nsd = testDir + "nested-separator-delimited.tdml"
- lazy val runnerNSD = new DFDLTestSuite(Misc.getRequiredResource(nsd))
-
-
- lazy val runnerRD = Runner(testDir, "runtime-diagnostics.tdml",
compileAllTopLevel = true, validateTDMLFile = false)
-
- val sq = testDir + "sequence.tdml"
- lazy val runnerSQ = new DFDLTestSuite(Misc.getRequiredResource(sq))
-
- lazy val runnerNG = new DFDLTestSuite(Misc.getRequiredResource(testDir +
"nested_group_ref.tdml"))
- val af = testDir + "AF.tdml"
- lazy val runnerAF = new DFDLTestSuite(Misc.getRequiredResource(af))
- val ag = testDir + "AG.tdml"
- lazy val runnerAG = new DFDLTestSuite(Misc.getRequiredResource(ag))
-
- val aw = testDir + "AW.tdml"
- lazy val runnerAW = new DFDLTestSuite(Misc.getRequiredResource(aw))
-
- val ay = testDir + "AY.tdml"
- lazy val runnerAY = new DFDLTestSuite(Misc.getRequiredResource(ay))
-
- val az = testDir + "AZ.tdml"
- lazy val runnerAZ = new DFDLTestSuite(Misc.getRequiredResource(az))
-
- val ba = testDir + "BA.tdml"
- lazy val runnerBA = new DFDLTestSuite(Misc.getRequiredResource(ba))
-
- val bc = testDir + "BC.tdml"
- lazy val runnerBC = new DFDLTestSuite(Misc.getRequiredResource(bc))
- val bd = testDir + "BD.tdml"
- lazy val runnerBD = new DFDLTestSuite(Misc.getRequiredResource(bd))
+ lazy val runnerBB = Runner(testDir, "BB.tdml")
+ lazy val runnerBE = Runner(testDir, "BE.tdml")
+ lazy val runnerBF1 = Runner(testDir, "BF.tdml")
+ lazy val runnerBG = Runner(testDir, "BG.tdml")
+ lazy val runnerMB = Runner(testDir, "mixed-binary-text.tdml")
+
+ lazy val runnerAP = Runner(testDir, "AP.tdml")
+ lazy val runnerAX = Runner(testDir, "AX.tdml")
+ lazy val runnerAV000 = Runner(testDir, "AV000.tdml")
+ lazy val runnerAV001 = Runner(testDir, "AV001.tdml")
+ lazy val runnerAV002 = Runner(testDir, "AV002.tdml")
+ lazy val runnerAV003 = Runner(testDir, "AV003.tdml")
+ lazy val runnerNSD = Runner(testDir, "nested-separator-delimited.tdml")
+
+ lazy val runnerRD = Runner(testDir, "runtime-diagnostics.tdml",
RunnerOpts(compileAllTopLevel = true, validateTDMLFile = false))
+
+ lazy val runnerSQ = Runner(testDir, "sequence.tdml")
+
+ lazy val runnerNG = Runner(testDir, "nested_group_ref.tdml")
+
+ lazy val runnerAF = Runner(testDir, "AF.tdml")
+
+ lazy val runnerAG = Runner(testDir, "AG.tdml")
+
+ lazy val runnerAW = Runner(testDir, "AW.tdml")
+
+ lazy val runnerAY = Runner(testDir, "AY.tdml")
+
+ lazy val runnerAZ = Runner(testDir, "AZ.tdml")
+
+ lazy val runnerBA = Runner(testDir, "BA.tdml")
+
+ lazy val runnerBC = Runner(testDir, "BC.tdml")
+
+ lazy val runnerBD = Runner(testDir, "BD.tdml")
@AfterClass def shutDown: Unit = {
runnerDelimited.reset
runnerMD.reset
runnerMD_NV.reset
+ runnerBB.reset
Review comment:
Since we are modifying so many test files do we also want to add the
``shutDown`` method to reset the runners? I think many of them are missing.
I'm wondering if a convenient way to do this would be to add a new mixin
(i.e. DaffodilTestSuite) that implements shutDown and uses reflection to find
any Runner members and automatically call reset on them?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
