> On Feb 21, 2017, at 6:17 AM, Geert Janssens <geert.gnuc...@kobaltwit.be>
> wrote:
>
> Hi John,
>
> Op maandag 20 februari 2017 19:06:19 CET schreef John Ralls:
>
>> @@ -246,7 +246,7 @@ inline GncRational operator+(GncRational a, GncInt128 b)
>> }
>> inline GncRational operator+(GncInt128 a, GncRational b)
>> {
>> - return b + GncRational(a, 1);
>> + return GncRational(a, 1) + a;
>> }
>
> Looks like one more copy/paste error or typo...
> return GncRational(a, 1) + *b*;
>
> It's weird this isn't caught in our unit tests (unless this is not
> specifically tested for yet).
Darn. No, the unit tests test the class operator methods not the overloads.
>
>
>> diff --git a/src/libqof/qof/test/gtest-gnc-numeric.cpp
>> b/src/libqof/qof/test/gtest-gnc-numeric.cpp index 9ba7a68..34f56b1 100644
>> --- a/src/libqof/qof/test/gtest-gnc-numeric.cpp
>> +++ b/src/libqof/qof/test/gtest-gnc-numeric.cpp
>> @@ -211,6 +211,16 @@ TEST(gncnumeric_stream, output_stream)
>> GncNumeric rational_string(123, 456);
>> output << rational_string;
>> EXPECT_EQ("123/456", output.str());
>> + output.imbue(std::locale("de_DE"));
>
> I was going to suggest using fr_FR here instead of de_DE, because the latter
> is not installed by
> default on travis and we explicitly install the former. However I also see
> you are excluding this
> test from travis due to bugs in gcc 4.8 in your followup commit.
>
> I was curious enough to run a couple of tests on this and these are my
> findings:
> - I had to include the typeinfo header in gnc-numeric.hpp or it wouldn't
> compile due to
> std::bad_cast not being defined. With the typeinfo header included it builds,
> however tests still
> fail
> - Using "fr_FR" instead of de_DE also fails with "invalid locale name",
> however the function does
> accept "fr_FR.utf8"
>
>> + output.str("");
>> + output << simple_int;
>> + EXPECT_EQ("123456", output.str());
>
> - Finally the French locale uses a thousands separator, so the expected
> output for a simple
> integer should have one as well
>
> I have pushed these changes to my personal github repo [1] (after reverting
> your last commit)
> and with this travis completes all tests without error [2]
>
> I'm holding off to pushing the same to the gnucash repo until you had a
> chance to provide your
> thoughts on this.
Interesting. I didn't get that particular failure on Travis; I got it on my
local Ubuntu 14.04 VM which I use to debug Travis failures. It didn't even
occur to me that the exception (which isn't the one the standard requires and
which it doesn't throw when one compiles with -O0 -g) was caused by an
uninstalled language.
Yes, please do push it.
Regards,
John Ralls
_______________________________________________
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
