Replace ICU-based date/time comparison in TDML Runner with XMLGregorianCalendar#1677
Replace ICU-based date/time comparison in TDML Runner with XMLGregorianCalendar#1677gdesrosiers1805 wants to merge 1 commit into
Conversation
stevedlawrence
left a comment
stevedlawrence
left a comment
There was a problem hiding this comment.
+1, just a number of really minor comments
| @Test def yearfromdatetime_01 = test | ||
| @Test def yearfromdatetime_02 = test | ||
| @Test def yearfromdatetime_03 = test | ||
| @Ignore("year zero is invalid per XSD 1.0") @Test def yearfromdatetime_03 = test |
There was a problem hiding this comment.
Instead of adding an ignore message, our convention has been to create an issue and add a comment, e.g.
// DAFFODIL-XYZ
@Ignore @Test def yearfromdatetime_03 = testWe may want to consider a convention where the message is the bug number, i.e. @Ignore("DAFFODIL-XYZ"), but I would suggest we make that change wholesale at once if we do want to do it. Probably not worth the effort for now though.
There was a problem hiding this comment.
I was actually confused as to why we were ignoreng the tests with no context, can we add the Daffodil bug comment above so poeple know?
…anCalendar The TDML date/time comparison previously used the DFDLDate/Time/DateTimeConversion classes, which create ICU objects. This broke cross-testing against the IBM DFDL cross tester, which depends on an older ICU version (e.g. it lacks Calendar.clone()). Compare xs:date/time/dateTime values by parsing into XMLGregorianCalendar and using its XSD order relation (compare()), keeping ICU off the comparison path entirely. XMLGregorianCalendar implements XSD 1.0, which does not permit year zero, so it rejects values like "0000-01-01". Two tests whose data uses year zero (yearfromdate_03 and yearfromdatetime_03) are temporarily ignored as a result. These expose a pre-existing Daffodil bug: Daffodil produces year-zero values that are invalid under XSD 1.0, which needs to be addressed separately. The tests should be re-enabled (or converted to negative tests) once that is resolved. DAFFODIL-3077
gdesrosiers1805
force-pushed
the
bugfix/daffodil-3077
branch
from
808e6dd to
767168b
Compare
olabusayoT
left a comment
olabusayoT
left a comment
There was a problem hiding this comment.
+1 Looks good, but couple comments about the contexless ignore
| @Test def yearfromdatetime_01 = test | ||
| @Test def yearfromdatetime_02 = test | ||
| @Test def yearfromdatetime_03 = test | ||
| @Ignore @Test def yearfromdatetime_03 = test |
There was a problem hiding this comment.
Can you add a comment to this ignore explaining why we are ignoring these tests
| @Test def yearfromdatetime_01 = test | ||
| @Test def yearfromdatetime_02 = test | ||
| @Test def yearfromdatetime_03 = test | ||
| @Ignore("year zero is invalid per XSD 1.0") @Test def yearfromdatetime_03 = test |
There was a problem hiding this comment.
I was actually confused as to why we were ignoreng the tests with no context, can we add the Daffodil bug comment above so poeple know?
3 participants
The TDML date/time comparison previously used the DFDLDate/Time/DateTimeConversion classes, which create ICU objects. This broke cross-testing against the IBM DFDL cross tester, which depends on an older ICU version (e.g. it lacks Calendar.clone()).
Compare xs:date/time/dateTime values by parsing into XMLGregorianCalendar and using its XSD order relation (compare()), keeping ICU off the comparison path entirely.
XMLGregorianCalendar implements XSD 1.0, which does not permit year zero, so it rejects values like "0000-01-01". Two tests whose data uses year zero (yearfromdate_03 and yearfromdatetime_03) are temporarily ignored as a result. These expose a pre-existing Daffodil bug: Daffodil produces year-zero values that are invalid under XSD 1.0, which needs to be addressed separately. The tests should be re-enabled (or converted to negative tests) once that is resolved.
DAFFODIL-3077