[PATCH] [libcxx] Add locales to available_features for tests.

Eric Fiselier eric at efcs.ca
Fri Aug 1 16:00:06 PDT 2014


I'm super happy to see this patch. I had just been skipping the localization tests all together on machines that lacked these locales. I also like the idea of splitting up the test by locale and will lend a hand to do so. The patch itself looks good but I have some questions about its general direction.

1. Do you understand why these specific locales where chosen for testing? (I dont).  I'm concerned that we might are marginalizing important tests. 
2. As more LIT features become integrated in the testsuite whats the best way to prevent fragmentation between LIT and testit? (does testit even work on linux?) 

As an addendum to this patch I plan to add documentation to the website about the locales required to run the testsuite. Since its possible for all of these tests to be supported within a linux-gnu environment I think we need to document that. I've been meaning to add this documentation for a while and this patch forces my hand.

================
Comment at: test/lit.cfg:192-193
@@ +191,4 @@
+        config.available_features.add('locale.{}'.format(loc))
+    except:
+        pass
+
----------------
Should we emit a warning here about the missing locale?

http://reviews.llvm.org/D4759






More information about the cfe-commits mailing list