Jens-G opened a new pull request, #3804: URL: https://github.com/apache/thrift/pull/3804
[THRIFT-6194](https://issues.apache.org/jira/browse/THRIFT-6194) Two cases in `ToStringTest` call `std::locale::global()` and never put back what was there: ```cpp BOOST_AUTO_TEST_CASE(locale_de_DE_floating_point_to_string) { std::locale::global(std::locale("de_DE.UTF-8")); ... } ``` Setting the global locale is the *point* of these cases — they prove `to_string()` does not follow it, which is why `TToString.h` imbues a default locale on the streams it builds. Leaving it set afterwards is not the point. Every stream constructed later in the same `UnitTests` binary picks it up, in suites that have nothing to do with this one, and numbers formatted through an `ostringstream` come out with digit grouping. ### Why it is worth fixing It has already cost a debugging session. A case in `THttpBufferBoundTest` formats a chunk size: ```cpp std::ostringstream size; size << std::hex << 4096; // expected "1000" ``` and got `1.000`, which is not a hex chunk size — so the HTTP transport under test read a completely different stream, and the case passed or failed depending on whether it ran alone or as part of a full run. `ToStringTest` is disabled by default and only runs when the whole binary does, so the two modes disagreed. An order-dependent result whose cause lives in an unrelated file is expensive to chase, and the next test in this binary that formats a number would hit it again. ### The change A scoped guard that saves the previous global locale and restores it on exit, applied to both cases. What they assert is unchanged — the locale is still set for the duration of each case. ### Verified both ways The defensive `imbue(std::locale::classic())` that `THttpBufferBoundTest` had to add for its own protection was removed, and a full `UnitTests` run repeated with and without this fix: | | result | |---|---| | with this fix | chunked case passes | | without it | `chunks_are_bounded_by_their_sum_and_not_one_at_a_time` fails, `inner->served() < 2 * kMaxMessageSize` → `262772 >= 131072` | (The one unrelated failure in both runs is `TServerSocketTest/test_bind_to_address`, [THRIFT-6191](https://issues.apache.org/jira/browse/THRIFT-6191).) Test-only; no shipped code is affected. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
