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.

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

Date Matchers #6

Closed
wants to merge 3 commits into from
Closed

Date Matchers #6

wants to merge 3 commits into from

Conversation

stewbis
Copy link

@stewbis stewbis commented Aug 22, 2012

Hi,

I've added a suite of date matchers which add support for testing if the date is before, after, or within a time period. There are also matchers for testing if a date is the same hour, second, minute, year, etc.

I've added them to the matchers.xml and also setup unit tests, however they use the annotations so aren't invoked by your ant script but if you're happy with the code I can port these over to base off TestCase

When you get some time could you review the changes and consider them for inclusion.

Best Regards

Stewbis

@stewbis
Copy link
Author

stewbis commented Oct 3, 2012

There were some old copyright notices hanging around in the code which I've stripped out. Any chance you could check out the code and see if you'd like to pull it into the main hamcrest branch.

Thanks,
stewbis

@louisth
Copy link

louisth commented Jan 19, 2013

I don't see any provision for dealing with time zones. If you are dealing with Calendars, you need to think about time zones. IsSameDayOfMonth is great until your times are 12:30am and 6:30am and your test succeeds when run on the East Coast and fails when run on the West Coast. I've been there and retrofitting these things is not pretty.

One off-the-cuff possibility is to pass a TimeZone along as well, but you might come up with something better if you have some time to spend thinking about it.

@Proximator
Copy link

Hi,

Why don't you name your method : hasSameHour, hasSameMinute, etc (instead of isSameHour, isSameMinute,...)
For example : assertThat(date1, hasSameHour(date2)) is more readable as an english sentence than
assertThat(date1, isSameHour(date2))
What do you think ?

@stewbis
Copy link
Author

stewbis commented Jun 10, 2013

Hi Proximator,

I agree that the renamed methods would be better. The project is being developed under https://github.com/modularit/hamcrest-date and has moved on from this original pull request. I'm actively developing the hamcrest-date lib at the other location.

@stewbis stewbis closed this Jun 10, 2013
@Proximator
Copy link

Thanks for your reply.
I have an other comment about the IsSameDayOfTheMonth/IsSameDayOfTheWeek/IsSameDayOfTheYear
It is better to have IsSameDayOfMonth/IsSameDayOfWeek/IsSameDayOfYear (without the extra The) to be consistent with java calendar constants (DAY_OF_MONTH, DAY_OF_WEEK, DAY_OF_YEAR).
What do you think ?

@stewbis
Copy link
Author

stewbis commented Jun 10, 2013

Hi Proximator,

These methods have been deprecated in the latest version of the code. It has been replaced with isSameDatePart() to support any of the Calendar constants. The code is at https://github.com/modularit/hamcrest-date. The hamcrest-date library is also now on the maven central repos if you need to use it. If you'd like changes can I suggest to raise them on the https://github.com/modularit/hamcrest-date project.

Thanks

stewbis

@Proximator
Copy link

Ok, I will do it 👍

@dantheperson
Copy link

@louisth Timezones are system default unless supplied otherwise. Why would 12:30am and 6:30am US East Coast time be the same day, but 12:30am and 6:30am US West Coast time be different days?

@clintonbosch
Copy link

Hi

This pull request seems to have been closed but these doesn't seem to be any sign of the date package in hamcrest-library. Will these date matchers be part of the standard hamcrest-library distribution going forward or must we get them from the modularit repository?

Regards,
Clinton

@npryce
Copy link
Contributor

npryce commented Aug 19, 2013

It will be an add-on library. See the comment above (#6 (comment))

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.

None yet

6 participants