Skip to content

WIP: CI: Pameterize locale tests specifically for Linux #44972

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.

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 28 commits into from

Conversation

mroeschke
Copy link
Member

Looking to eventually eliminate the local-specific builds by parametrizing in specific tests

@mroeschke mroeschke marked this pull request as draft December 19, 2021 23:14
@jreback jreback added the CI Continuous Integration label Dec 20, 2021
@burnpanck
Copy link
Contributor

What are the motivations which have led to considering to remove the locale-specific builds? The reason I ask is because I see two different reasons to test against multiple locales:

  1. Test intended locale-specific behaviour
  2. Prevent unintended locale-specific behaviour

The first category will probably benefit from this PR, as testing multiple locales becomes much faster and thus can be performed more frequently and efficiently. However, the second category will effectively lose coverage.

Specifically, I personally run on the locale de_CH, where multiple tests cause false positives. That makes impractical for me to run the test suite locally, so all my contributions are blind. I was about to open a PR to include de_CH in the test-set, as a basis to provide contributions to fix those tests. Should I do that anyway, or how would you like me to proceed with that issue?

On another note, just to double-check: The with_locale fixture and the tm.set_locale context manager it uses are neither thread-safe nor context safe. They set the locale process-wide. This could cause trouble in some parallel testing scenarios, although I believe pytest-xdist does spawn separate processes before running any test, so the sub-processes should have independent locales.

@burnpanck
Copy link
Contributor

See also #44625 and #45013.

@mroeschke
Copy link
Member Author

Currently pandas only tests a European locale it_IT and non-latin locale zh_CN on PY38 (in 2 separate builds)

This PR ensures we are testing these locales (also with en_US) on all our Linux PY38/39/3.10 builds so locale testing is being expanded not removed.

After this PR it would be easier in theory to tests more locales like de_CH. And pyetest-xdist runs tests in multiple processes so hopefully the thread safety concerns should not be an issue here.

@mroeschke mroeschke closed this Dec 31, 2021
@mroeschke mroeschke deleted the ci/locale_simplify branch February 21, 2022 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants