[libcxx-commits] [PATCH] D119930: [libcxx] [test] Use proper UTF-8 locales on Windows

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed Feb 16 13:24:00 PST 2022


ldionne added inline comments.


================
Comment at: libcxx/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/tolower_1.pass.cpp:22
+// or Windows 11).
+// XFAIL: windows
+
----------------
mstorsjo wrote:
> Mordante wrote:
> > Would it be easy to detect the UCRT version in CMake and add an appropriate flag `libcpp-no-vcruntime`? Then we can use `UNSUPPORTED: libcpp-ucrt-has-utf8-bug`. That way others who want to test on Windows don't need to use the exact same version as the CI has.
> > 
> > When that's not possible I would prefer UNSUPPORTED over XFAIL. Again to avoid failing tests depending on which Windows platform they're executed on.
> Hmm, unsure if it's possible to check the version of UCRT. But I guess we could do a proper runtime check when starting testing - like we already do to see if the system supports e.g. the locales we test. I'll give that a shot. (That can be used for other temporary platform bugs too.)
> 
> Yeah UNSUPPORTED is good if there's going to be variability - but with the drawback that it's easy to forget that in place for too long, when the issue is resolved in the CI environment. (While it's nice if the tests pass on any version of Windows, once the CI environment no longer require workarounds, I'd move towards removing them.)
I think it's definitely better to use XFAIL than UNSUPPORTED, especially in these circumstances. XFAIL allows forgetting about this, since it'll start XPASSing when the issue gets fixed eventually. If we use UNSUPPORTED, we are basically dropping this test coverage forever, unless someone comes back and notices that.

I agree even better would be to have a version number -- then we can use either UNSUPPORTED or XFAIL, it doesn't matter, since it won't apply once we update to something newer.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119930/new/

https://reviews.llvm.org/D119930



More information about the libcxx-commits mailing list