Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add more varchar to date conversion tests #13011

Merged
merged 2 commits into from
Jun 28, 2022
Merged

Add more varchar to date conversion tests #13011

merged 2 commits into from
Jun 28, 2022

Conversation

findepi
Copy link
Member

@findepi findepi commented Jun 28, 2022

No description provided.

findepi added 2 commits June 28, 2022 12:05
@findepi
Test date cast from varchar in testDateCastFromVarchar
d965393 
The method was testing DATE literals, which isn't exactly the same
thing.

The commit keeps existing test cases in `testDateLiteral` new method.
@findepi
Add more varchar to date conversion tests
908e373 
Add test cases for non-canonically formatted input, whitespace,
zero-padding, etc.
@findepi findepi added test no-release-notes This pull request does not require release notes entry labels Jun 28, 2022
@cla-bot cla-bot bot added the cla-signed label Jun 28, 2022
@findepi findepi mentioned this pull request Jun 28, 2022
Closed
hashhar
hashhar approved these changes Jun 28, 2022
View reviewed changes
Copy link
Member

@hashhar hashhar left a comment •
edited
Loading

Choose a reason for hiding this comment

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

nit: I'd prefer to keep invalid and valid cases either grouped separately or in different test methods.

non-plain whitespace tests are interesting - any specific motivation? (EDIT: I see the linked issue explains this).

@findepi
Copy link
Member Author

findepi commented Jun 28, 2022

nit: I'd prefer to keep invalid and valid cases either grouped separately or in different test methods.

They invalid and valid ones are similar.
For example, you can have June as 6 or 06, but 006 is invalid.
Or, you can have surrounding whitespace, but not intra whitespace.

I grouped them by how the input deviates from the canonical form, to highlight what's test-covered.

@findepi findepi merged commit a094360 into trinodb:master Jun 28, 2022
@findepi findepi deleted the findepi/cast-date-test branch June 28, 2022 11:34
@github-actions github-actions bot added this to the 388 milestone Jun 28, 2022
@colebow colebow mentioned this pull request Jun 28, 2022
Merged
@findepi findepi mentioned this pull request Aug 9, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hashhar hashhar hashhar approved these changes

@losipiuk losipiuk losipiuk approved these changes

@martint martint Awaiting requested review from martint

@ebyhr ebyhr Awaiting requested review from ebyhr

Assignees
No one assigned
Labels
cla-signed no-release-notes This pull request does not require release notes entry test
Milestone
388
Development

Successfully merging this pull request may close these issues.
3 participants
@findepi @losipiuk @hashhar