gnucash master: Multiple changes pushed

John Ralls jralls at ceridwen.us
Tue Feb 21 10:07:58 EST 2017


> On Feb 21, 2017, at 6:17 AM, Geert Janssens <geert.gnucash at 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



More information about the gnucash-devel mailing list