Skip to content

Fix always lenient calendar parsing/unparsing#1359

Merged
olabusayoT merged 1 commit intoapache:mainfrom
olabusayoT:daf-2951-activateStrictCalendarParsing
Nov 4, 2024
Merged

Fix always lenient calendar parsing/unparsing#1359
olabusayoT merged 1 commit intoapache:mainfrom
olabusayoT:daf-2951-activateStrictCalendarParsing

Conversation

@olabusayoT
Copy link
Copy Markdown
Contributor

  • we used to override the leniency regardless of what the calendar check policy was, now this ticket fixes that by setting it based on the isLenient value of calendar.
  • uncomment previously failing tests
  • make newly failing tests lax
  • add additional test

DAFFODIL-2951, DAFFODIL-1042

- we used to override the leniency regardless of what the calendar check policy was, now this ticket fixes that by setting it based on the isLenient value of calendar.
- uncomment previously failing tests
- make newly failing tests lax
- add additional test

DAFFODIL-2951, DAFFODIL-1042
Copy link
Copy Markdown
Member

@stevedlawrence stevedlawrence left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change seems reasonable, but it looks like the existing tests already failed, makes me wonder if this does already work as expected? What does change formatter.setLenient actually change?

<tdml:error/>
<tdml:error>Unable to parse</tdml:error>
<tdml:error>2001-03-35</tdml:error>
<tdml:error>valid range</tdml:error>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this tests already failed, it's just updated to include an error message. Did `calendarCheckPolicy="strict" already correctly fail to parse this in certain cases, or was the existing error something else? We had a report that this exact kind of test would successfully parse even with a strict policy.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test was commented out with DAFFODIL-1042. It used to work with Saxon, but failed with the new implementation

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see now. Thanks!

I think DAFFODIL-2951 is a duplicate then. I'll close that one.

Copy link
Copy Markdown
Member

@stevedlawrence stevedlawrence left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 👍

<tdml:error/>
<tdml:error>Unable to parse</tdml:error>
<tdml:error>2001-03-35</tdml:error>
<tdml:error>valid range</tdml:error>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see now. Thanks!

I think DAFFODIL-2951 is a duplicate then. I'll close that one.

Copy link
Copy Markdown
Contributor

@jadams-tresys jadams-tresys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@olabusayoT olabusayoT merged commit d79efe1 into apache:main Nov 4, 2024
@olabusayoT olabusayoT deleted the daf-2951-activateStrictCalendarParsing branch November 4, 2024 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants