organicmaps/organicmaps-tmp
Archived
13
Fork 1
Code Issues 2.7k Pull requests 182 Projects Releases 65 Wiki Activity Actions

[tests] Fix distance formatting tests using system locale #6470

Open
gpesquero wants to merge 1 commit from gpesquero/test_distance into master
pull from: gpesquero/test_distance
merge into: organicmaps:master
Conversation 0 Commits 1 Files changed 5 +105 -21
5 changed files with 105 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
platform/platform_tests/CMakeLists.txt
View file

@ -10,6 +10,7 @@ set(SRC
jansson_test.cpp
language_test.cpp
local_country_file_tests.cpp
locale_utils.cpp
location_test.cpp
measurement_tests.cpp
platform_test.cpp

22
platform/platform_tests/distance_tests.cpp
View file

@ -2,8 +2,12 @@
#include "platform/distance.hpp"
#include "platform/measurement_utils.hpp"
#include "platform/platform_tests/locale_utils.hpp"
#include "platform/settings.hpp"
#include <iostream>
#include <string>
namespace platform
{
std::string MakeDistanceStr(std::string const & value, std::string const & unit)
@ -65,6 +69,8 @@ UNIT_TEST(Distance_CreateFormatted)
UNIT_TEST(Distance_CreateAltitudeFormatted)
{
std::string groupSep = GetSystemGroupSeparator();
{
ScopedSettings const guard(measurement_utils::Units::Metric);
@ -78,12 +84,12 @@ UNIT_TEST(Distance_CreateAltitudeFormatted)
{
ScopedSettings const guard(measurement_utils::Units::Metric);
TEST_EQUAL(Distance::FormatAltitude(12345), MakeDistanceStr("12,345", "m"), ());
TEST_EQUAL(Distance::FormatAltitude(12345), MakeDistanceStr("12" + groupSep + "345", "m"), ());
}
{
ScopedSettings const guard(measurement_utils::Units::Imperial);
biodranik commented 2024-02-28 22:30:14 +00:00 (Migrated from github.com)
Review
Copy link

Is there an option to set/override the locale from the test, so we can check different separators for different locales explicitly?

Is there an option to set/override the locale from the test, so we can check different separators for different locales explicitly?
gpesquero commented 2024-03-03 15:39:14 +00:00
Review
Copy link

Hi @biodranik: I'm afraid that with the current implementation, we may not be able to test the Distance::FormatAltitude() function for different locales.

The main reason is that this function calls the measurement_utils::ToStringPrecision() function, which uses a static variable to store the locale to use. This locale is therefore updated only in the first iteration, so if we change the locale during the tests, it won't have any effect.

We could change the behavior of the measurement_utils::ToStringPrecision() function by removing this static variable, but that would lead to a worse runtime performance.

Hi @biodranik: I'm afraid that with the current implementation, we may not be able to test the `Distance::FormatAltitude()` function for different locales. The main reason is that this function calls the `measurement_utils::ToStringPrecision()` function, which uses a static variable to store the locale to use. This locale is therefore updated only in the first iteration, so if we change the locale during the tests, it won't have any effect. We could change the behavior of the `measurement_utils::ToStringPrecision()` function by removing this static variable, but that would lead to a worse runtime performance.
gpesquero commented 2024-03-21 23:27:10 +00:00
Review
Copy link

@biodranik , @vng: PTAL

@biodranik , @vng: PTAL
TEST_EQUAL(Distance::FormatAltitude(10000), MakeDistanceStr("32,808", "ft"), ());
TEST_EQUAL(Distance::FormatAltitude(10000), MakeDistanceStr("32" + groupSep + "808", "ft"), ());
}
}
@ -171,6 +177,8 @@ UNIT_TEST(Distance_To)
UNIT_TEST(Distance_ToPlatformUnitsFormatted)
{
std::string decSep = GetSystemDecimalSeparator();
{
ScopedSettings const guard(measurement_utils::Units::Metric);
@ -207,8 +215,8 @@ UNIT_TEST(Distance_ToPlatformUnitsFormatted)
TEST_EQUAL(newDistance.GetUnits(), Distance::Units::Miles, (d.ToString()));
TEST_ALMOST_EQUAL_ULPS(newDistance.GetDistance(), 6.8, (d.ToString()));
TEST_EQUAL(newDistance.GetDistanceString(), "6.8", (d.ToString()));
TEST_EQUAL(newDistance.ToString(), MakeDistanceStr("6.8", "mi"), (d.ToString()));
TEST_EQUAL(newDistance.GetDistanceString(), "6" + decSep + "8", (d.ToString()));
TEST_EQUAL(newDistance.ToString(), MakeDistanceStr("6" + decSep + "8", "mi"), (d.ToString()));
}
}
@ -340,6 +348,8 @@ UNIT_TEST(Distance_FormattedDistance)
{Distance(999'999, Units::Feet), 189, Units::Miles, "189", MakeDistanceStr("189", "mi")},
};
Locale loc = GetCurrentLocale();
// clang-format on
for (TestData const & data : testData)
{
@ -349,8 +359,8 @@ UNIT_TEST(Distance_FormattedDistance)
{
TEST_ALMOST_EQUAL_ULPS(d.GetDistance(), data.formattedDistance, (data.distance));
TEST_EQUAL(d.GetUnits(), data.formattedUnits, (data.distance));
TEST_EQUAL(d.GetDistanceString(), data.formattedDistanceString, (data.distance));
TEST_EQUAL(d.ToString(), data.formattedString, (data.distance));
TEST_EQUAL(d.GetDistanceString(), LocalizeValueString(data.formattedDistanceString, loc), (data.distance));
TEST_EQUAL(d.ToString(), LocalizeValueString(data.formattedString, loc), (data.distance));
}
}
}

72
platform/platform_tests/locale_utils.cpp Normal file
View file

@ -0,0 +1,72 @@
#include "platform/locale.hpp"
#include <string>
using namespace platform;
namespace platform
{
std::string GetSystemDecimalSeparator()
{
Locale loc = GetCurrentLocale();
return loc.m_decimalSeparator;
}
std::string GetSystemGroupSeparator()
{
Locale loc = GetCurrentLocale();
return loc.m_groupingSeparator;
}
std::string ReplaceGroupingSeparators(std::string const & valueString, std::string const & groupingSeparator)
{
std::string out(valueString);
if (groupingSeparator == ",")
return out;
size_t pos;
while((pos = out.find(",")) != std::string::npos)
out.replace(pos, 1, groupingSeparator);
return out;
}
std::string ReplaceDecimalSeparator(std::string const & valueString, std::string const & decimalSeparator)
{
std::string out(valueString);
if (decimalSeparator == ".")
return out;
size_t pos = valueString.find(".");
if (pos != std::string::npos)
out.replace(pos, 1, decimalSeparator);
return out;
}
std::string LocalizeValueString(std::string const & valueString, Locale const & loc)
{
std::string out;
if (valueString.find(".") != std::string::npos)
biodranik commented 2024-03-22 00:12:32 +00:00 (Migrated from github.com)
Review
Copy link

This looks hacky. Why is the decimal point always replaced? Is the input always in the US locale?

Generally, can the code be simplified?

This looks hacky. Why is the decimal point always replaced? Is the input always in the US locale? Generally, can the code be simplified?
{
// String contains a value with decimal separator.
out = ReplaceDecimalSeparator(valueString, loc.m_decimalSeparator);
}
else if (valueString.find(",") != std::string::npos)
{
// String contains a value with grouping separator.
out = ReplaceGroupingSeparators(valueString, loc.m_groupingSeparator);
}
else
out = valueString;
return out;
}
}

9
platform/platform_tests/locale_utils.hpp Normal file
View file

@ -0,0 +1,9 @@
namespace platform
{
std::string GetSystemDecimalSeparator();
std::string GetSystemGroupSeparator();
std::string ReplaceGroupingSeparators(std::string const & valueString, std::string const & groupingSeparator);
std::string ReplaceDecimalSeparator(std::string const & valueString, std::string const & decimalSeparator);
std::string LocalizeValueString(std::string const & valueString, Locale const & loc);
}

22
platform/platform_tests/measurement_tests.cpp
View file

@ -1,6 +1,7 @@
#include "testing/testing.hpp"
#include "platform/measurement_utils.hpp"
#include "platform/platform_tests/locale_utils.hpp"
#include "base/math.hpp"
@ -10,17 +11,6 @@
using namespace measurement_utils;
using namespace platform;
std::string AddGroupingSeparators(std::string const & valueString, std::string const & groupingSeparator)
{
std::string out(valueString);
if (out.size() > 4 && !groupingSeparator.empty())
for (int pos = out.size() - 3; pos > 0; pos -= 3)
out.insert(pos, groupingSeparator);
return out;
}
UNIT_TEST(LatLonToDMS_Origin)
{
TEST_EQUAL(FormatLatLonAsDMS(0, 0, false), "00°00′00″ 00°00′00″", ());
@ -66,11 +56,13 @@ UNIT_TEST(FormatOsmLink)
UNIT_TEST(FormatSpeedNumeric)
{
std::string decSep = GetSystemDecimalSeparator();
TEST_EQUAL(FormatSpeedNumeric(10, Units::Metric), "36", ());
TEST_EQUAL(FormatSpeedNumeric(1, Units::Metric), "3.6", ());
TEST_EQUAL(FormatSpeedNumeric(1, Units::Metric), "3" + decSep + "6", ());
TEST_EQUAL(FormatSpeedNumeric(10, Units::Imperial), "22", ());
TEST_EQUAL(FormatSpeedNumeric(1, Units::Imperial), "2.2", ());
TEST_EQUAL(FormatSpeedNumeric(1, Units::Imperial), "2" + decSep + "2", ());
}
UNIT_TEST(OSMDistanceToMetersString)
@ -152,7 +144,7 @@ UNIT_TEST(ToStringPrecisionLocale)
double d2 = 12345.0;
int pr2 = 0;
std::string d2String("12345");
std::string d2String("12,345");
struct TestData
{
@ -180,6 +172,6 @@ UNIT_TEST(ToStringPrecisionLocale)
TEST_EQUAL(measurement_utils::ToStringPrecisionLocale(loc, d1, pr1), data.d1String, ());
TEST_EQUAL(measurement_utils::ToStringPrecisionLocale(loc, d2, pr2),
AddGroupingSeparators(d2String, loc.m_groupingSeparator), ());
ReplaceGroupingSeparators(d2String, loc.m_groupingSeparator), ());
}
}