You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by GitBox <gi...@apache.org> on 2020/05/18 21:39:41 UTC
[GitHub] [incubator-daffodil] tuxji opened a new pull request #384: Validate PadCharacter with pattern facet
tuxji opened a new pull request #384:
URL: https://github.com/apache/incubator-daffodil/pull/384
Change Daffodil's XML schema for DFDL to validate pad properties instead
of relying solely on internal logic to throw a schema definition error
when a pad property contains a literal whitespace character. Update
internal logic tests to keep them working and add new schema validation
tests.
Validate pad characters with a pattern facet which matches only
non-empty strings with no toleration of literal whitespace characters
(NonEmptyStringLiteral in DFDL_part1_simpletypes.xsd). Fix IntelliJ
warnings about unused & duplicate attributes in schema header.
Modify TestTDMLRunner2 to create invalid TDML chunk directly in test
method and remove tdml-with-embedded-schema-errors.tdml to reduce number
of invalid TDML files. Use similar approach to add new schema
validation tests in PadCharacter.
Update internal logic pad character tests in SimpleTypes.tdml and
Entities.tdml to use element-form properties which still need to use
internal logic for validation. Note these tests may need to be removed
or updated to expect schema validation errors if DAFFODIL-2149 gets
fixed.
Add a few miscellaneous changes or comments after noticing existing
problems such as invalid DFDL entity %SP (no ;) in AT.dfdl.xsd,
TresysTests not running AT.tdml, AT.tdml and
example-of-most-dfdl-constructs.dfdl.xml throwing a SDE if
newVariableInstance is uncommented, and IntelliJ needing "public"
modifier in order to run TestTDMLFromJava successfully.
DAFFODIL-2120
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-daffodil] tuxji commented on a change in pull request #384: Validate PadCharacter with pattern facet
Posted by GitBox <gi...@apache.org>.
tuxji commented on a change in pull request #384:
URL: https://github.com/apache/incubator-daffodil/pull/384#discussion_r427611774
##########
File path: daffodil-tdml-processor/src/test/scala/org/apache/daffodil/tdml/TestTDMLRunner2.scala
##########
@@ -424,11 +424,26 @@ abc # a comment
}
@Test def testTDMLWithInvalidDFDLSchemaEmbedded() = {
+ val testSuite =
+ <tdml:testSuite suiteName="theSuiteName" xmlns:tns={ tns } xmlns:tdml={ tdml } xmlns:dfdl={ dfdl } xmlns:xs={ xsd }>
+ <!-- This embedded schema has validation errors. -->
+ <tdml:defineSchema name="mySchema">
+ <xs:include schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+ <xs:format ref="tns:GeneralFormat" />
+ <xs:element name="data" type="fn:string" dfdl:lengthKind="delimited" />
+ </tdml:defineSchema>
+ <tdml:parserTestCase name="test1" root="data" model="mySchema">
+ <tdml:document>
+ <tdml:documentPart type="text"><![CDATA[abcdef]]></tdml:documentPart>
+ </tdml:document>
+ <tdml:errors>
+ <tdml:error>Cannot resolve 'fn:string' as a QName</tdml:error>
+ </tdml:errors>
+ </tdml:parserTestCase>
+ </tdml:testSuite>
- val testDir = ""
- val aa = testDir + "tdml-with-embedded-schema-errors.tdml"
- lazy val runner = new DFDLTestSuite(Misc.getRequiredResource(aa), validateTDMLFile = false, validateDFDLSchemas = true)
- runner.runOneTest("test1")
+ lazy val ts = new DFDLTestSuite(testSuite, validateTDMLFile = false, validateDFDLSchemas = true)
Review comment:
IntelliJ didn't warn me, but when I ran sbt compile, I found out you can't add default arguments to two overloaded apply methods:
`in object Runner, multiple overloaded alternatives of method apply define default arguments`
I will work around by adding a new apply method that passes both scala.xml.Elem and validateDFDLSchema as required arguments.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-daffodil] tuxji commented on a change in pull request #384: Validate PadCharacter with pattern facet
Posted by GitBox <gi...@apache.org>.
tuxji commented on a change in pull request #384:
URL: https://github.com/apache/incubator-daffodil/pull/384#discussion_r427611774
##########
File path: daffodil-tdml-processor/src/test/scala/org/apache/daffodil/tdml/TestTDMLRunner2.scala
##########
@@ -424,11 +424,26 @@ abc # a comment
}
@Test def testTDMLWithInvalidDFDLSchemaEmbedded() = {
+ val testSuite =
+ <tdml:testSuite suiteName="theSuiteName" xmlns:tns={ tns } xmlns:tdml={ tdml } xmlns:dfdl={ dfdl } xmlns:xs={ xsd }>
+ <!-- This embedded schema has validation errors. -->
+ <tdml:defineSchema name="mySchema">
+ <xs:include schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+ <xs:format ref="tns:GeneralFormat" />
+ <xs:element name="data" type="fn:string" dfdl:lengthKind="delimited" />
+ </tdml:defineSchema>
+ <tdml:parserTestCase name="test1" root="data" model="mySchema">
+ <tdml:document>
+ <tdml:documentPart type="text"><![CDATA[abcdef]]></tdml:documentPart>
+ </tdml:document>
+ <tdml:errors>
+ <tdml:error>Cannot resolve 'fn:string' as a QName</tdml:error>
+ </tdml:errors>
+ </tdml:parserTestCase>
+ </tdml:testSuite>
- val testDir = ""
- val aa = testDir + "tdml-with-embedded-schema-errors.tdml"
- lazy val runner = new DFDLTestSuite(Misc.getRequiredResource(aa), validateTDMLFile = false, validateDFDLSchemas = true)
- runner.runOneTest("test1")
+ lazy val ts = new DFDLTestSuite(testSuite, validateTDMLFile = false, validateDFDLSchemas = true)
Review comment:
IntelliJ didn't warn me, but when I ran sbt compile, I found out you can't add default arguments to two overloaded apply methods:
`in object Runner, multiple overloaded alternatives of method apply define default arguments`
I will work around by adding a new apply method that passes both scala.xml.Elem and validateTDMLFile as required arguments.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-daffodil] stevedlawrence merged pull request #384: Validate PadCharacter with pattern facet
Posted by GitBox <gi...@apache.org>.
stevedlawrence merged pull request #384:
URL: https://github.com/apache/incubator-daffodil/pull/384
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #384: Validate PadCharacter with pattern facet
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #384:
URL: https://github.com/apache/incubator-daffodil/pull/384#discussion_r427224841
##########
File path: daffodil-core/src/test/resources/test/example-of-most-dfdl-constructs.dfdl.xml
##########
@@ -249,7 +249,8 @@
<annotation>
<appinfo source="http://www.ogf.org/dfdl/">
<dfdl:element nilKind="literalValue" />
- <!-- Gives a not yet implemented, so commented out.
+ <!-- Gives a SDE: newVariableInstance may only be used on
+ group reference, sequence or choice, so needs review (5/18/20).
Review comment:
This should probably be moved to a model group in this file. Doesn't neccesarily need to be done as part of this commit. Could be done as a new PR as an update to DAFFODIL-341 along with the other newVariableInstance changes.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-daffodil] tuxji commented on pull request #384: Validate PadCharacter with pattern facet
Posted by GitBox <gi...@apache.org>.
tuxji commented on pull request #384:
URL: https://github.com/apache/incubator-daffodil/pull/384#issuecomment-631663754
Commits squashed, checks pass, ready for merge.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-daffodil] tuxji commented on a change in pull request #384: Validate PadCharacter with pattern facet
Posted by GitBox <gi...@apache.org>.
tuxji commented on a change in pull request #384:
URL: https://github.com/apache/incubator-daffodil/pull/384#discussion_r427588178
##########
File path: daffodil-tdml-processor/src/test/scala/org/apache/daffodil/tdml/TestTDMLRunner2.scala
##########
@@ -424,11 +424,26 @@ abc # a comment
}
@Test def testTDMLWithInvalidDFDLSchemaEmbedded() = {
+ val testSuite =
+ <tdml:testSuite suiteName="theSuiteName" xmlns:tns={ tns } xmlns:tdml={ tdml } xmlns:dfdl={ dfdl } xmlns:xs={ xsd }>
+ <!-- This embedded schema has validation errors. -->
+ <tdml:defineSchema name="mySchema">
+ <xs:include schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+ <xs:format ref="tns:GeneralFormat" />
+ <xs:element name="data" type="fn:string" dfdl:lengthKind="delimited" />
+ </tdml:defineSchema>
+ <tdml:parserTestCase name="test1" root="data" model="mySchema">
+ <tdml:document>
+ <tdml:documentPart type="text"><![CDATA[abcdef]]></tdml:documentPart>
+ </tdml:document>
+ <tdml:errors>
+ <tdml:error>Cannot resolve 'fn:string' as a QName</tdml:error>
+ </tdml:errors>
+ </tdml:parserTestCase>
+ </tdml:testSuite>
- val testDir = ""
- val aa = testDir + "tdml-with-embedded-schema-errors.tdml"
- lazy val runner = new DFDLTestSuite(Misc.getRequiredResource(aa), validateTDMLFile = false, validateDFDLSchemas = true)
- runner.runOneTest("test1")
+ lazy val ts = new DFDLTestSuite(testSuite, validateTDMLFile = false, validateDFDLSchemas = true)
Review comment:
I'm passing a scala.xml.Elem as the first argument. The factory object Runner has an apply method taking a scala.xml.Elem, but it doesn't have any extra parameters like the apply method taking a dir and file. May I add these extra parameters to the scala.xml.Elem apply method so that I can call Runner instead of new DFDLTestSuite with the arguments (testSuite, validateTDMLFile = false, validateDFDLSchemas = true)?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #384: Validate PadCharacter with pattern facet
Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #384:
URL: https://github.com/apache/incubator-daffodil/pull/384#discussion_r427617734
##########
File path: daffodil-tdml-processor/src/test/scala/org/apache/daffodil/tdml/TestTDMLRunner2.scala
##########
@@ -424,11 +424,26 @@ abc # a comment
}
@Test def testTDMLWithInvalidDFDLSchemaEmbedded() = {
+ val testSuite =
+ <tdml:testSuite suiteName="theSuiteName" xmlns:tns={ tns } xmlns:tdml={ tdml } xmlns:dfdl={ dfdl } xmlns:xs={ xsd }>
+ <!-- This embedded schema has validation errors. -->
+ <tdml:defineSchema name="mySchema">
+ <xs:include schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+ <xs:format ref="tns:GeneralFormat" />
+ <xs:element name="data" type="fn:string" dfdl:lengthKind="delimited" />
+ </tdml:defineSchema>
+ <tdml:parserTestCase name="test1" root="data" model="mySchema">
+ <tdml:document>
+ <tdml:documentPart type="text"><![CDATA[abcdef]]></tdml:documentPart>
+ </tdml:document>
+ <tdml:errors>
+ <tdml:error>Cannot resolve 'fn:string' as a QName</tdml:error>
+ </tdml:errors>
+ </tdml:parserTestCase>
+ </tdml:testSuite>
- val testDir = ""
- val aa = testDir + "tdml-with-embedded-schema-errors.tdml"
- lazy val runner = new DFDLTestSuite(Misc.getRequiredResource(aa), validateTDMLFile = false, validateDFDLSchemas = true)
- runner.runOneTest("test1")
+ lazy val ts = new DFDLTestSuite(testSuite, validateTDMLFile = false, validateDFDLSchemas = true)
Review comment:
lazy val valName = ....
Is seldom needed in method code. There are a few places where we define two structures that refer to each other where we need this, but yeah, generally in method code val suffices.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #384: Validate PadCharacter with pattern facet
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #384:
URL: https://github.com/apache/incubator-daffodil/pull/384#discussion_r427413931
##########
File path: daffodil-propgen/src/main/resources/org/apache/daffodil/xsd/DFDL_part1_simpletypes.xsd
##########
@@ -17,9 +17,9 @@
-->
<xsd:schema targetNamespace="http://www.ogf.org/dfdl/dfdl-1.0/"
- xmlns:xsd="http://www.w3.org/2001/XMLSchema"
- xmlns:dfdl="http://www.ogf.org/dfdl/dfdl-1.0/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
- attributeFormDefault="unqualified" elementFormDefault="qualified">
Review comment:
I think if attributeFormDefault isn't provided it defaults to "unqualified". So this behavior should be the same. I guess it's just a matter of if we want to be explicit about it or not.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-daffodil] jadams-tresys commented on a change in pull request #384: Validate PadCharacter with pattern facet
Posted by GitBox <gi...@apache.org>.
jadams-tresys commented on a change in pull request #384:
URL: https://github.com/apache/incubator-daffodil/pull/384#discussion_r427446906
##########
File path: daffodil-test-ibm1/src/test/resources/test-suite/tresys-contributed/AT.dfdl.xsd
##########
@@ -116,8 +116,10 @@
<!-- this have BEFORE timing, so for the first local_name
$tns:counter = 1 -->
<!-- NOTE: THIS REQUIRES dfdl:newVariableInstance BECAUSE VARIABLES ARE SINGLE ASSIGNMENT IN DFDL -->
- <!-- Updating test to use newVariableInstance but the feature is not yet implemented
- so this will need to be reviewed -Beth 8/29/13 -->
+ <!-- Using newVariableInstance here gives SDE:
+ newVariableInstance may only be used on group
+ reference, sequence or choice so this will
+ need to be reviewed (5/18/20) -->
<dfdl:newVariableInstance ref="tns:counter" defaultValue="{ $tns:counter + 1 }" />
Review comment:
Agreed. I took a look at this and on top of not adding any obviously new coverage, they needed more work than just putting the newVariableInstance call in the right location. I was getting a few other SDE's for other issues too.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-daffodil] tuxji commented on a change in pull request #384: Validate PadCharacter with pattern facet
Posted by GitBox <gi...@apache.org>.
tuxji commented on a change in pull request #384:
URL: https://github.com/apache/incubator-daffodil/pull/384#discussion_r427394851
##########
File path: daffodil-propgen/src/main/resources/org/apache/daffodil/xsd/DFDL_part1_simpletypes.xsd
##########
@@ -17,9 +17,9 @@
-->
<xsd:schema targetNamespace="http://www.ogf.org/dfdl/dfdl-1.0/"
- xmlns:xsd="http://www.w3.org/2001/XMLSchema"
- xmlns:dfdl="http://www.ogf.org/dfdl/dfdl-1.0/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
- attributeFormDefault="unqualified" elementFormDefault="qualified">
Review comment:
Do you mean we need to tolerate attributeFormDefault being duplicated here two times? I deleted only the first duplicate so if we just need a single attributeFormDefault in some test to make sure it really is tolerated, we're still OK.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #384: Validate PadCharacter with pattern facet
Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #384:
URL: https://github.com/apache/incubator-daffodil/pull/384#discussion_r427360685
##########
File path: daffodil-propgen/src/main/resources/org/apache/daffodil/xsd/DFDL_part1_simpletypes.xsd
##########
@@ -17,9 +17,9 @@
-->
<xsd:schema targetNamespace="http://www.ogf.org/dfdl/dfdl-1.0/"
- xmlns:xsd="http://www.w3.org/2001/XMLSchema"
- xmlns:dfdl="http://www.ogf.org/dfdl/dfdl-1.0/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
- attributeFormDefault="unqualified" elementFormDefault="qualified">
Review comment:
We tolerate attributeFormDefault on purpose even though it is unused by DFDL. This was because some IBM tooling put it into DFDL schemas. So long as there is some other test that shows we're tolerating it elsewhere I'm fine with removing it from here.
##########
File path: daffodil-test-ibm1/src/test/resources/test-suite/tresys-contributed/AT.dfdl.xsd
##########
@@ -116,8 +116,10 @@
<!-- this have BEFORE timing, so for the first local_name
$tns:counter = 1 -->
<!-- NOTE: THIS REQUIRES dfdl:newVariableInstance BECAUSE VARIABLES ARE SINGLE ASSIGNMENT IN DFDL -->
- <!-- Updating test to use newVariableInstance but the feature is not yet implemented
- so this will need to be reviewed -Beth 8/29/13 -->
+ <!-- Using newVariableInstance here gives SDE:
+ newVariableInstance may only be used on group
+ reference, sequence or choice so this will
+ need to be reviewed (5/18/20) -->
<dfdl:newVariableInstance ref="tns:counter" defaultValue="{ $tns:counter + 1 }" />
Review comment:
This whole test and example can only be a negative test if it's trying to use a variable as an accumulator, and trying to use newVariableInstance on an element.
These tests with the 2-letter names like AT are really old (like 10 years now), and concepts in DFDL shifted out from under them.
I bet we can just drop test AT without losing any coverage.
##########
File path: daffodil-tdml-processor/src/test/scala/org/apache/daffodil/tdml/TestTDMLRunner2.scala
##########
@@ -424,11 +424,26 @@ abc # a comment
}
@Test def testTDMLWithInvalidDFDLSchemaEmbedded() = {
+ val testSuite =
+ <tdml:testSuite suiteName="theSuiteName" xmlns:tns={ tns } xmlns:tdml={ tdml } xmlns:dfdl={ dfdl } xmlns:xs={ xsd }>
+ <!-- This embedded schema has validation errors. -->
+ <tdml:defineSchema name="mySchema">
+ <xs:include schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+ <xs:format ref="tns:GeneralFormat" />
+ <xs:element name="data" type="fn:string" dfdl:lengthKind="delimited" />
+ </tdml:defineSchema>
+ <tdml:parserTestCase name="test1" root="data" model="mySchema">
+ <tdml:document>
+ <tdml:documentPart type="text"><![CDATA[abcdef]]></tdml:documentPart>
+ </tdml:document>
+ <tdml:errors>
+ <tdml:error>Cannot resolve 'fn:string' as a QName</tdml:error>
+ </tdml:errors>
+ </tdml:parserTestCase>
+ </tdml:testSuite>
- val testDir = ""
- val aa = testDir + "tdml-with-embedded-schema-errors.tdml"
- lazy val runner = new DFDLTestSuite(Misc.getRequiredResource(aa), validateTDMLFile = false, validateDFDLSchemas = true)
- runner.runOneTest("test1")
+ lazy val ts = new DFDLTestSuite(testSuite, validateTDMLFile = false, validateDFDLSchemas = true)
Review comment:
We have class Runner now with a factory object Runner. This is preferred over using DFDLTestSuite directly. We never cut everything over to Runner, hoping to do that incrementally. Eventually we'd like to make DFDLTestSuite package private so it isn't usable.
##########
File path: daffodil-tdml-processor/src/test/scala/org/apache/daffodil/tdml/TestTDMLRunner2.scala
##########
@@ -424,11 +424,26 @@ abc # a comment
}
@Test def testTDMLWithInvalidDFDLSchemaEmbedded() = {
+ val testSuite =
+ <tdml:testSuite suiteName="theSuiteName" xmlns:tns={ tns } xmlns:tdml={ tdml } xmlns:dfdl={ dfdl } xmlns:xs={ xsd }>
+ <!-- This embedded schema has validation errors. -->
+ <tdml:defineSchema name="mySchema">
+ <xs:include schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+ <xs:format ref="tns:GeneralFormat" />
+ <xs:element name="data" type="fn:string" dfdl:lengthKind="delimited" />
+ </tdml:defineSchema>
+ <tdml:parserTestCase name="test1" root="data" model="mySchema">
+ <tdml:document>
+ <tdml:documentPart type="text"><![CDATA[abcdef]]></tdml:documentPart>
+ </tdml:document>
+ <tdml:errors>
+ <tdml:error>Cannot resolve 'fn:string' as a QName</tdml:error>
Review comment:
The reason these tdml:error strings are generally so sparse is so that the tests aren't so fragile if an error message evolves slightly. E.g., this test should pass so long as the error message mentions fn:string, and mentions "resolve". Beyond that, if it says "Cannot" or "QName", or the wording changes to use double quotes around "fn:string" instead of single quotes, or changes to say "A QName is required." instead of "as a QName", we don't really care.
I would suggest this should be <tdml:error>resolve</tdml:error><tdml:error>fn:string</tdml:error> so that it is just looking for those two concepts.
##########
File path: daffodil-test-ibm1/src/test/scala/org/apache/daffodil/TresysTests.scala
##########
@@ -175,6 +175,7 @@ class TresysTests {
// @Test def test_AP000() { runnerAP.runOneTest("AP000") } // lengthKind endOfParent - DAFFODIL-567
+ // Were these tests overlooked while fixing DAFFODIL-341?
Review comment:
Yes I believe you are correct. If we have coverage elsewhere of catching multiple-assignment and I know we have tests of newVariableInstance elsewhere. Mostlikely we should just drop these tests.
##########
File path: daffodil-test/src/test/scala/org/apache/daffodil/section13/validation_errors/PadCharacter.scala
##########
@@ -0,0 +1,123 @@
+/*
+ * 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.section13.validation_errors
+
+import org.apache.daffodil.tdml.DFDLTestSuite
+import org.apache.daffodil.xml.XMLUtils
+import org.junit.Test
+
+class PadCharacter {
+
+ val tdml = XMLUtils.TDML_NAMESPACE
+ val dfdl = XMLUtils.DFDL_NAMESPACE
+ val xsd = XMLUtils.XSD_NAMESPACE
+ val tns = XMLUtils.EXAMPLE_NAMESPACE
+
+ @Test def test_short_form_pad_char() = {
+ // This test demonstrates that you cannot use a literal whitespace character
+ // in the short form of the pad character's property binding syntax
+ val testSuite =
+ <tdml:testSuite suiteName="PadCharTests" description="Section 13 - Pad Character Tests"
+ xmlns:tdml={ tdml } xmlns:dfdl={ dfdl } xmlns:xs={ xsd } xmlns:tns={ tns }>
+ <tdml:defineSchema name="padCharSchema">
+ <xs:include schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd" />
+ <dfdl:format ref="tns:GeneralFormat" textTrimKind="padChar" textPadKind="padChar" />
+ <xs:element name="shortForm" type="xs:string" dfdl:textStringPadCharacter=" " />
+ </tdml:defineSchema>
+
+ <tdml:parserTestCase name="short_form_pad_char" root="shortForm" model="padCharSchema">
+ <tdml:document>
+ <tdml:documentPart type="text"><![CDATA[ hello]]></tdml:documentPart>
+ </tdml:document>
+ <tdml:errors>
+ <tdml:error>Value ' ' is not facet-valid</tdml:error>
Review comment:
I would make this <tdml:error>facet</tdml:error><tdml:error>valid</tdml:error><tdml:error>NonEmptyStringLiteral</tdml:error> so that it's not sensitive to wording or quoting conventions.
Same idea applies to your other tests.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-daffodil] tuxji commented on a change in pull request #384: Validate PadCharacter with pattern facet
Posted by GitBox <gi...@apache.org>.
tuxji commented on a change in pull request #384:
URL: https://github.com/apache/incubator-daffodil/pull/384#discussion_r427595307
##########
File path: daffodil-tdml-processor/src/test/scala/org/apache/daffodil/tdml/TestTDMLRunner2.scala
##########
@@ -424,11 +424,26 @@ abc # a comment
}
@Test def testTDMLWithInvalidDFDLSchemaEmbedded() = {
+ val testSuite =
+ <tdml:testSuite suiteName="theSuiteName" xmlns:tns={ tns } xmlns:tdml={ tdml } xmlns:dfdl={ dfdl } xmlns:xs={ xsd }>
+ <!-- This embedded schema has validation errors. -->
+ <tdml:defineSchema name="mySchema">
+ <xs:include schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+ <xs:format ref="tns:GeneralFormat" />
+ <xs:element name="data" type="fn:string" dfdl:lengthKind="delimited" />
+ </tdml:defineSchema>
+ <tdml:parserTestCase name="test1" root="data" model="mySchema">
+ <tdml:document>
+ <tdml:documentPart type="text"><![CDATA[abcdef]]></tdml:documentPart>
+ </tdml:document>
+ <tdml:errors>
+ <tdml:error>Cannot resolve 'fn:string' as a QName</tdml:error>
+ </tdml:errors>
+ </tdml:parserTestCase>
+ </tdml:testSuite>
- val testDir = ""
- val aa = testDir + "tdml-with-embedded-schema-errors.tdml"
- lazy val runner = new DFDLTestSuite(Misc.getRequiredResource(aa), validateTDMLFile = false, validateDFDLSchemas = true)
- runner.runOneTest("test1")
+ lazy val ts = new DFDLTestSuite(testSuite, validateTDMLFile = false, validateDFDLSchemas = true)
Review comment:
Also, do we really need "lazy val ts =" in methods when the val is used immediately afterwards? I think we can replace "lazy val ts =" with "val ts =" in these methods without changing what these methods are doing.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-daffodil] jadams-tresys commented on a change in pull request #384: Validate PadCharacter with pattern facet
Posted by GitBox <gi...@apache.org>.
jadams-tresys commented on a change in pull request #384:
URL: https://github.com/apache/incubator-daffodil/pull/384#discussion_r427433086
##########
File path: daffodil-core/src/test/resources/test/example-of-most-dfdl-constructs.dfdl.xml
##########
@@ -249,7 +249,8 @@
<annotation>
<appinfo source="http://www.ogf.org/dfdl/">
<dfdl:element nilKind="literalValue" />
- <!-- Gives a not yet implemented, so commented out.
+ <!-- Gives a SDE: newVariableInstance may only be used on
+ group reference, sequence or choice, so needs review (5/18/20).
Review comment:
That is correct. The DFDL Spec specifies that newVariableInstance may only be used on group reference, sequence, or choice. The correct place for the newVariableInstance call would probably be within the "gr" group declaration on line 175
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-daffodil] stevedlawrence commented on pull request #384: Validate PadCharacter with pattern facet
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on pull request #384:
URL: https://github.com/apache/incubator-daffodil/pull/384#issuecomment-631620261
Updates look good to me. +1 to squash and merge
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-daffodil] tuxji commented on pull request #384: Validate PadCharacter with pattern facet
Posted by GitBox <gi...@apache.org>.
tuxji commented on pull request #384:
URL: https://github.com/apache/incubator-daffodil/pull/384#issuecomment-631178077
I noticed some missing Runner.reset calls and added them where necessary, which ended up affecting a number of additional files. If these changes pass all checks and look OK, I'll squash the commits together so someone can merge the pull request.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-daffodil] jadams-tresys commented on a change in pull request #384: Validate PadCharacter with pattern facet
Posted by GitBox <gi...@apache.org>.
jadams-tresys commented on a change in pull request #384:
URL: https://github.com/apache/incubator-daffodil/pull/384#discussion_r427447035
##########
File path: daffodil-test-ibm1/src/test/scala/org/apache/daffodil/TresysTests.scala
##########
@@ -175,6 +175,7 @@ class TresysTests {
// @Test def test_AP000() { runnerAP.runOneTest("AP000") } // lengthKind endOfParent - DAFFODIL-567
+ // Were these tests overlooked while fixing DAFFODIL-341?
Review comment:
Agreed.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-daffodil] tuxji commented on a change in pull request #384: Validate PadCharacter with pattern facet
Posted by GitBox <gi...@apache.org>.
tuxji commented on a change in pull request #384:
URL: https://github.com/apache/incubator-daffodil/pull/384#discussion_r427581766
##########
File path: daffodil-propgen/src/main/resources/org/apache/daffodil/xsd/DFDL_part1_simpletypes.xsd
##########
@@ -17,9 +17,9 @@
-->
<xsd:schema targetNamespace="http://www.ogf.org/dfdl/dfdl-1.0/"
- xmlns:xsd="http://www.w3.org/2001/XMLSchema"
- xmlns:dfdl="http://www.ogf.org/dfdl/dfdl-1.0/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
- attributeFormDefault="unqualified" elementFormDefault="qualified">
Review comment:
Daffodil already has a test to check that attributeFormDefault is tolerated in a DFDL schema (AttributeFormDefault.dfdl.xsd, SchemaDefinitionErrors.tdml, TestSDE.scala), However, the file I edited isn't a DFDL schema - it's Daffodil's XML schema for DFDL. So attributeFormDefault simply was being defined explicitly here, and I won't change that since several other files in this directory do the same thing too.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-daffodil] tuxji commented on a change in pull request #384: Validate PadCharacter with pattern facet
Posted by GitBox <gi...@apache.org>.
tuxji commented on a change in pull request #384:
URL: https://github.com/apache/incubator-daffodil/pull/384#discussion_r427412118
##########
File path: daffodil-propgen/src/main/resources/org/apache/daffodil/xsd/DFDL_part1_simpletypes.xsd
##########
@@ -17,9 +17,9 @@
-->
<xsd:schema targetNamespace="http://www.ogf.org/dfdl/dfdl-1.0/"
- xmlns:xsd="http://www.w3.org/2001/XMLSchema"
- xmlns:dfdl="http://www.ogf.org/dfdl/dfdl-1.0/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
- attributeFormDefault="unqualified" elementFormDefault="qualified">
Review comment:
Oops, attributeFormDefault indeed is different from elementFormDefault. IntelliJ told me something about it being unused and I must've thought it was duplicated twice. I'll check for any other test using it. If none, I'll add a comment explaining why we use it here.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #384: Validate PadCharacter with pattern facet
Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #384:
URL: https://github.com/apache/incubator-daffodil/pull/384#discussion_r427617052
##########
File path: daffodil-tdml-processor/src/test/scala/org/apache/daffodil/tdml/TestTDMLRunner2.scala
##########
@@ -424,11 +424,26 @@ abc # a comment
}
@Test def testTDMLWithInvalidDFDLSchemaEmbedded() = {
+ val testSuite =
+ <tdml:testSuite suiteName="theSuiteName" xmlns:tns={ tns } xmlns:tdml={ tdml } xmlns:dfdl={ dfdl } xmlns:xs={ xsd }>
+ <!-- This embedded schema has validation errors. -->
+ <tdml:defineSchema name="mySchema">
+ <xs:include schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd"/>
+ <xs:format ref="tns:GeneralFormat" />
+ <xs:element name="data" type="fn:string" dfdl:lengthKind="delimited" />
+ </tdml:defineSchema>
+ <tdml:parserTestCase name="test1" root="data" model="mySchema">
+ <tdml:document>
+ <tdml:documentPart type="text"><![CDATA[abcdef]]></tdml:documentPart>
+ </tdml:document>
+ <tdml:errors>
+ <tdml:error>Cannot resolve 'fn:string' as a QName</tdml:error>
+ </tdml:errors>
+ </tdml:parserTestCase>
+ </tdml:testSuite>
- val testDir = ""
- val aa = testDir + "tdml-with-embedded-schema-errors.tdml"
- lazy val runner = new DFDLTestSuite(Misc.getRequiredResource(aa), validateTDMLFile = false, validateDFDLSchemas = true)
- runner.runOneTest("test1")
+ lazy val ts = new DFDLTestSuite(testSuite, validateTDMLFile = false, validateDFDLSchemas = true)
Review comment:
Sounds right.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #384: Validate PadCharacter with pattern facet
Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #384:
URL: https://github.com/apache/incubator-daffodil/pull/384#discussion_r427447356
##########
File path: daffodil-propgen/src/main/resources/org/apache/daffodil/xsd/DFDL_part1_simpletypes.xsd
##########
@@ -17,9 +17,9 @@
-->
<xsd:schema targetNamespace="http://www.ogf.org/dfdl/dfdl-1.0/"
- xmlns:xsd="http://www.w3.org/2001/XMLSchema"
- xmlns:dfdl="http://www.ogf.org/dfdl/dfdl-1.0/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
- attributeFormDefault="unqualified" elementFormDefault="qualified">
Review comment:
Yes, it's just about whether DFDL processors complain and issue an SDE about having it. In principle they *could* do that because DFDL v1.0 doesn't use attributes at all. But... we found in practice that it's better to just ignore it being there.
----------------------------------------------------------------
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:
users@infra.apache.org