test: Minor fix in test - locale in terminal #28286
Conversation
Running tests with non-English locale set in Linux terminal makes system tests (system_tests/run_command') fail. Setting the locale env variable 'LC_ALL' in failing test explicitly to 'C' fixes the failure.
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
During the workshop wit Gloria @glozow on Dev/Hack/Day (part of BTC Prague conference) my tests failed. Later that day I found what the problem was and now I am finally presenting my fix as an attempt to make my first PR in this repo. |
|
Concept ACK, though I'm unable to reproduce this on macOS, which makes sense per // On most POSIX systems (e.g. Linux, but not BSD) the environment's locale
// may be invalid, in which case the "C.UTF-8" locale is used as fallback.
#if !defined(WIN32) && !defined(MAC_OSX) && !defined(__FreeBSD__) && !defined(__OpenBSD__) && !defined(__NetBSD__)
try {
std::locale(""); // Raises a runtime error if current locale is invalid
} catch (const std::runtime_error&) {
setenv("LC_ALL", "C.UTF-8", 1);
} |
| @@ -79,6 +79,7 @@ BOOST_AUTO_TEST_CASE(run_command) | |||
| const std::string command{"cmd.exe /c dir nosuchfile"}; | |||
| const std::string expected{wine_runtime ? "File not found." : "File Not Found"}; | |||
| #else | |||
| setenv("LC_ALL", "C", 1); // explicitly set locale env to get error message in English | |||
| const std::string command{"ls nosuchfile"}; | |||
There was a problem hiding this comment.
lgtm, but if the file exists, this will also fail.
Might be better to use a pure command that does not depend on the env vars and filesystem state
|
🐙 This pull request conflicts with the target branch and needs rebase. |
|
Apologies for the slow review on this, and it eventually going stale. I'll close for now, but the issue will still need a fix. Ideally like #28286 (comment) Anyone is welcome to open a new pull request with the fix. |
Running tests with non-English locale set in Linux terminal makes system tests (
system_tests/run_command) fail. Setting the locale in failing test explicitly to 'C' fixes the failure.How to reproduce test failure:
Setting LC_ALL=cs_CZ.UTF-8 (
export LC_ALL=cs_CZ.UTF-8) in terminal and then runningmake checkshould be enough.Fix:
My terminal is by default set to non-English locale and the test failed because it checks for English error message, but the terminal produces error message in the language set by locale. Then I manually set
export LC_ALL=Cin my terminal and all tests passed. So I explicitly set env variable 'LC_ALL' to 'C' in the code of failing test.