time to the build
ClosedPublic
Actions

Authored by yuripv on Oct 13 2018, 1:00 PM.

Details

Reviewers

ngie
kevans

Commits

rS339904: Connect libc/tests/time to the build, adding test cases for strptime()

Summary

Connect libc/tests/time to the build and modify strptime test program a bit:

add check for 0 in %d conversion
add check for 0 in %l conversion
address the fact that we (correctly) fail 61 in %S conversion
disable timezone test cases for the moment as timezone conversions are non-standard and differ between implementations

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

yuripv created this revision.Oct 13 2018, 1:00 PM

Herald added a subscriber: imp. · View Herald TranscriptOct 13 2018, 1:00 PM

Macro goatforit:
I wish I could commit this change on your behalf, but I turned in my src bit last year.

This still fails for century/year conversions indicating issues with our strptime that need to be taken care of.

Regarding this comment, do you mean that the test fails when you remove the atf_tc_expect_fail call on 231-234? If so, please restore that line, but feel free to update the message to improve the clarity, since what I put in there a couple years back is pretty vague.

contrib/netbsd-tests/lib/libc/time/t_strptime.c
430–439 ↗	(On Diff #49093)	This should probably be reported back to the NetBSD folks.

This revision is now accepted and ready to land.Oct 13 2018, 3:57 PM

imp added inline comments.Oct 13 2018, 4:26 PM

contrib/netbsd-tests/lib/libc/time/t_strptime.c
430–439 ↗	(On Diff #49093)	The sooner we can kill all bogus and insane references to double leap seconds the better. They never were a real thing, just a figment of POSIX's imagination...

imp added inline comments.Oct 13 2018, 4:27 PM

contrib/netbsd-tests/lib/libc/time/t_strptime.c
104 ↗	(On Diff #49093)	we should really fix them to test what we want our implementation to do, rather than cutting them out entirely

ngie added inline comments.Oct 13 2018, 4:54 PM

contrib/netbsd-tests/lib/libc/time/t_strptime.c
104 ↗	(On Diff #49093)	Totally agree!
430–439 ↗	(On Diff #49093)	I agree. I was hoping that by getting NetBSD onboard, it would be one less implementation to handle :)..

In D17546#374448, @ngie wrote:

Regarding this comment, do you mean that the test fails when you remove the atf_tc_expect_fail call on 231-234? If so, please restore that line, but feel free to update the message to improve the clarity, since what I put in there a couple years back is pretty vague.

No, that TC is passing now without errors. I guess I need to mark century/year TC the same now.

In D17546#374471, @yuripv wrote:

No, that TC is passing now without errors. I guess I need to mark century/year TC the same now.

Macro like:

yuripv marked 2 inline comments as done.Oct 13 2018, 5:00 PM

yuripv added inline comments.

contrib/netbsd-tests/lib/libc/time/t_strptime.c
104 ↗	(On Diff #49093)	I didn't want to add even more #ifdef spaghetti and was thinking about creating separate test program to handle just %z and %Z conversions. Also note that this change is mainly about connecting the tests to the build, so we aren't missing any value at the moment.
430–439 ↗	(On Diff #49093)	It could be that they rely on historical behavior, more so seeing as glibc does the same.

yuripv updated this revision to Diff 49467.Oct 22 2018, 9:33 PM

yuripv edited the summary of this revision. (Show Details)

This revision now requires review to proceed.Oct 22 2018, 9:33 PM

I have updated the suite to pass everything that is enabled, still having tz tests disabled as there's no easy way to make them pass and our parsing code is rudimentary compared to netbsd's one.

This revision was not accepted when it landed; it landed in state Needs Review.Oct 30 2018, 2:37 AM

Closed by commit rS339904: Connect libc/tests/time to the build, adding test cases for strptime() (authored by yuripv). · Explain Why

This revision was automatically updated to reflect the committed changes.