Skip to content

TST: Fix locale test that fails in GitHub actions #29852

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

TST: Fix locale test that fails in GitHub actions #29852

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e3afa3d
TST: Fix locale test that fails in GitHub actions
datapythonista Nov 26, 2019
718be0d
Merge remote-tracking branch 'upstream/master' into locale_test
datapythonista Nov 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions pandas/tests/config/test_localization.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,18 @@ def test_set_locale():
# getlocale() returned (None, None).
pytest.skip("Current locale is not set.")

locale_override = os.environ.get("LOCALE_OVERRIDE", None)
locale_override = os.environ.get("LOCALE_OVERRIDE")

if locale_override is None:
if not locale_override:
lang, enc = "it_CH", "UTF-8"
elif locale_override == "C":
lang, enc = "en_US", "ascii"
elif len(locale_override.split(".")) != 2:
raise ValueError(
"Unknown format for LOCALE_OVERRIDE, "
"expected country_VARIANT.ENCODING "
f"(e.g. en_US.UTF-8), found {locale_override!r}"
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused why we're raising an error in a test instead of just skipping for example.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. This test is already failing if the locale got an incorrect format. I just wanted that when it fails, it provides a useful error, so we can fix the locale in the CI. An assert could make more sense, but raising made the code simpler.

I'm confused myself about how the localization works. Not even sure if it works, looking at https://github.com/pandas-dev/pandas/blob/master/ci/run_tests.sh#L13. And I have the feeling that we should simply test LANG and LC_ALL in the CI, and get rid of the LOCALE_OVERRIDE. But it's quite likely that I'm missing something.

In any case, didn't want to work on the localization, this PR is to prepare the tests to work on GitHub Actions, and the raise is an extra to have better error messages if something is wrong in the variable. I'm happy to get rid of it here, but then we may end up with a tuple unpacking error that will require extra debugging to understand what's going on.

else:
lang, enc = locale_override.split(".")

Expand Down