OK, I've factored out the default case to its own subtest. @embray, would you mind trying this patch out on Cygwin and seeing how it breaks?

I'm thinking the right order to do things will be to get #4334 merged for Android, rebase this test case change on top of that one, and then figure out a reasonable way to make the expected behaviour in the default locale automatically adjust itself (without making the test case tautological).

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be put in the comfy chair!

I also think that this is one of those cases where seemingly tautological test cases may still be worth running, due to non-trivial interactions between the interpreter, the environment, libc, libintl, etc...

I'm actually starting to wonder if we should put together a configure check that reports back the result of:

• setting LC_ALL, LC_CTYPE, and LANG to the empty string
• calling setlocale(LC_CTYPE, "");
• calling setlocale(LC_CTYPE, NULL);

That way we could query sysconfig to find out what the build platform did. My main concern with that idea is that it could get complicated when it came to cross-compilation support, so plan B would be to write a short Programs/_querydefaultlocale.c test helper that did the same thing.

This test still fails on Cygwin because it assumes that by default the locale will be "C" and that coercion will happen (which results in the LC_CTYPE environment variable getting set). So these failures look something like:

======================================================================
FAIL: test_PYTHONCOERCECLOCALE_not_set (test.test_c_locale_coercion.LocaleCoercionTests) (default_locale=True, PYTHONCOERCECLOCALE=None)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/embray/src/python/cpython/Lib/test/test_c_locale_coercion.py", line 325, in _check_c_locale_coercion
    coercion_expected)
  File "/home/embray/src/python/cpython/Lib/test/test_c_locale_coercion.py", line 221, in _check_child_encoding_details
    self.assertEqual(encoding_details, expected_details)
AssertionError: {'fse[124 chars]slashreplace', 'lang': '', 'lc_ctype': '', 'lc_all': ''} != {'fse[124 chars]slashreplace', 'lang': '', 'lc_ctype': 'c.utf-8', 'lc_all': ''}
  {'fsencoding': 'utf-8',
   'lang': '',
   'lc_all': '',
-  'lc_ctype': '',
+  'lc_ctype': 'c.utf-8',
?               +++++++

   'stderr_info': 'utf-8:backslashreplace',
   'stdin_info': 'utf-8:surrogateescape',
   'stdout_info': 'utf-8:surrogateescape'}

In #4361 I just skipped the cases didn't explicitly set one of the LC_ vars to "C" or "POSIX" since otherwise the assumptions in the test of when coercion should occur didn't hold up.

@ncoghlan I think the configure-time check is a good idea. It could still be disabled when cross-compiling, in which case a fallback is needed. But it might be useful to have as a first pass, especially for use by the test suite.

On problem I have noticed, at least on Cygwin, is that setlocale() actually behaves differently depending on whether or not one compiles with libintl 😢. That's because libintl has its own wrapper around setlocale() with different behavior.

I'm actually starting to wonder if we should put together a configure check

I think we should limit the use of configure for setting the platform type instead and hard code the specific expectations for a platform in test_c_locale_coercion.py. The platform type is needed elsewhere.

For example Android had an entry in the sysconfig dictionary at one time (issue 27442) which has been replaced later with the new sys.getandroidapilevel() function (issue 28740) to modify _bootlocale.py that needs to know very early in the startup sequence whether the platform is Android (issue 28596). Knowing that the platform is Android is also needed in many tests and in subprocess.py because its shell is /system/bin/sh.

This test still fails on Cygwin because it assumes that by default the locale will be "C"

@embray what are the test results if you replace if test.support.is_android: with if True: in test_c_locale_coercion.py with the current implementation of this PR already applied ?

@xdegaye That fails all the same on Cygwin. The difference between Cygwin and Android here are different. On Cygwin the meaning of the "C" locale is as in POSIX--it means ASCII encoding. The only issue on Cygwin is that the system default locale is "C.UTF-8" instead of just "C" as the tests currently assume.

In other words, _Py_LegacyLocaleDetected() is returning 0 on Cygwin in cases where the tests assume it returns 1.

@xdegaye @embray I'm going to merge this once the current CI run finishes (so I can build a patch for https://bugs.python.org/issue30672 on top of it), but I'll leave the tracker issue as "pending" until you've confirmed that this works as intended on Cygwin and Android

Although it would help not to overwrite the Android fixes when rebasing the patch :)

So change of plan: I'll hold off on merging until you've both given the thumbs up. If it looks good, @xdegaye can merge it, otherwise I'll deal with it after I get back from NZ.

@ncoghlan, @embray

The test_c_locale_coercion.log file is the output of running test_c_locale_coercion on Android.

test_c_locale_coercion is successful when the android-bpo-32002.diff.txt patch is applied on top of Nick last commit (It has a .txt extension to trick github into accepting it as an attachment).

I still get the following failures on Cygwin:

======================================================================
FAIL: test_LC_ALL_set_to_C (test.test_c_locale_coercion.LocaleCoercionTests) (default_locale=True, PYTHONCOERCECLOCALE='warn')
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/embray/src/python/cpython/Lib/test/test_c_locale_coercion.py", line 344, in _check_c_locale_coercion
    _coercion_expected)
  File "/home/embray/src/python/cpython/Lib/test/test_c_locale_coercion.py", line 235, in _check_child_encoding_details
    self.assertEqual(stderr_lines, expected_warnings)
AssertionError: Lists differ: ['Python runtime initialized with LC_CTYPE[192 chars]ed.'] != []

First list contains 1 additional elements.
First extra element 0:
'Python runtime initialized with LC_CTYPE=C (a locale with default ASCII encoding), which may cause Unicode compatibility problems. Using C.UTF-8, C.utf8, or UTF-8 (if available) as alternative Unicode-compatible locales is recommended.'

+ []
- ['Python runtime initialized with LC_CTYPE=C (a locale with default ASCII '
-  'encoding), which may cause Unicode compatibility problems. Using C.UTF-8, '
-  'C.utf8, or UTF-8 (if available) as alternative Unicode-compatible locales is '
-  'recommended.']

======================================================================
FAIL: test_PYTHONCOERCECLOCALE_set_to_zero (test.test_c_locale_coercion.LocaleCoercionTests) (default_locale=True, PYTHONCOERCECLOCALE='0')
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/embray/src/python/cpython/Lib/test/test_c_locale_coercion.py", line 344, in _check_c_locale_coercion
    _coercion_expected)
  File "/home/embray/src/python/cpython/Lib/test/test_c_locale_coercion.py", line 232, in _check_child_encoding_details
    self.assertEqual(encoding_details, expected_details)
AssertionError: {'fsencoding': 'utf-8', 'stdin_info': 'utf-8:surrogateesc[123 chars]: ''} != {'fsencoding': 'ascii', 'stdin_info': 'ascii:surrogateesc[123 chars]: ''}
- {'fsencoding': 'utf-8',
?                 ^^^^^

+ {'fsencoding': 'ascii',
?                 ^^^^^

   'lang': '',
   'lc_all': '',
   'lc_ctype': '',
-  'stderr_info': 'utf-8:backslashreplace',
?                  ^^^^^

+  'stderr_info': 'ascii:backslashreplace',
?                  ^^^^^

-  'stdin_info': 'utf-8:surrogateescape',
?                 ^^^^^

+  'stdin_info': 'ascii:surrogateescape',
?                 ^^^^^

-  'stdout_info': 'utf-8:surrogateescape'}
?                  ^^^^^

+  'stdout_info': 'ascii:surrogateescape'}
?                  ^^^^^

But +1 to going ahead and merging, if you want, as this is significantly cleaned up, and I can look into addressing the final Cygwin-related bits on top of this.

Merged! Thanks for the reviews and updates, folks :)