[
https://issues.apache.org/jira/browse/THRIFT-6194?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer resolved THRIFT-6194.
--------------------------------
Fix Version/s: 0.25.0
Assignee: Jens Geyer
Resolution: Fixed
> ToStringTest leaves the global locale set for the rest of the UnitTests binary
> ------------------------------------------------------------------------------
>
> Key: THRIFT-6194
> URL: https://issues.apache.org/jira/browse/THRIFT-6194
> Project: Thrift
> Issue Type: Bug
> Components: C++ - Library
> Reporter: Jens Geyer
> Assignee: Jens Geyer
> Priority: Minor
> Fix For: 0.25.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> h2. What happens
> {{lib/cpp/test/ToStringTest.cpp}} has two cases that call
> {{std::locale::global()}} and never
> restore it:
> {code:cpp}
> BOOST_AUTO_TEST_CASE(locale_en_US_int_to_string) {
> std::locale::global(std::locale("en_US.UTF-8"));
> BOOST_CHECK_EQUAL(to_string(1000000), "1000000");
> }
> BOOST_AUTO_TEST_CASE(locale_de_DE_floating_point_to_string) {
> std::locale::global(std::locale("de_DE.UTF-8"));
> ...
> }
> {code}
> 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 {{std::ostringstream}} then come out with digit grouping.
> h2. Why it is worth fixing
> It has already cost one debugging session. A test in {{THttpBufferBoundTest}}
> formats a chunk
> size through an {{ostringstream}}:
> {code:cpp}
> std::ostringstream size;
> size << std::hex << 4096; // expected "1000"
> {code}
> which produced {{1.000}} instead. That 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 was run
> alone or as part of a full run -- {{ToStringTest}} is disabled by default and
> only runs when the
> whole binary does.
> An order-dependent test result whose cause lives in an unrelated file is
> expensive to chase, and
> the next test in this binary that formats a number will hit it again.
> h2. Fix
> A small RAII guard in the test file that saves the previous global locale and
> restores it on
> scope exit, applied to both cases. The test intent is unchanged -- the locale
> is still set for
> the duration of each case.
> h2. Verified
> The fix was checked by removing the defensive
> {{imbue(std::locale::classic())}} that
> {{THttpBufferBoundTest}} had to add for its own protection, and confirming
> its chunked case then
> passes a full run anyway. Before the fix it failed there and passed in
> isolation.
> Test-only; no shipped code is affected, which is why this is filed as Minor.
> _Filed with AI assistance (Claude Opus 5); reviewed and filed by Jens Geyer._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
