bear / parsedatetime

Parse human-readable date/time strings
Apache License 2.0
695 stars 106 forks source link

Bad test case in TestComplexDateTimes #141

Closed codereverser closed 9 years ago

codereverser commented 9 years ago

File : TestComplexDateTimes.py - Function : testDate3ConfusedHourAndYear

        self.assertExpectedResult(
            self.cal.parse('December 30th 23:02', start),
            (datetime.datetime(self.yr
                               if (self.mth < 12 and
                                   self.dy < 30 and
                                   self.hr < 23 and
                                   self.mn < 2)
                               else self.yr + 1,
                               12, 30, 23, 2, 0).timetuple(), 3))

The check for self.yr + 1 doesn't seem right. It always fails if self.mn > 2, irrespective of the current date. PS:- There are a couple more similar bad tests in the same function.

philiptzou commented 9 years ago

Hmm I think it's because some flag control the auto-complicated year if the date is larger/smaller than current date. So this is a small trick to get over of the side effect of it. codereverser notifications@github.com于2015年11月5日 周四19:08写道:

File : TestComplexDateTimes.py - Function : testDate3ConfusedHourAndYear

    self.assertExpectedResult(
        self.cal.parse('December 30th 23:02', start),
        (datetime.datetime(self.yr
                           if (self.mth < 12 and
                               self.dy < 30 and
                               self.hr < 23 and
                               self.mn < 2)
                           else self.yr + 1,
                           12, 30, 23, 2, 0).timetuple(), 3))

The check for self.yr + 1 doesn't seem right. It always fails if self.mn

2, irrespective of the current date. PS:- There are a couple more similar bad tests in the same function.

— Reply to this email directly or view it on GitHub https://github.com/bear/parsedatetime/issues/141.

bear commented 9 years ago

I was curious about that myself the other night. Did the switch to contexts or my merge for Python v3 break something?

codereverser commented 9 years ago

I don't think so. The tests are failing because the check to compare two dates is not correct. i.e.

(self.mth < 12 and
self.dy < 30 and
self.hr < 23 and
self.mn < 2)

always fails if say, self.dy = 31 or self.mn = 10 regardless of other values (where technically it should fail only during Dec 30 23:03 - 23:59).

I think it should've been something like

datetime.datetime(self.yr, self.mth, self.dy, self.hr, self.mn) <= datetime.datetime(self.yr, 12, 30, 23, 2)
bear commented 9 years ago

please do create a pull request with what you feel it should be so we can fix this

philiptzou commented 9 years ago

Well I get what you are saying for... Yes I made a stupid logic mistake here. Also be fixed in #142.

codereverser commented 9 years ago

Thanks!