From 39dc254bf4d5be10dcaba6561b569e21298896a4 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 29 Nov 2017 02:22:16 +0100 Subject: [PATCH 01/13] Don't call time() from wxLogRecordInfo ctor This is a tiny optimization (or maybe not so tiny on platforms other than Linux where time() might not as fast as just reading a memory location), but mostly is done to work around faketime bug[*] which prevented it from being used for testing programs using wxWidgets, such as our own unit tests because time() was called from wxLogTrace() in wxCSConv::DoCreate() called when creating global conversion objects during the library initialization. Arguably, it might be better to avoid calling wxLogTrace() during the initialization, but this can't be done as simply and this change might have a small performance benefit too. [*] https://github.com/wolfcw/libfaketime/issues/132 --- include/wx/log.h | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/include/wx/log.h b/include/wx/log.h index d6945eb0d2..3d7d273cbe 100644 --- a/include/wx/log.h +++ b/include/wx/log.h @@ -158,7 +158,11 @@ public: line = line_; component = component_; - timestamp = time(NULL); + // don't initialize the timestamp yet, we might not need it at all if + // the message doesn't end up being logged and otherwise we'll fill it + // just before logging it, which won't change it by much and definitely + // less than a second resolution of the timestamp + timestamp = 0; #if wxUSE_THREADS threadId = wxThread::GetCurrentId(); @@ -1162,6 +1166,11 @@ private: void DoCallOnLog(wxLogLevel level, const wxString& format, va_list argptr) { + // As explained in wxLogRecordInfo ctor, we don't initialize its + // timestamp to avoid calling time() unnecessary, but now that we are + // about to log the message, we do need to do it. + m_info.timestamp = time(NULL); + wxLog::OnLog(level, wxString::FormatV(format, argptr), m_info); } From f13b7c6a55bd529062fdb748efc016259de6b94b Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 29 Nov 2017 22:02:16 +0100 Subject: [PATCH 02/13] Extract common code in a new wxTryGetTm() helper function Replace exactly the same code occurring in both wxDateTime::GetTm() and Format() with a single version in a new wxTryGetTm() function. No real changes yet, except for removing of some useless asserts. --- src/common/datetime.cpp | 52 +++++++++++++++----------------------- src/common/datetimefmt.cpp | 36 +++----------------------- 2 files changed, 23 insertions(+), 65 deletions(-) diff --git a/src/common/datetime.cpp b/src/common/datetime.cpp index 525404ed0c..1fda366782 100644 --- a/src/common/datetime.cpp +++ b/src/common/datetime.cpp @@ -1430,6 +1430,24 @@ unsigned long wxDateTime::GetAsDOS() const // time_t <-> broken down time conversions // ---------------------------------------------------------------------------- +const tm* wxTryGetTm(tm& tmstruct, time_t t, const wxDateTime::TimeZone& tz) +{ + if ( tz.GetOffset() == -wxGetTimeZone() ) + { + // we are working with local time + return wxLocaltime_r(&t, &tmstruct); + } + else + { + t += (time_t)tz.GetOffset(); +#if !defined(__VMS__) // time is unsigned so avoid warning + if ( t < 0 ) + return NULL; +#endif + return wxGmtime_r(&t, &tmstruct); + } +} + wxDateTime::Tm wxDateTime::GetTm(const TimeZone& tz) const { wxASSERT_MSG( IsValid(), wxT("invalid wxDateTime") ); @@ -1437,39 +1455,9 @@ wxDateTime::Tm wxDateTime::GetTm(const TimeZone& tz) const time_t time = GetTicks(); if ( time != (time_t)-1 ) { - // use C RTL functions + // Try to use the RTL. struct tm tmstruct; - tm *tm; - if ( tz.GetOffset() == -wxGetTimeZone() ) - { - // we are working with local time - tm = wxLocaltime_r(&time, &tmstruct); - - // should never happen - wxCHECK_MSG( tm, Tm(), wxT("wxLocaltime_r() failed") ); - } - else - { - time += (time_t)tz.GetOffset(); -#if defined(__VMS__) // time is unsigned so avoid warning - int time2 = (int) time; - if ( time2 >= 0 ) -#else - if ( time >= 0 ) -#endif - { - tm = wxGmtime_r(&time, &tmstruct); - - // should never happen - wxCHECK_MSG( tm, Tm(), wxT("wxGmtime_r() failed") ); - } - else - { - tm = (struct tm *)NULL; - } - } - - if ( tm ) + if ( const tm* tm = wxTryGetTm(tmstruct, time, tz) ) { // adjust the milliseconds Tm tm2(*tm, tz); diff --git a/src/common/datetimefmt.cpp b/src/common/datetimefmt.cpp index 908f5f7efd..721a149da0 100644 --- a/src/common/datetimefmt.cpp +++ b/src/common/datetimefmt.cpp @@ -65,6 +65,7 @@ // ---------------------------------------------------------------------------- extern void wxInitTm(struct tm& tm); +extern const tm* wxTryGetTm(tm& tmstruct, time_t t, const wxDateTime::TimeZone& tz); extern wxString wxCallStrftime(const wxString& format, const tm* tm); @@ -367,40 +368,9 @@ wxString wxDateTime::Format(const wxString& formatp, const TimeZone& tz) const if ( canUseStrftime ) { - // use strftime() + // Try using strftime() struct tm tmstruct; - struct tm *tm; - if ( tz.GetOffset() == -wxGetTimeZone() ) - { - // we are working with local time - tm = wxLocaltime_r(&time, &tmstruct); - - // should never happen - wxCHECK_MSG( tm, wxEmptyString, wxT("wxLocaltime_r() failed") ); - } - else - { - time += (int)tz.GetOffset(); - -#if defined(__VMS__) // time is unsigned so avoid warning - int time2 = (int) time; - if ( time2 >= 0 ) -#else - if ( time >= 0 ) -#endif - { - tm = wxGmtime_r(&time, &tmstruct); - - // should never happen - wxCHECK_MSG( tm, wxEmptyString, wxT("wxGmtime_r() failed") ); - } - else - { - tm = (struct tm *)NULL; - } - } - - if ( tm ) + if ( const tm* tm = wxTryGetTm(tmstruct, time, tz) ) { return wxCallStrftime(format, tm); } From 543c522cb8ee7b46ae68af914bfa7caabcd06636 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 29 Nov 2017 22:58:06 +0100 Subject: [PATCH 03/13] Explicitly disambiguate local time zone from UTC Don't rely on time zone offset to check whether it is local as this doesn't, and can't, work for the local time zone in Great Britain which uses the same offset as UTC, but does use DST, unlike the latter. Add a unit test (albeit disabled by default) checking that the code that previously didn't work correctly in BST does work now (run the tests using "TZ=Europe/London ./test wxDateTime-BST-bugs" under Unix to test). Closes #14317, #17220. See #10445. --- include/wx/datetime.h | 4 +++- interface/wx/datetime.h | 11 +++++++++++ src/common/datetime.cpp | 14 ++++++++++---- tests/datetime/datetimetest.cpp | 25 ++++++++++++++++++++++++- 4 files changed, 48 insertions(+), 6 deletions(-) diff --git a/include/wx/datetime.h b/include/wx/datetime.h index e6b271c60c..8ed8dfcbda 100644 --- a/include/wx/datetime.h +++ b/include/wx/datetime.h @@ -306,7 +306,9 @@ public: return tz; } - long GetOffset() const { return m_offset; } + bool IsLocal() const { return m_offset == -1; } + + long GetOffset() const; private: // offset for this timezone from GMT in seconds diff --git a/interface/wx/datetime.h b/interface/wx/datetime.h index 0f9eb55966..289ac1da48 100644 --- a/interface/wx/datetime.h +++ b/interface/wx/datetime.h @@ -254,6 +254,17 @@ public: /// Create a time zone with the given offset in seconds. static TimeZone Make(long offset); + /** + Return true if this is the local time zone. + + This method can be useful for distinguishing between UTC time zone + and local time zone in Great Britain, which use the same offset as + UTC (i.e. 0), but do use DST. + + @since 3.1.1 + */ + bool IsLocal() const; + /// Return the offset of this time zone from UTC, in seconds. long GetOffset() const; }; diff --git a/src/common/datetime.cpp b/src/common/datetime.cpp index 1fda366782..5b6013b8a8 100644 --- a/src/common/datetime.cpp +++ b/src/common/datetime.cpp @@ -456,9 +456,8 @@ wxDateTime::TimeZone::TimeZone(wxDateTime::TZ tz) switch ( tz ) { case wxDateTime::Local: - // get the offset from C RTL: it returns the difference GMT-local - // while we want to have the offset _from_ GMT, hence the '-' - m_offset = -wxGetTimeZone(); + // Use a special value for local time zone. + m_offset = -1; break; case wxDateTime::GMT_12: @@ -503,6 +502,13 @@ wxDateTime::TimeZone::TimeZone(wxDateTime::TZ tz) } } +long wxDateTime::TimeZone::GetOffset() const +{ + // get the offset from C RTL: it returns the difference GMT-local + // while we want to have the offset _from_ GMT, hence the '-' + return m_offset == -1 ? -wxGetTimeZone() : m_offset; +} + // ---------------------------------------------------------------------------- // static functions // ---------------------------------------------------------------------------- @@ -1432,7 +1438,7 @@ unsigned long wxDateTime::GetAsDOS() const const tm* wxTryGetTm(tm& tmstruct, time_t t, const wxDateTime::TimeZone& tz) { - if ( tz.GetOffset() == -wxGetTimeZone() ) + if ( tz.IsLocal() ) { // we are working with local time return wxLocaltime_r(&t, &tmstruct); diff --git a/tests/datetime/datetimetest.cpp b/tests/datetime/datetimetest.cpp index fdcd9f70cb..0ae4321900 100644 --- a/tests/datetime/datetimetest.cpp +++ b/tests/datetime/datetimetest.cpp @@ -683,7 +683,7 @@ void DateTimeTestCase::TestTimeFormat() const long timeZonesOffsets[] = { - wxDateTime::TimeZone(wxDateTime::Local).GetOffset(), + -1, // This is pseudo-offset used for local time zone // Fictitious TimeZone offsets to ensure time zone formating and // interpretation works @@ -1638,4 +1638,27 @@ TEST_CASE("wxDateTime::SetOnDST", "[datetime][dst]") } } +// Tests random problems that used to appear in BST time zone during DST. +// This test is disabled by default as it only passes in BST time zone, due to +// the times hard-coded in it. +TEST_CASE("wxDateTime-BST-bugs", "[datetime][dst][BST][.]") +{ + SECTION("bug-17220") + { + wxDateTime dt; + dt.Set(22, wxDateTime::Oct, 2015, 10, 10, 10, 10); + REQUIRE( dt.IsDST() ); + + CHECK( dt.GetTm().hour == 10 ); + CHECK( dt.GetTm(wxDateTime::UTC).hour == 9 ); + + CHECK( dt.Format("%Y-%m-%d %H:%M:%S", wxDateTime::Local ) == "2015-10-22 10:10:10" ); + CHECK( dt.Format("%Y-%m-%d %H:%M:%S", wxDateTime::UTC ) == "2015-10-22 09:10:10" ); + + dt.MakeFromUTC(); + CHECK( dt.Format("%Y-%m-%d %H:%M:%S", wxDateTime::Local ) == "2015-10-22 11:10:10" ); + CHECK( dt.Format("%Y-%m-%d %H:%M:%S", wxDateTime::UTC ) == "2015-10-22 10:10:10" ); + } +} + #endif // wxUSE_DATETIME From c7c30504c83ff21bb0ef0686afac418d12042d2e Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 29 Nov 2017 23:15:57 +0100 Subject: [PATCH 04/13] Do nothing when converting wxDateTime to/from local time zone In particular, do not (unexpectedly) adjust time by the DST. Closes #16585. See #10445. --- interface/wx/datetime.h | 9 ++------- src/common/datetime.cpp | 12 ++++++------ tests/datetime/datetimetest.cpp | 19 +++++++++---------- 3 files changed, 17 insertions(+), 23 deletions(-) diff --git a/interface/wx/datetime.h b/interface/wx/datetime.h index 289ac1da48..814c77c5c6 100644 --- a/interface/wx/datetime.h +++ b/interface/wx/datetime.h @@ -1257,10 +1257,7 @@ public: If @a noDST is @true, no DST adjustments will be made. - Notice using wxDateTime::Local for @a tz parameter doesn't really make - sense and may result in unexpected results as it will return a - different object when DST is in use and @a noDST has its default value - of @false. + If @a tz parameter is wxDateTime::Local, no adjustment is performed. @return The date adjusted by the different between the given and the local time zones. @@ -1297,9 +1294,7 @@ public: If @a noDST is @true, no DST adjustments will be made. - Notice that, as with FromTimezone(), using wxDateTime::Local as @a tz - doesn't really make sense and may return a different object when DST is - in effect and @a noDST is @false. + If @a tz parameter is wxDateTime::Local, no adjustment is performed. @return The date adjusted by the different between the local and the given time zones. diff --git a/src/common/datetime.cpp b/src/common/datetime.cpp index 5b6013b8a8..2696551520 100644 --- a/src/common/datetime.cpp +++ b/src/common/datetime.cpp @@ -2094,11 +2094,11 @@ wxDateTime& wxDateTime::MakeTimezone(const TimeZone& tz, bool noDST) { long secDiff = wxGetTimeZone() + tz.GetOffset(); - // We are converting from the local time, but local time zone does not - // include the DST offset (as it varies depending on the date), so we have - // to handle DST manually, unless a special flag inhibiting this was - // specified. - if ( !noDST && (IsDST() == 1) ) + // We are converting from the local time to some other time zone, but local + // time zone does not include the DST offset (as it varies depending on the + // date), so we have to handle DST manually, unless a special flag + // inhibiting this was specified. + if ( !noDST && (IsDST() == 1) && !tz.IsLocal() ) { secDiff -= DST_OFFSET; } @@ -2111,7 +2111,7 @@ wxDateTime& wxDateTime::MakeFromTimezone(const TimeZone& tz, bool noDST) long secDiff = wxGetTimeZone() + tz.GetOffset(); // See comment in MakeTimezone() above, the logic here is exactly the same. - if ( !noDST && (IsDST() == 1) ) + if ( !noDST && (IsDST() == 1) && !tz.IsLocal() ) { secDiff -= DST_OFFSET; } diff --git a/tests/datetime/datetimetest.cpp b/tests/datetime/datetimetest.cpp index 0ae4321900..246193790b 100644 --- a/tests/datetime/datetimetest.cpp +++ b/tests/datetime/datetimetest.cpp @@ -1565,21 +1565,20 @@ void DateTimeTestCase::TestTranslateFromUnicodeFormat() void DateTimeTestCase::TestConvToFromLocalTZ() { - // Choose a date when the DST is on in many time zones: in this case, - // converting to/from local TZ does modify the object because it - // adds/subtracts DST to/from it, so to get the expected results we need to - // explicitly disable DST support in these functions. + // Choose a date when the DST is on in many time zones and verify that + // converting from/to local time zone still doesn't modify time in this + // case as this used to be broken. wxDateTime dt(18, wxDateTime::Apr, 2017, 19); - CPPUNIT_ASSERT_EQUAL( dt.FromTimezone(wxDateTime::Local, true), dt ); - CPPUNIT_ASSERT_EQUAL( dt.ToTimezone(wxDateTime::Local, true), dt ); + CHECK( dt.FromTimezone(wxDateTime::Local) == dt ); + CHECK( dt.ToTimezone(wxDateTime::Local) == dt ); - // And another one when it is off: in this case, there is no need to pass - // "true" as "noDST" argument to these functions. + // For a date when the DST is not used, this always worked, but still + // verify that it continues to. dt = wxDateTime(18, wxDateTime::Jan, 2018, 19); - CPPUNIT_ASSERT_EQUAL( dt.FromTimezone(wxDateTime::Local), dt ); - CPPUNIT_ASSERT_EQUAL( dt.ToTimezone(wxDateTime::Local), dt ); + CHECK( dt.FromTimezone(wxDateTime::Local) == dt ); + CHECK( dt.ToTimezone(wxDateTime::Local) == dt ); } static void DoTestSetFunctionsOnDST(const wxDateTime &orig) From 4868ec08932f136cf12f781c53974c35eae7ff2e Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 29 Nov 2017 23:19:01 +0100 Subject: [PATCH 05/13] Show more information if DateTimeTestCase::TestTimeFormat() fails No real changes, just show the variable values if any checks fail and also continue running the test for the other data points even if one of them fails. --- tests/datetime/datetimetest.cpp | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/tests/datetime/datetimetest.cpp b/tests/datetime/datetimetest.cpp index 246193790b..832ff1a7be 100644 --- a/tests/datetime/datetimetest.cpp +++ b/tests/datetime/datetimetest.cpp @@ -665,6 +665,15 @@ void DateTimeTestCase::TestTimeFormat() CompareTime // time only }; + const char* const compareKindStrings[] = + { + "nothing", + "both date and time", + "both date and time but without century", + "only dates", + "only times", + }; + static const struct { CompareKind compareKind; @@ -785,13 +794,16 @@ void DateTimeTestCase::TestTimeFormat() if ( !strstr(fmt, "%z") && !isLocalTz ) dt2.MakeFromTimezone(tz); + INFO("Comparing " << compareKindStrings[kind] << " for " + << dt << " with " << dt2 + << " (format result=\"" << s << "\")"); + switch ( kind ) { case CompareYear: if ( dt2.GetCentury() != dt.GetCentury() ) { - CPPUNIT_ASSERT_EQUAL(dt.GetYear() % 100, - dt2.GetYear() % 100); + CHECK( dt.GetYear() % 100 == dt2.GetYear() % 100); dt2.SetYear(dt.GetYear()); } @@ -799,15 +811,15 @@ void DateTimeTestCase::TestTimeFormat() wxFALLTHROUGH; case CompareBoth: - CPPUNIT_ASSERT_EQUAL( dt, dt2 ); + CHECK( dt == dt2 ); break; case CompareDate: - CPPUNIT_ASSERT( dt.IsSameDate(dt2) ); + CHECK( dt.IsSameDate(dt2) ); break; case CompareTime: - CPPUNIT_ASSERT( dt.IsSameTime(dt2) ); + CHECK( dt.IsSameTime(dt2) ); break; case CompareNone: From d3a01e3fe6701f35e91af42f5406fb861b680b7f Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 29 Nov 2017 23:55:13 +0100 Subject: [PATCH 06/13] Show more information in other DateTimeTestCase tests too Show the loop variable when doing checks inside a loop to make it more obvious for which test case the failures occur. Also use CHECK(), instead of REQUIRE(), to which CPPUNIT_ASSERT_EQUAL expands, to continue with the other loop iterations after failure. --- tests/datetime/datetimetest.cpp | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/tests/datetime/datetimetest.cpp b/tests/datetime/datetimetest.cpp index 832ff1a7be..3916885841 100644 --- a/tests/datetime/datetimetest.cpp +++ b/tests/datetime/datetimetest.cpp @@ -301,15 +301,13 @@ void DateTimeTestCase::TestTimeSet() for ( size_t n = 0; n < WXSIZEOF(testDates); n++ ) { const Date& d1 = testDates[n]; - wxDateTime dt = d1.DT(); + const wxDateTime dt = d1.DT(); Date d2; d2.Init(dt.GetTm()); - wxString s1 = d1.Format(), - s2 = d2.Format(); - - CPPUNIT_ASSERT_EQUAL( s1, s2 ); + INFO("n=" << n); + CHECK( d1.Format() == d2.Format() ); } } @@ -324,10 +322,11 @@ void DateTimeTestCase::TestTimeJDN() // JDNs must be computed for UTC times double jdn = dt.FromUTC().GetJulianDayNumber(); - CPPUNIT_ASSERT_EQUAL( d.jdn, jdn ); + INFO("n=" << n); + CHECK( d.jdn == jdn ); dt.Set(jdn); - CPPUNIT_ASSERT_EQUAL( jdn, dt.GetJulianDayNumber() ); + CHECK( jdn == dt.GetJulianDayNumber() ); } } @@ -341,8 +340,10 @@ void DateTimeTestCase::TestTimeWDays() const Date& d = testDates[n]; wxDateTime dt(d.day, d.month, d.year, d.hour, d.min, d.sec); + INFO("n=" << n); + wxDateTime::WeekDay wday = dt.GetWeekDay(); - CPPUNIT_ASSERT_EQUAL( d.wday, wday ); + CHECK( d.wday == wday ); } // test SetToWeekDay() @@ -1020,16 +1021,18 @@ void DateTimeTestCase::TestTimeTicks() wxDateTime dt = d.DT().MakeTimezone(TZ_TEST, true /* no DST */); + INFO("n=" << n); + // GetValue() returns internal UTC-based representation, we need to // convert it to local TZ before comparing time_t ticks = (dt.GetValue() / 1000).ToLong() + TZ_LOCAL.GetOffset(); if ( dt.IsDST() ) ticks += 3600; - CPPUNIT_ASSERT_EQUAL( d.gmticks, ticks + tzOffset ); + CHECK( d.gmticks == ticks + tzOffset ); dt = d.DT().FromTimezone(wxDateTime::UTC); ticks = (dt.GetValue() / 1000).ToLong(); - CPPUNIT_ASSERT_EQUAL( d.gmticks, ticks ); + CHECK( d.gmticks == ticks ); } } From d49784b0a254586cef72afbdf03046a7793590bc Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 30 Nov 2017 17:44:41 +0100 Subject: [PATCH 07/13] Disable wxDateTime tests failing due to TZ offset changes wxDateTime timezone-related methods always use the current timezone offset, while other methods, using CRT, use correct value for the given date, which may be different. This discrepancy accounted for test failures in Europe/Minsk time zone as Belarus has switched from UTC+2 to UTC+3 since 1999 date used in the test. It is impossible to really fix the problem easily, so just skip the test in this case and also mention this bug in the documentation. See #15370. --- interface/wx/datetime.h | 4 ++++ tests/datetime/datetimetest.cpp | 15 +++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/interface/wx/datetime.h b/interface/wx/datetime.h index 814c77c5c6..a643280209 100644 --- a/interface/wx/datetime.h +++ b/interface/wx/datetime.h @@ -1248,6 +1248,10 @@ public: for more information about time zones. Normally, these functions should be rarely used. + Note that all functions in this section always use the current offset + for the specified time zone and don't take into account its possibly + different historical value at the given date. + Related functions in other groups: GetBeginDST(), GetEndDST() */ //@{ diff --git a/tests/datetime/datetimetest.cpp b/tests/datetime/datetimetest.cpp index 3916885841..ba68cfcb66 100644 --- a/tests/datetime/datetimetest.cpp +++ b/tests/datetime/datetimetest.cpp @@ -763,6 +763,21 @@ void DateTimeTestCase::TestTimeFormat() // do convert date to string wxString s = dt.Format(fmt, tz); + // Normally, passing time zone to Format() should have exactly + // the same effect as converting to this time zone before + // calling it, however the former may use standard library date + // handling in strftime() implementation while the latter + // always uses our own code and they may disagree if the offset + // for this time zone has changed since the given date, as the + // standard library handles it correctly (at least under Unix), + // while our code doesn't handle time zone changes at all. + // + // Short of implementing full support for time zone database, + // we can't really do anything about this other than skipping + // the test in this case. + if ( s != dt.ToTimezone(tz).Format(fmt) ) + continue; + // convert back wxDateTime dt2; const char *result = dt2.ParseFormat(s, fmt); From 322144299d040356ca8d5418861a81e51000bd0b Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 30 Nov 2017 17:53:52 +0100 Subject: [PATCH 08/13] Use wxDateTime::TimeZone::IsLocal() in the unit test No real changes, as this doesn't affect this test, but use the new IsLocal() method instead of comparing the time zone offset with the time zone, which doesn't work correctly for BST. --- tests/datetime/datetimetest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/datetime/datetimetest.cpp b/tests/datetime/datetimetest.cpp index ba68cfcb66..3437040b1f 100644 --- a/tests/datetime/datetimetest.cpp +++ b/tests/datetime/datetimetest.cpp @@ -722,7 +722,7 @@ void DateTimeTestCase::TestTimeFormat() for ( unsigned idxtz = 0; idxtz < WXSIZEOF(timeZonesOffsets); ++idxtz ) { wxDateTime::TimeZone tz(timeZonesOffsets[idxtz]); - const bool isLocalTz = tz.GetOffset() == -wxGetTimeZone(); + const bool isLocalTz = tz.IsLocal(); for ( size_t d = 0; d < WXSIZEOF(formatTestDates); d++ ) { From ef372460bed62334c4daab68e3ae327a3f959642 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 30 Nov 2017 21:43:27 +0100 Subject: [PATCH 09/13] Recognize BST and GMT as time zones for UK This doesn't really change anything yet, but is a prerequisite for implementing better DST handling for the BST time zone. --- src/common/datetime.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/common/datetime.cpp b/src/common/datetime.cpp index 2696551520..da0a512299 100644 --- a/src/common/datetime.cpp +++ b/src/common/datetime.cpp @@ -861,7 +861,8 @@ wxDateTime::Country wxDateTime::GetCountry() struct tm *tm = wxLocaltime_r(&t, &tmstruct); wxString tz = wxCallStrftime(wxS("%Z"), tm); - if ( tz == wxT("WET") || tz == wxT("WEST") ) + if ( tz == wxT("WET") || tz == wxT("WEST") || + tz == wxT("BST") || tz == wxT("GMT") ) { ms_country = UK; } From 0e65d91a38e3a77f0c040a18006835627cd88c2d Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 30 Nov 2017 21:44:38 +0100 Subject: [PATCH 10/13] Correctly detect DST in UK during 1969-1971 BST-only period Take into account that from 1968-10-27 and until 1971-10-31 UK used BST time instead of GMT, i.e. that DST was permanently on. This fixes unit test failures in BST time zone for the dates around the Unix epoch of 1970-01-01. See #15370. --- src/common/datetime.cpp | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/src/common/datetime.cpp b/src/common/datetime.cpp index da0a512299..96c8c12a56 100644 --- a/src/common/datetime.cpp +++ b/src/common/datetime.cpp @@ -2081,10 +2081,28 @@ int wxDateTime::IsDST(wxDateTime::Country country) const { int year = GetYear(); - if ( !IsDSTApplicable(year, country) ) + country = GetCountry(); + switch ( country ) { - // no DST time in this year in this country - return -1; + case UK: + // There is a special, but important, case of UK which was + // permanently on BST, i.e. using DST, during this period. It + // is important because it covers Unix epoch and without + // accounting for the DST during it, various tests done around + // the epoch time would fail in BST time zone (only!). + if ( IsEarlierThan(wxDateTime(31, Oct, 1971)) && + IsLaterThan(wxDateTime(27, Oct, 1968)) ) + { + return true; + } + wxFALLTHROUGH; + + default: + if ( !IsDSTApplicable(year, country) ) + { + // no DST time in this year in this country + return -1; + } } return IsBetween(GetBeginDST(year, country), GetEndDST(year, country)); From 179dced0e096a577bc2550e521dc020808ac2d0d Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 30 Nov 2017 21:52:46 +0100 Subject: [PATCH 11/13] Include testdate.h before catch.hpp in unit tests This ensures that dates are printed out correctly if comparing them fails. It might be better to avoid always including this header, but this is the simplest solution. --- tests/datetime/datetimetest.cpp | 2 -- tests/testdate.h | 2 ++ tests/testprec.h | 4 ++++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/datetime/datetimetest.cpp b/tests/datetime/datetimetest.cpp index 3437040b1f..8069c4d845 100644 --- a/tests/datetime/datetimetest.cpp +++ b/tests/datetime/datetimetest.cpp @@ -24,8 +24,6 @@ #include "wx/wxcrt.h" // for wxStrstr() -#include "testdate.h" - // to test Today() meaningfully we must be able to change the system date which // is not usually the case, but if we're under Win32 we can try it -- define // the macro below to do it diff --git a/tests/testdate.h b/tests/testdate.h index 674a3e41b6..dba0b0f74a 100644 --- a/tests/testdate.h +++ b/tests/testdate.h @@ -11,6 +11,8 @@ #include "wx/datetime.h" +#include + // need this to be able to use CPPUNIT_ASSERT_EQUAL with wxDateTime objects inline std::ostream& operator<<(std::ostream& ostr, const wxDateTime& dt) { diff --git a/tests/testprec.h b/tests/testprec.h index f5661446ed..03f5f624ce 100644 --- a/tests/testprec.h +++ b/tests/testprec.h @@ -4,6 +4,10 @@ #include "wx/wxprec.h" #include "wx/stopwatch.h" #include "wx/evtloop.h" + +// This needs to be included before catch.hpp to be taken into account. +#include "testdate.h" + #include "wx/catch_cppunit.h" // Custom test macro that is only defined when wxUIActionSimulator is available From 15a97924b66b99c7b7114825a9e349deeddf38a5 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 30 Nov 2017 22:23:29 +0100 Subject: [PATCH 12/13] Simplify wxDateTime ticks test by only using UTC times Converting to another time zone and dealing with DST is completely useless here, ticks values are always in UTC, so we can just use UTC values from the beginning. --- tests/datetime/datetimetest.cpp | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/tests/datetime/datetimetest.cpp b/tests/datetime/datetimetest.cpp index 8069c4d845..a5258d417f 100644 --- a/tests/datetime/datetimetest.cpp +++ b/tests/datetime/datetimetest.cpp @@ -1019,32 +1019,17 @@ void DateTimeTestCase::TestTimeSpanFormat() void DateTimeTestCase::TestTimeTicks() { - static const wxDateTime::TimeZone TZ_LOCAL(wxDateTime::Local); - static const wxDateTime::TimeZone TZ_TEST(wxDateTime::NZST); - - // this offset is needed to make the test work in any time zone when we - // only have expected test results in UTC in testDates - static const long tzOffset = TZ_LOCAL.GetOffset() - TZ_TEST.GetOffset(); - for ( size_t n = 0; n < WXSIZEOF(testDates); n++ ) { const Date& d = testDates[n]; if ( d.gmticks == -1 ) continue; - wxDateTime dt = d.DT().MakeTimezone(TZ_TEST, true /* no DST */); + const wxDateTime dt = d.DT().FromTimezone(wxDateTime::UTC); INFO("n=" << n); - // GetValue() returns internal UTC-based representation, we need to - // convert it to local TZ before comparing - time_t ticks = (dt.GetValue() / 1000).ToLong() + TZ_LOCAL.GetOffset(); - if ( dt.IsDST() ) - ticks += 3600; - CHECK( d.gmticks == ticks + tzOffset ); - - dt = d.DT().FromTimezone(wxDateTime::UTC); - ticks = (dt.GetValue() / 1000).ToLong(); + time_t ticks = (dt.GetValue() / 1000).ToLong(); CHECK( d.gmticks == ticks ); } } From 36cc6ac1d2ce30ce3818df06bc2460f9924c5cf4 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 30 Nov 2017 23:38:48 +0100 Subject: [PATCH 13/13] Add a hack to work around DST problems in unit tests in BST wxDateTime is still broken with respect to DST outside of the standard time_t range, but make a special exception for the dates near Unix epoch when using BST as not accounting for DST for them broke the unit tests when run in this time zone. Note that constructing the dates outside of the standard range is broken too, so if this problem is ever fully fixed in GetTm(), it would need to be fixed in Set() too, where the DST is not taken into account neither. The problems in unit tests were due to using the (correctly behaving) standard mktime() for 1970-01-01 but then using our (broken) GetTm() for accessing them and could, in principle, be also worked around by not using mktime() for this date, but it seems better to keep correct behaviour at least for it, even if it's still broken for many earlier dates. See #15370. --- src/common/datetime.cpp | 41 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/src/common/datetime.cpp b/src/common/datetime.cpp index 96c8c12a56..5c79723772 100644 --- a/src/common/datetime.cpp +++ b/src/common/datetime.cpp @@ -341,6 +341,33 @@ void wxInitTm(struct tm& tm) tm.tm_isdst = -1; // auto determine } +// Internal helper function called only for times outside of standard time_t +// range. +// +// It is just a hack to work around the fact that we can't call IsDST() and +// related methods from GetTm() for the reasons explained there. +static int GetDSTOffset(wxLongLong t) +{ + bool isDST = false; + + switch ( wxDateTime::GetCountry() ) + { + case wxDateTime::UK: + // We don't need to check for the end value in 1971 as this is + // inside the standard range, so check just for beginning of the + // permanent BST period in UK, see IsDST(). + if ( t < 0 && + t >= wxDateTime(27, wxDateTime::Oct, 1968).GetValue() ) + isDST = true; + break; + + default: + break; + } + + return isDST ? wxDateTime::DST_OFFSET : 0; +} + // ============================================================================ // implementation of wxDateTime // ============================================================================ @@ -1475,11 +1502,21 @@ wxDateTime::Tm wxDateTime::GetTm(const TimeZone& tz) const //else: use generic code below } + long secDiff = tz.GetOffset(); + + // We need to account for DST as always when converting to broken down time + // components, but we can't call IsDST() from here because this would + // result in infinite recursion as IsDST() starts by calling GetYear() + // which just calls back to this function. So call a special function which + // is used just here to determine the DST offset to add. + if ( tz.IsLocal() ) + secDiff += GetDSTOffset(m_time); + + wxLongLong timeMidnight = m_time + secDiff * 1000; + // remember the time and do the calculations with the date only - this // eliminates rounding errors of the floating point arithmetics - wxLongLong timeMidnight = m_time + tz.GetOffset() * 1000; - long timeOnly = (timeMidnight % MILLISECONDS_PER_DAY).ToLong(); // we want to always have positive time and timeMidnight to be really