You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by "stevedlawrence (via GitHub)" <gi...@apache.org> on 2024/12/10 16:00:43 UTC
[PR] Add new daffodil-tdml-junit library [daffodil]
stevedlawrence opened a new pull request, #1389:
URL: https://github.com/apache/daffodil/pull/1389
There is currently a large amount of boilerplate and duplication associated with defining TDML tests with JUnit. To fix this, this adds two new traits that can be mixed in to the companion object and class associated with a JUnit test suite to greatly simplify test suite definitions. Test suites can now be defined with something like this:
object MyTests extends TdmlSuite {
val tdmlResource = "/resource/path/to/tests.tdml"
}
class MyTests extends TdmlTests {
val tdmlSuite = MyTests
@Test def test1 = test
@Test def test2 = test
@Test def test3 = test
}
To allow use with older versions of Daffodil, this defaults to using creating a Runner with only the path parameter. This is sufficient for all known schema projects, since most Runner parameters can either be set directly in a TDML file (which is preferred), or are intended for use by Daffodil tests and should not be changed by schema projects.
The sbt-plugin project will eventually be updated to include this as a dependency for DFDL Schema projects, but until that change is released, projects that want to use this new syntax should add the following to dependency to build.sbt file, in addition to any existing daffodil dependencies:
libraryDependencies ++= Seq(
"org.apache.daffodil" %% "daffodil-tdml-junit" % "<version>" % "test" intransitive()
)
Where "<version>" is hardcoded to the first version that daffodil-tdml-junit is released for. The new library works with older versions of Daffodil, so by hardcoding the version and setting it as an intransitive dependency, it allows the library the be used with TDML tests even when testing older daffodil versions that do not publish the library. As we no longer need to support older versions of Daffodil and sbt-daffodil supports the new library, this will not be needed.
DAFFODIL-2958
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new daffodil-tdml-junit library [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence merged PR #1389:
URL: https://github.com/apache/daffodil/pull/1389
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new daffodil-tdml-junit library [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on PR #1389:
URL: https://github.com/apache/daffodil/pull/1389#issuecomment-2532793222
In fact, implementing it in Java might clean things up more by removing the need for the companion object. I think the only thing that must be static is the `@AfterClass` function and a Java abstract class could implement that and Scala will implicitly create the companion object.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new daffodil-tdml-junit library [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1389:
URL: https://github.com/apache/daffodil/pull/1389#discussion_r1890771393
##########
daffodil-tdml-junit/src/main/scala/org/apache/daffodil/junit/tdml/TdmlSuite.scala:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.junit.tdml
+
+import java.io.File
+
+import org.apache.daffodil.tdml.Runner
+import org.apache.daffodil.tdml.TDMLTestNotCompatibleException
+
+import org.junit.AfterClass
+import org.junit.AssumptionViolatedException
+import org.junit.Rule
+import org.junit.rules.TestName
+
+/**
+ * Mixin for a JUnit test suite companion object
+ */
+trait TdmlSuite {
+
+ /**
+ * Resource path to the TDML file to use for this JUnit suite
+ */
+ val tdmlResource: String
+
+ /**
+ * Function to get the directory containing tdmlResource. Useful if createRunner is overridden
+ * to use the Runner(dir, file, ...) factory method
+ */
+ def tdmlDir: String = new File(tdmlResource).getParent()
+
+ /**
+ * Function to get basename of tdmlResource. Useful if createRunner is overridden to use the
+ * Runner(dir, file, ...) factory method
+ */
+ def tdmlFile: String = new File(tdmlResource).getName()
+
+ /**
+ * Default implementation to create a Runner using the tdmlResource. This function can be
+ * overridden if additional options need to be passed to the Runner factory object, for
+ * example to disable TDML validation. In most cases this should not be needed, since most
+ * parameters should not be changed by normal schema tests, or can be defined in the TDML
+ * file.
+ */
+ def createRunner(): Runner = Runner(tdmlResource)
+
+ /**
+ * Lazily build the runner when needed
+ */
+ final lazy val runner = createRunner()
+
+ /**
+ * Ensure all resources associated with the Runner (e.g. cached compiled schemas, parsed TDML
+ * XML) to be freed once the test suite has completed
+ */
+ @AfterClass
+ final def shutDown(): Unit = runner.reset
+}
+
+/**
+ * Mixin for a JUnit test suite companion class
+ */
+trait TdmlTests {
Review Comment:
So far knownToFail is pretty straightforward to implement. I did find an another drackback with it--you can't use it with older versions of the TDML runner since it errors if it sees an attribute it doesn't understand. So if we do add support for this you can't use it if you want to maintain compatibility with older daffodil versions.
This is probably okay? We can still use it internally for Daffodil tests and most DFDL schema projects can probably either be update to the latest version of Daffodil, or if they do need to maintain compatibility with older versions they can just stick to commenting out tests (or using `@Ignore`).
Though, my biggest concern is still that testing Daffodil with this change shows 100+ red "assumption violated" messages, making it look like a lot of things are broken, even though we have 4000+ working tests. I imagine that noise will also make it hard to more difficult to see actual errors among the sea of red messages.
Makes me consider that using @Ignore or just keeping with commenting out is the best method.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new daffodil-tdml-junit library [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on PR #1389:
URL: https://github.com/apache/daffodil/pull/1389#issuecomment-2532785004
I have almost finished converting tests to the new API, and the only issue I've found so far is that it can't be used in Java. Scala adds an abstract equality function that cannot be implemented in a Java class due to the mangled names Scala uses.
We might be able to support Java if TdmlSuite and TdmlTests were written in Java--is that something worth supporting?
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new daffodil-tdml-junit library [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on PR #1389:
URL: https://github.com/apache/daffodil/pull/1389#issuecomment-2554839341
I've rebased to the latest master, and added a second commit that converts all of our TDML tests to use the new format. It's big change but makes the tests much less verbose. It's getting difficult to deal with merge conflicts each time a new commit adds tests, so if this second commit could get two +1's I would prefer if this be merged before anything else.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new daffodil-tdml-junit library [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on PR #1389:
URL: https://github.com/apache/daffodil/pull/1389#issuecomment-2532870457
Actually, Java limitations of static methods makes it pretty difficult to make a reasonable API for this. I think we'll just have to stick with Scala-only support for now.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new daffodil-tdml-junit library [daffodil]
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #1389:
URL: https://github.com/apache/daffodil/pull/1389#discussion_r1892360357
##########
daffodil-tdml-junit/src/main/scala/org/apache/daffodil/junit/tdml/TdmlSuite.scala:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.junit.tdml
+
+import java.io.File
+
+import org.apache.daffodil.tdml.Runner
+import org.apache.daffodil.tdml.TDMLTestNotCompatibleException
+
+import org.junit.AfterClass
+import org.junit.AssumptionViolatedException
+import org.junit.Rule
+import org.junit.rules.TestName
+
+/**
+ * Mixin for a JUnit test suite companion object
+ */
+trait TdmlSuite {
+
+ /**
+ * Resource path to the TDML file to use for this JUnit suite
+ */
+ val tdmlResource: String
+
+ /**
+ * Function to get the directory containing tdmlResource. Useful if createRunner is overridden
+ * to use the Runner(dir, file, ...) factory method
+ */
+ def tdmlDir: String = new File(tdmlResource).getParent()
+
+ /**
+ * Function to get basename of tdmlResource. Useful if createRunner is overridden to use the
+ * Runner(dir, file, ...) factory method
+ */
+ def tdmlFile: String = new File(tdmlResource).getName()
+
+ /**
+ * Default implementation to create a Runner using the tdmlResource. This function can be
+ * overridden if additional options need to be passed to the Runner factory object, for
+ * example to disable TDML validation. In most cases this should not be needed, since most
+ * parameters should not be changed by normal schema tests, or can be defined in the TDML
+ * file.
+ */
+ def createRunner(): Runner = Runner(tdmlResource)
+
+ /**
+ * Lazily build the runner when needed
+ */
+ final lazy val runner = createRunner()
+
+ /**
+ * Ensure all resources associated with the Runner (e.g. cached compiled schemas, parsed TDML
+ * XML) to be freed once the test suite has completed
+ */
+ @AfterClass
+ final def shutDown(): Unit = runner.reset
+}
+
+/**
+ * Mixin for a JUnit test suite companion class
+ */
+trait TdmlTests {
Review Comment:
I vote for the '@ignore' annnotation.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new daffodil-tdml-junit library [daffodil]
Posted by "mbeckerle (via GitHub)" <gi...@apache.org>.
mbeckerle commented on code in PR #1389:
URL: https://github.com/apache/daffodil/pull/1389#discussion_r1890386221
##########
daffodil-tdml-junit/src/main/scala/org/apache/daffodil/junit/tdml/TdmlSuite.scala:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.junit.tdml
+
+import java.io.File
+
+import org.apache.daffodil.tdml.Runner
+import org.apache.daffodil.tdml.TDMLTestNotCompatibleException
+
+import org.junit.AfterClass
+import org.junit.AssumptionViolatedException
+import org.junit.Rule
+import org.junit.rules.TestName
+
+/**
+ * Mixin for a JUnit test suite companion object
+ */
+trait TdmlSuite {
+
+ /**
+ * Resource path to the TDML file to use for this JUnit suite
+ */
+ val tdmlResource: String
+
+ /**
+ * Function to get the directory containing tdmlResource. Useful if createRunner is overridden
+ * to use the Runner(dir, file, ...) factory method
+ */
+ def tdmlDir: String = new File(tdmlResource).getParent()
+
+ /**
+ * Function to get basename of tdmlResource. Useful if createRunner is overridden to use the
+ * Runner(dir, file, ...) factory method
+ */
+ def tdmlFile: String = new File(tdmlResource).getName()
+
+ /**
+ * Default implementation to create a Runner using the tdmlResource. This function can be
+ * overridden if additional options need to be passed to the Runner factory object, for
+ * example to disable TDML validation. In most cases this should not be needed, since most
+ * parameters should not be changed by normal schema tests, or can be defined in the TDML
+ * file.
+ */
+ def createRunner(): Runner = Runner(tdmlResource)
+
+ /**
+ * Lazily build the runner when needed
+ */
+ final lazy val runner = createRunner()
+
+ /**
+ * Ensure all resources associated with the Runner (e.g. cached compiled schemas, parsed TDML
+ * XML) to be freed once the test suite has completed
+ */
+ @AfterClass
+ final def shutDown(): Unit = runner.reset
+}
+
+/**
+ * Mixin for a JUnit test suite companion class
+ */
+trait TdmlTests {
Review Comment:
I really like the idea of adding `knownToFail='DAFFODIL-123` to the TDML files and being able to just generate the scala driver files which then become something of an IDE-support tool to enable running individual tests conveniently from an IDE.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new daffodil-tdml-junit library [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on PR #1389:
URL: https://github.com/apache/daffodil/pull/1389#issuecomment-2532174794
Note, I've converted about 75% of existing TDML tests in Daffodil to use this so I'm pretty confident this API covers our needs. All the DFDL Schema projects are much simplier that Daffodil tests, so I'm pretty confident that if we can update all Daffodil TDML test to use this, updating DFDL Schema projects will be straightforward.
Once this API is deemed suitable and is merged I'll create a separate PR that updates everything to use the new API.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add new daffodil-tdml-junit library [daffodil]
Posted by "stevedlawrence (via GitHub)" <gi...@apache.org>.
stevedlawrence commented on code in PR #1389:
URL: https://github.com/apache/daffodil/pull/1389#discussion_r1880410848
##########
daffodil-tdml-junit/src/main/scala/org/apache/daffodil/junit/tdml/TdmlSuite.scala:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.junit.tdml
+
+import java.io.File
+
+import org.apache.daffodil.tdml.Runner
+import org.apache.daffodil.tdml.TDMLTestNotCompatibleException
+
+import org.junit.AfterClass
+import org.junit.AssumptionViolatedException
+import org.junit.Rule
+import org.junit.rules.TestName
+
+/**
+ * Mixin for a JUnit test suite companion object
+ */
+trait TdmlSuite {
+
+ /**
+ * Resource path to the TDML file to use for this JUnit suite
+ */
+ val tdmlResource: String
+
+ /**
+ * Function to get the directory containing tdmlResource. Useful if createRunner is overridden
+ * to use the Runner(dir, file, ...) factory method
+ */
+ def tdmlDir: String = new File(tdmlResource).getParent()
+
+ /**
+ * Function to get basename of tdmlResource. Useful if createRunner is overridden to use the
+ * Runner(dir, file, ...) factory method
+ */
+ def tdmlFile: String = new File(tdmlResource).getName()
+
+ /**
+ * Default implementation to create a Runner using the tdmlResource. This function can be
+ * overridden if additional options need to be passed to the Runner factory object, for
+ * example to disable TDML validation. In most cases this should not be needed, since most
+ * parameters should not be changed by normal schema tests, or can be defined in the TDML
+ * file.
+ */
+ def createRunner(): Runner = Runner(tdmlResource)
+
+ /**
+ * Lazily build the runner when needed
+ */
+ final lazy val runner = createRunner()
+
+ /**
+ * Ensure all resources associated with the Runner (e.g. cached compiled schemas, parsed TDML
+ * XML) to be freed once the test suite has completed
+ */
+ @AfterClass
+ final def shutDown(): Unit = runner.reset
+}
+
+/**
+ * Mixin for a JUnit test suite companion class
+ */
+trait TdmlTests {
Review Comment:
While I am making modifications to all our test files, I wonder if it's worth considering how we identify tests as broken?
Currently we just comment them out, e.g.
```scala
//@Test def foo = test
```
One alternative to this might be to use the `@Ignore` annotation, and optionally provide a bug number in the description, e.g.
```scala
@Ignore("DAFFODIL-1234") @Test def foo = test
```
Benefits:
1. It ensures the "foo" function is valid and isn't duplicated or something.
2. When running `sbt test`, it adds a line for each ignored test and at the end of testing it shows the total count of ignored tests. So there's a metric of how many tests are failing. However, this does make the test output a bit more verbose and if you're not aware of why a test might be ignored it might be confusing to see a bunch of ignored tests.
3. There is a clear place for bug number information, a convention that we could enforce
Here's what it ignored tests look like in SBT:
Another approach is to add a new "knownToFail" method, which could also take an optional bug number, e.g.
```scala
@Test def foo = knownToFail("DAFFODIL-123")
```
This has similar benefits to `@Ignore`, but the `knownToFail` method could make sure the test actually exists in a TDML file, so there aren't concerns of accidentally moving/deleting a failing test. A downside of this approach is the new function would need to cause an assumption failure to prevent it from actually causing a failure, and in SBT these assumption failures look kindof fatal even though they really aren't. So this might be confusing to new users to see a bunch of red "assumption failed" messages when running tests. We could make it so the error message would include something like "known to fail" but it still could be a bit intimidating.
For reference, here's what this looks like in SBT:
Note that `@Ignore`'d tests show up in stats as "ignored", but "assumption failed" show up as skipped.
Another option, similar to the above, is to add a new attribute to TDML test cases, e.g.
```xml
<tdml:parserTestCase ... knownToFail="DAFFODIL-123">
```
If a test were run with this attribute set it would throw an exception that would be turned into an assumption failure. So it gives the same result as the "knownToFail" method approach, but the Scala files for a failed test are exactly the same as for passing tests. This makes it really easy to generate test scala files from a TDML file since there are no differences between working or known-to-fail test. But otherwise this is the same as the knownToFail method, including the same drawbacks about verbosity. It's just now controlled by the TDML file and the TDML Runner instead of the scala file.
Or, we just keep it as is. We've gone this far by just commenting out tests and that's been sufficient. Just thought this was a good time to consider alternatives since I'm already changing all these files and it potentially affects the TdmlTests API.
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org