This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] [test] Fix time.get.byname get_one for Glibc and Windows
ClosedPublic

Authored by mstorsjo on Feb 21 2022, 2:24 PM.

Details

Summary

This matches the fixes for the wchar version in
f081cc50372f9415ef4fa2204a4b7f54153af455.

Diff Detail

Event Timeline

mstorsjo requested review of this revision.Feb 21 2022, 2:24 PM
mstorsjo created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 21 2022, 2:24 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript

I'll let @Mordante accept, since I don't really know about locales. Looking at this test makes me think "What is even the point of standardized locales, if every platform is going to do them differently anyway? They don't enable portable code, that's for sure!" But I'll just think of this as adding regression coverage, so that if the dumb behavior of Win/Linux/MacOS locales ever quietly changes, we'll be able to notice.

libcxx/test/std/localization/locale.categories/category.time/locale.time.get.byname/get_one.pass.cpp
129–130

Prefer not to break up this string literal.

170–181

Pre-existing: Prefer not to break this one up either.

Mordante accepted this revision.Feb 23 2022, 9:21 AM

I'll let @Mordante accept, since I don't really know about locales. Looking at this test makes me think "What is even the point of standardized locales, if every platform is going to do them differently anyway?

I share that feeling. It's odd that a localized date looks different on Mac and Linux. I'm quite sure the people living in the region with that locale have a certain expectation how things look.

But I'll just think of this as adding regression coverage, so that if the dumb behavior of Win/Linux/MacOS locales ever quietly changes, we'll be able to notice.

We already have some cases where the locale depends on the glibc version :-( Still I feel the test has value. We know we parse the input properly. Especially for dates like 12/2/2022, the 12th of February or the 2nd of December.

LGTM after addressing @Quuxplusone's comments.

This revision is now accepted and ready to land.Feb 23 2022, 9:21 AM