Re: [jira] Commented: (STDCXX-900) 22.locale.time.get test fails 15 assertions
Travis Vitek wrote: Martin Sebor wrote: [...] IMO, we should definitely remove bogus assertions. Unless we already have one, we should also open an issue to get these directives implemented. We can then decide exactly how and to what extent. Well I don't really feel that the assertions should be removed. I think the assertions should test things that are possible. I.e., the assertions should be something like... TEST (T (0, 0, 0, 0, 0, 0, 0), "9", 1, "U", 0, Eof); TEST (T (0, 0, 0, 0, 0, 0, 0), "2", 1, "W", 0, Eof); This would verify that the value is parsed, but not used (because there isn't enough information available), or even TEST (T (0, 0, 0, 0, 0, 0, 0), "9", 1, "U", 0, Fail); TEST (T (0, 0, 0, 0, 0, 0, 0), "2", 1, "W", 0, Fail); This would verify that the format specifier is rejected, and would need to be fixed when the appropriate support is added. I agree. I just don't think that starting by deciding what the behavior should be in the corner/unspecified cases is going to the most efficient way to dispatch the issue. The test fails because of bad assertions. The most efficient way to resolve the issue is to remove the bad assertions :) The fact that the assertions exercise unimplemented functionality is a separate problem. I'd also expect to see the following assertions added (and possibly others that verify the week is being used to help calculate the resulting date). // expect 2008-12-29 given "2009-W01-1" TEST (T (0, 0, 0, 29, 12, 28, 1, 363), "2009-W01-1", 10, "%Y-W%W-%d", 0, Eof); // or Fail // expect 2010-01-03 given "2009-W53-7" TEST (T (0, 0, 0, 3, 1, 30, 3, 3), "2009-W53-7", 10, "%Y-W%W-%d", 0, Eof); // of Fail Of course that would mean that we'd have tests that are verifying the feature is not implemented. In that case I guess it would be best to just comment out the assertions and add a note explaining why. Input? Sounds good to me. Better to have good assertions commented out than bad ones firing! Even if I had a perfect implementation (i.e. I called the gnu strptime function internally), the first assertion could never be made to pass. Given a Sunday-based week number of 9, I cannot possibly guess the year, weekday and day of year to be 2220 AD, Tuesday and 60 respectively. The assertion is bad. This assertion needs to be updated or removed entirely. At the very least the expected date should be all zeros as the below testcase does. TEST (T (0, 0, 0, 0, 0, 0, 0), "0", 1, "W", 0, Eof); I modify time_get<>::do_get() to consume and ignore characters that match "%U" and "%W" from the stream. This would get the assertion to pass, but it wouldn't be incredibly useful. Right. It is impossible to reliably parse any useful date-time information from a string that contains only the formatted week number. Just a note; the gnu strptime() succeeds if parsing only the week number, but it doesn't modify the date. [...] Even if we have a partially specified date (I believe we need at least the year and day of week), we still need somewhere to store the additional data so that we can store the value we parse, and then after the parsing is done so that we can use it to calculate something useful. At the very least I think we'll be breaking binary compatibility. I remember this dilemma when implementing it. I don't recall how I thought it could be dealt with except for some hackery (borrowing some otherwise unused struct tm members for temporary storage in between recursive calls). I'd hate to see us break binary compatibility just for this. Yeah, there aren't any 'unused' members on most platforms, so that is a no-go. I meant unused by the current directive. Or simply borrow one, use it for the duration of the function, and restore it when we're done. But I haven't thought about this too hard so don't put too much faith in it. It's just an unbaked idea for a dirty hack to get around the ABI constraint. We certainly can't hope to make any changes to the public API of the facet. Yeah. I think I'll remove the offending assertions (with your approval) and just file a new issue to get support for the %{E,O}{U,W} format specifiers added and I'll leave it at that. Okay. Travis
RE: [jira] Commented: (STDCXX-900) 22.locale.time.get test fails 15 assertions
Martin Sebor wrote: >Travis Vitek wrote: >> Martin Sebor wrote: >>> Travis Vitek (JIRA) wrote: Well, most of these failures are destined to fail until the >>> test is rewritten. >>> >>> Are you sure you meant that the test needs to be rewritten? >>> (I'm trying to reconcile that with your subsequent comment >>> about binary compatibility). >> >> The entire test doesn't need to be rewritten, just the >assertions that >> deal with %U and %W... >> >> TEST (T (0, 0, 0, 0, 0, 320, 2, 60), "9", 1, "U", 0, Eof); > >Oh. Well, it's entirely possible that some of them are bogus. >The ChangeLog entry for the change that added them back in >2002 (see p4 change 138327) isn't helpful: > > * Exercised std::time_get. > * Exercised facet with user-defined LC_TIME data. > * Exercised extensions implementing the full POSIX spec. > >It could be that I added the assertions even though they were >failing, thinking I'd get the missing functionality implemented >next, and then got distracted by something and never got back >to it. Yeah, it looks like this may have been the case. > >IMO, we should definitely remove bogus assertions. Unless we >already have one, we should also open an issue to get these >directives implemented. We can then decide exactly how and >to what extent. Well I don't really feel that the assertions should be removed. I think the assertions should test things that are possible. I.e., the assertions should be something like... TEST (T (0, 0, 0, 0, 0, 0, 0), "9", 1, "U", 0, Eof); TEST (T (0, 0, 0, 0, 0, 0, 0), "2", 1, "W", 0, Eof); This would verify that the value is parsed, but not used (because there isn't enough information available), or even TEST (T (0, 0, 0, 0, 0, 0, 0), "9", 1, "U", 0, Fail); TEST (T (0, 0, 0, 0, 0, 0, 0), "2", 1, "W", 0, Fail); This would verify that the format specifier is rejected, and would need to be fixed when the appropriate support is added. I'd also expect to see the following assertions added (and possibly others that verify the week is being used to help calculate the resulting date). // expect 2008-12-29 given "2009-W01-1" TEST (T (0, 0, 0, 29, 12, 28, 1, 363), "2009-W01-1", 10, "%Y-W%W-%d", 0, Eof); // or Fail // expect 2010-01-03 given "2009-W53-7" TEST (T (0, 0, 0, 3, 1, 30, 3, 3), "2009-W53-7", 10, "%Y-W%W-%d", 0, Eof); // of Fail Of course that would mean that we'd have tests that are verifying the feature is not implemented. In that case I guess it would be best to just comment out the assertions and add a note explaining why. Input? >> >> Even if I had a perfect implementation (i.e. I called the >gnu strptime >> function internally), the first assertion could never be >made to pass. >> Given a Sunday-based week number of 9, I cannot possibly >guess the year, >> weekday and day of year to be 2220 AD, Tuesday and 60 >respectively. The >> assertion is bad. This assertion needs to be updated or removed >> entirely. At the very least the expected date should be all >zeros as the >> below testcase does. >> >> TEST (T (0, 0, 0, 0, 0, 0, 0), "0", 1, "W", 0, Eof); >> >> I modify time_get<>::do_get() to consume and ignore characters that >> match "%U" and "%W" from the stream. This would get the assertion to >> pass, but it wouldn't be incredibly useful. > >Right. > >> It is impossible to reliably parse any useful date-time >>> information from a string that contains only the formatted >>> week number. >> >> Just a note; the gnu strptime() succeeds if parsing only the week >> number, but it doesn't modify the date. > [...] >> >>> Even if we have a partially specified date (I believe >>> we need at least the year and day of week), we still need >>> somewhere to store the additional data so that we can store >>> the value we parse, and then after the parsing is done so that >>> we can use it to calculate something useful. At the very least >>> I think we'll be breaking binary compatibility. > >I remember this dilemma when implementing it. I don't recall >how I thought it could be dealt with except for some hackery >(borrowing some otherwise unused struct tm members for >temporary storage in between recursive calls). I'd hate to >see us break binary compatibility just for this. Yeah, there aren't any 'unused' members on most platforms, so that is a no-go. >We certainly >can't hope to make any changes to the public API of the facet. > Yeah. I think I'll remove the offending assertions (with your approval) and just file a new issue to get support for the %{E,O}{U,W} format specifiers added and I'll leave it at that. Travis
Re: [jira] Commented: (STDCXX-900) 22.locale.time.get test fails 15 assertions
Travis Vitek wrote: Martin Sebor wrote: Travis Vitek (JIRA) wrote: Well, most of these failures are destined to fail until the test is rewritten. Are you sure you meant that the test needs to be rewritten? (I'm trying to reconcile that with your subsequent comment about binary compatibility). The entire test doesn't need to be rewritten, just the assertions that deal with %U and %W... TEST (T (0, 0, 0, 0, 0, 320, 2, 60), "9", 1, "U", 0, Eof); Oh. Well, it's entirely possible that some of them are bogus. The ChangeLog entry for the change that added them back in 2002 (see p4 change 138327) isn't helpful: * Exercised std::time_get. * Exercised facet with user-defined LC_TIME data. * Exercised extensions implementing the full POSIX spec. It could be that I added the assertions even though they were failing, thinking I'd get the missing functionality implemented next, and then got distracted by something and never got back to it. IMO, we should definitely remove bogus assertions. Unless we already have one, we should also open an issue to get these directives implemented. We can then decide exactly how and to what extent. Even if I had a perfect implementation (i.e. I called the gnu strptime function internally), the first assertion could never be made to pass. Given a Sunday-based week number of 9, I cannot possibly guess the year, weekday and day of year to be 2220 AD, Tuesday and 60 respectively. The assertion is bad. This assertion needs to be updated or removed entirely. At the very least the expected date should be all zeros as the below testcase does. TEST (T (0, 0, 0, 0, 0, 0, 0), "0", 1, "W", 0, Eof); I modify time_get<>::do_get() to consume and ignore characters that match "%U" and "%W" from the stream. This would get the assertion to pass, but it wouldn't be incredibly useful. Right. It is impossible to reliably parse any useful date-time information from a string that contains only the formatted week number. Just a note; the gnu strptime() succeeds if parsing only the week number, but it doesn't modify the date. I think it does in some cases modify it if can use the parsed data to complete the contents of the struct. This is allowed by the following sentence in POSIX strptime(): It is unspecified whether multiple calls to strptime() using the same tm structure will update the current contents of the structure or overwrite all contents of the structure. On solaris, that same call seems to fail if it encounters the %U or %W specifier in the format string, even if that string contains a fully formatted date. The test tries to to do just this for {{%U}} and {{%W}}. I'm not exactly sure what the expected behavior should be, given the above results on linux and solaris. Perhaps parsing the values and not using them is sufficient, or just failing outright when reading them (as it does now). Regardless of what we decide, Even if we have a partially specified date (I believe we need at least the year and day of week), we still need somewhere to store the additional data so that we can store the value we parse, and then after the parsing is done so that we can use it to calculate something useful. At the very least I think we'll be breaking binary compatibility. I remember this dilemma when implementing it. I don't recall how I thought it could be dealt with except for some hackery (borrowing some otherwise unused struct tm members for temporary storage in between recursive calls). I'd hate to see us break binary compatibility just for this. We certainly can't hope to make any changes to the public API of the facet. Martin So, for the time being, I think the right thing to do is to fix the portion of the test that attempts to to verify {{%U}} and {{%W}} so that it expects failure. As mentioned above... If we wish to fully support this extension, then we should add a new test that verifies {{%U}} and {{%W}} with the necessary format specifiers and then fix the code for 4.3 or later. The test would need include something like this. As mentioned in my previous post, I think we would need to break binary compatibility to even have a chance of making these pass. // expect 2008-12-29 given "2009-W01-1" TEST (T (0, 0, 0, 29, 12, 28, 1, 363), "2009-W01-1", 1, "%Y-W%W-%d", 0, Eof); // expect 2010-01-03 given "2009-W53-7" TEST (T (0, 0, 0, 3, 1, 30, 3, 3), "2009-W53-7", 1, "%Y-W%W-%d", 0, Eof); Travis
RE: [jira] Commented: (STDCXX-900) 22.locale.time.get test fails 15 assertions
Martin Sebor wrote: > >Travis Vitek (JIRA) wrote: >> >> Well, most of these failures are destined to fail until the >test is rewritten. > >Are you sure you meant that the test needs to be rewritten? >(I'm trying to reconcile that with your subsequent comment >about binary compatibility). The entire test doesn't need to be rewritten, just the assertions that deal with %U and %W... TEST (T (0, 0, 0, 0, 0, 320, 2, 60), "9", 1, "U", 0, Eof); Even if I had a perfect implementation (i.e. I called the gnu strptime function internally), the first assertion could never be made to pass. Given a Sunday-based week number of 9, I cannot possibly guess the year, weekday and day of year to be 2220 AD, Tuesday and 60 respectively. The assertion is bad. This assertion needs to be updated or removed entirely. At the very least the expected date should be all zeros as the below testcase does. TEST (T (0, 0, 0, 0, 0, 0, 0), "0", 1, "W", 0, Eof); I modify time_get<>::do_get() to consume and ignore characters that match "%U" and "%W" from the stream. This would get the assertion to pass, but it wouldn't be incredibly useful. > >> It is impossible to reliably parse any useful date-time >information from a string that contains only the formatted >week number. Just a note; the gnu strptime() succeeds if parsing only the week number, but it doesn't modify the date. On solaris, that same call seems to fail if it encounters the %U or %W specifier in the format string, even if that string contains a fully formatted date. >The test tries to to do just this for {{%U}} and >{{%W}}. I'm not exactly sure what the expected behavior should be, given the above results on linux and solaris. Perhaps parsing the values and not using them is sufficient, or just failing outright when reading them (as it does now). Regardless of what we decide, >Even if we have a partially specified date (I believe >we need at least the year and day of week), we still need >somewhere to store the additional data so that we can store >the value we parse, and then after the parsing is done so that >we can use it to calculate something useful. At the very least >I think we'll be breaking binary compatibility. >> >> So, for the time being, I think the right thing to do is to >fix the portion of the test that attempts to to verify {{%U}} >and {{%W}} so that it expects failure. As mentioned above... >If we wish to fully >support this extension, then we should add a new test that >verifies {{%U}} and {{%W}} with the necessary format >specifiers and then fix the code for 4.3 or later. >> The test would need include something like this. As mentioned in my previous post, I think we would need to break binary compatibility to even have a chance of making these pass. // expect 2008-12-29 given "2009-W01-1" TEST (T (0, 0, 0, 29, 12, 28, 1, 363), "2009-W01-1", 1, "%Y-W%W-%d", 0, Eof); // expect 2010-01-03 given "2009-W53-7" TEST (T (0, 0, 0, 3, 1, 30, 3, 3), "2009-W53-7", 1, "%Y-W%W-%d", 0, Eof); Travis
Re: structure of tuple tests ([Fwd: Re: svn commit: r675044 - in /stdcxx/branches/4.3.x: include/rw/_tuple.h include/tuple tests/utilities/20.tuple.cnstr.cpp tests/utilities/20.tuple.creation.cpp test
Eric Lemings wrote: -Original Message- From: Martin Sebor [mailto:[EMAIL PROTECTED] On Behalf Of Martin Sebor Sent: Friday, July 11, 2008 1:06 PM To: dev@stdcxx.apache.org Subject: Re: structure of tuple tests ([Fwd: Re: svn commit: r675044 - in /stdcxx/branches/4.3.x: include/rw/_tuple.h include/tuple tests/utilities/20.tuple.cnstr.cpp tests/utilities/20.tuple.creation.cpp tests/utilities/20.tuple.h tests/utilities/20.tuple.helpers ... The description of the result could be just a character string with the values of the tuple members. For example, for the ctor tuple(1, 2), the description of the expected result could be "{1,2}" To implement the formatting in a general way you might want to make use of the rw_printf() callbacks. See test_user_defined_formatting() in the self/0.printf.cpp test. I was looking for a test that actually uses this user-definfed formatting but couldn't find one. Is there a use of this formatting directive other than the test case in 0.printf.cpp? I think all or most of the tests that make use of the UserClass class (in rw_value.h) do. The formatting function is defined in value.cpp: _rw_fmtxarrayv(). Martin Thanks, Brad.
RE: structure of tuple tests ([Fwd: Re: svn commit: r675044 - in /stdcxx/branches/4.3.x: include/rw/_tuple.h include/tuple tests/utilities/20.tuple.cnstr.cpp tests/utilities/20.tuple.creation.cpp test
> -Original Message- > From: Martin Sebor [mailto:[EMAIL PROTECTED] On Behalf Of Martin Sebor > Sent: Friday, July 11, 2008 1:06 PM > To: dev@stdcxx.apache.org > Subject: Re: structure of tuple tests ([Fwd: Re: svn commit: > r675044 - in /stdcxx/branches/4.3.x: include/rw/_tuple.h > include/tuple tests/utilities/20.tuple.cnstr.cpp > tests/utilities/20.tuple.creation.cpp > tests/utilities/20.tuple.h tests/utilities/20.tuple.helpers > ... > > The description of the result could be just a character string > with the values of the tuple members. For example, for the ctor > tuple(1, 2), the description of the expected result > could be "{1,2}" To implement the formatting in a general way > you might want to make use of the rw_printf() callbacks. See > test_user_defined_formatting() in the self/0.printf.cpp test. I was looking for a test that actually uses this user-definfed formatting but couldn't find one. Is there a use of this formatting directive other than the test case in 0.printf.cpp? Thanks, Brad.
Re: [jira] Commented: (STDCXX-900) 22.locale.time.get test fails 15 assertions
Travis Vitek (JIRA) wrote: [ https://issues.apache.org/jira/browse/STDCXX-900?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12613504#action_12613504 ] Travis Vitek commented on STDCXX-900: - Well, most of these failures are destined to fail until the test is rewritten. Are you sure you meant that the test needs to be rewritten? (I'm trying to reconcile that with your subsequent comment about binary compatibility). Martin It is impossible to reliably parse any useful date-time information from a string that contains only the formatted week number. The test tries to to do just this for {{%U}} and {{%W}}. Even if we have a partially specified date (I believe we need at least the year and day of week), we still need somewhere to store the additional data so that we can store the value we parse, and then after the parsing is done so that we can use it to calculate something useful. At the very least I think we'll be breaking binary compatibility. So, for the time being, I think the right thing to do is to fix the portion of the test that attempts to to verify {{%U}} and {{%W}} so that it expects failure. If we wish to fully support this extension, then we should add a new test that verifies {{%U}} and {{%W}} with the necessary format specifiers and then fix the code for 4.3 or later. 22.locale.time.get test fails 15 assertions --- Key: STDCXX-900 URL: https://issues.apache.org/jira/browse/STDCXX-900 Project: C++ Standard Library Issue Type: Bug Components: Tests Affects Versions: 4.2.0, 4.2.1 Environment: MSVC Reporter: Farid Zaripov Assignee: Travis Vitek Priority: Minor Fix For: 4.2.2 Original Estimate: 6h Remaining Estimate: 6h The 22.locale.time.get test fails with the following assertions: {noformat} # ASSERTION (S7) (5 lines): # TEXT: line 174. time_get::get ("9", ..., "U") ate 0, expected 1, rdstate() == eofbit, got failbit # CLAUSE: lib.locale.time.get # FILE: 22.locale.time.get.cpp # LINE: 758 # ASSERTION (S7) (5 lines): # TEXT: line 183. time_get::get ("9", ..., "U") got { tm_mday=0 [1,31] }, expected { tm_mday=0 [1,31], tm_year=320, tm_wday=2 Tue, tm_yday=60 }, flags = fmtflags(0), locale (C) # CLAUSE: lib.locale.time.get # FILE: 22.locale.time.get.cpp # LINE: 758 # ASSERTION (S7) (5 lines): # TEXT: line 174. time_get::get ("0", ..., "W") ate 0, expected 1, rdstate() == eofbit, got failbit # CLAUSE: lib.locale.time.get # FILE: 22.locale.time.get.cpp # LINE: 778 # ASSERTION (S7) (5 lines): # TEXT: line 174. time_get::get ("2nd weekday, 21st week, 2nd year", ..., "%Ex") ate 17, expected 32, rdstate() == goodbit, got failbit # CLAUSE: lib.locale.time.get # FILE: 22.locale.time.get.cpp # LINE: 1442 # ASSERTION (S7) (5 lines): # TEXT: line 183. time_get::get ("2nd weekday, 21st week, 2nd year", ..., "%Ex") got { tm_mday=0 [1,31] }, expected Tue May 21 00:00:00 3902, flags = fmtflags(0), locale (LC_COLLATE=C;LC_CTYPE=C;LC_MONETARY=C;LC_NUMERIC=C;LC_TIME=test-locale;LC_COLLATE=C) # CLAUSE: lib.locale.time.get # FILE: 22.locale.time.get.cpp # LINE: 1442 # ASSERTION (S7) (5 lines): # TEXT: line 174. time_get::get ("1st", ..., "%OW") ate 3, expected 3, rdstate() == goodbit, got failbit # CLAUSE: lib.locale.time.get # FILE: 22.locale.time.get.cpp # LINE: 1582 # ASSERTION (S7) (5 lines): # TEXT: line 183. time_get::get ("1st", ..., "%OW") got { tm_mday=0 [1,31] }, expected { tm_mday=0 [1,31], tm_wday=1 Mon }, flags = fmtflags(0), locale (LC_COLLATE=C;LC_CTYPE=C;LC_MONETARY=C;LC_NUMERIC=C;LC_TIME=test-locale;LC_COLLATE=C) # CLAUSE: lib.locale.time.get # FILE: 22.locale.time.get.cpp # LINE: 1582 # ASSERTION (S7) (5 lines): # TEXT: line 174. time_get::get (L"9", ..., "U") ate 0, expected 1, rdstate() == eofbit, got failbit # CLAUSE: lib.locale.time.get # FILE: 22.locale.time.get.cpp # LINE: 758 # ASSERTION (S7) (5 lines): # TEXT: line 183. time_get::get (L"9", ..., "U") got { tm_mday=0 [1,31] }, expected { tm_mday=0 [1,31], tm_year=320, tm_wday=2 Tue, tm_yday=60 }, flags = fmtflags(0), locale (C) # CLAUSE: lib.locale.time.get # FILE: 22.locale.time.get.cpp # LINE: 758 # ASSERTION (S7) (5 lines): # TEXT: line 174. time_get::get (L"0", ..., "W") ate 0, expected 1, rdstate() == eofbit, got failbit # CLAUSE: lib.locale.time.get # FILE: 22.locale.time.get.cpp # LINE: 778 # ASSERTION (S7) (5 lines): # TEXT: line 174. time_get::get (L"2nd weekday, 21st week, 2nd year", ..., "%Ex") ate 17, expected 32, rdstate() == goodbit, got failbit # CLAUSE: lib.locale.time.get # FILE: 22.locale.time.get.cpp # LINE: 1442 # ASSERTION (S7) (5 lines): # TEXT: line 183. time_get::get (L"2nd weekday, 21st week, 2nd year", ..., "%Ex") got { tm_mday=0 [1,31] }, expected Tue May 21 00:00:00 3902, flags = fmtflags(0), locale (LC_COLLATE=C;LC_CTYPE=C;LC_MONETARY=C;LC