Fwd: gnucash master: Add failing unittest for aqbanking lookup of trans_retrieval date.

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Fwd: gnucash master: Add failing unittest for aqbanking lookup of trans_retrieval date.

Christian Stimming-4
Hi John,

recently I noticed the "trans-retrieval" slot in the aqbanking account doesn't
remember its value.

I now added a unittest that indeed shows that the getter function from import-
export/aqb/gnc-ab-kvp.h doesn't return the same value that I've inserted in
the setter. However, your unittest in test-engine-kvp-properties.c indicated
that the "ab-trans-retrieval" property with the GNC_TYPE_TIMESPEC type works
fine.

Do you happen to have any idea why it doesn't work fine when called by the
gnc-ab-kvp.[hc] functions, even though your test in test-engine-kvp-properties
seemed to work? Thanks a lot in advance!

Best Regards,

Christian


----------  Weitergeleitete Nachricht  ----------

Betreff: gnucash master: Add failing unittest for aqbanking lookup of
trans_retrieval date.
Datum: Sonntag, 7. September 2014, 17:03:13
Von: Christian Stimming <[hidden email]>
An: [hidden email]

Updated via  https://github.com/Gnucash/gnucash/commit/67155158 (commit)
        from  https://github.com/Gnucash/gnucash/commit/80aa327a (commit)



commit 671551585eb636554f955f4ed095d5d8106bdd8d
Author: Christian Stimming <[hidden email]>
Date:   Sun Sep 7 22:58:07 2014 +0200

    Add failing unittest for aqbanking lookup of trans_retrieval date.
   
    The lookup of the "ab-trans-retrieval" property somehow fails to
    return the correct value. This is somewhat surprising as this
    property is already checked in the test-engine-kvp-properties.c and
    there it works fine. What's the problem here...?



Summary of changes:
 .../aqb/test/file-book-hbcislot.gnucash            | 39 +++++++++++++++++
 src/import-export/aqb/test/test-kvp.c              | 49
++++++++++++++++++++++
 2 files changed, 88 insertions(+)

_______________________________________________
gnucash-patches mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-patches
-------------------------------------------------------------
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: gnucash master: Add failing unittest for aqbanking lookup of trans_retrieval date.

John Ralls-2

On Sep 7, 2014, at 2:06 PM, Christian Stimming <[hidden email]> wrote:

> Hi John,
>
> recently I noticed the "trans-retrieval" slot in the aqbanking account doesn't
> remember its value.
>
> I now added a unittest that indeed shows that the getter function from import-
> export/aqb/gnc-ab-kvp.h doesn't return the same value that I've inserted in
> the setter. However, your unittest in test-engine-kvp-properties.c indicated
> that the "ab-trans-retrieval" property with the GNC_TYPE_TIMESPEC type works
> fine.
>
> Do you happen to have any idea why it doesn't work fine when called by the
> gnc-ab-kvp.[hc] functions, even though your test in test-engine-kvp-properties
> seemed to work? Thanks a lot in advance!

Christian,

Yeah, fixed and pushed, e210f8c.

The difference was that your test used the defective accessor function and mine used qof_instance_get directly with the right parameter type (Timespec ** instead of Timespec *).

Regards,
John Ralls


_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: gnucash master: Add failing unittest for aqbanking lookup of trans_retrieval date.

Christian Stimming-4
Zitat von John Ralls <[hidden email]>:

>> recently I noticed the "trans-retrieval" slot in the aqbanking  
>> account doesn't
>> remember its value.
>
> Christian,
>
> Yeah, fixed and pushed, e210f8c.
>
> The difference was that your test used the defective accessor  
> function and mine used qof_instance_get directly with the right  
> parameter type (Timespec ** instead of Timespec *).

Ah, the wonders of zero compile-time checks of variable argument lists in C...

Thanks a lot for fixing this!

Regards,

Christian

_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: gnucash master: Add failing unittest for aqbanking lookup of trans_retrieval date.

John Ralls-2

On Sep 8, 2014, at 5:31 AM, Christian Stimming <[hidden email]> wrote:

> Zitat von John Ralls <[hidden email]>:
>>> recently I noticed the "trans-retrieval" slot in the aqbanking account doesn't
>>> remember its value.
>>
>> Christian,
>>
>> Yeah, fixed and pushed, e210f8c.
>>
>> The difference was that your test used the defective accessor function and mine used qof_instance_get directly with the right parameter type (Timespec ** instead of Timespec *).
>
> Ah, the wonders of zero compile-time checks of variable argument lists in C...
>
> Thanks a lot for fixing this!
>

It's not C's fault; C compiler's know the difference between pointers and pointers-to-pointers and how to raise type errors. It's GValue's conversion of everything to void* that defeats the type checking.

Regards,
John Ralls


_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: gnucash master: Add failing unittest for aqbanking lookup of trans_retrieval date.

Derek Atkins
John Ralls <[hidden email]> writes:

> On Sep 8, 2014, at 5:31 AM, Christian Stimming <[hidden email]> wrote:
>
>> Zitat von John Ralls <[hidden email]>:
>>>> recently I noticed the "trans-retrieval" slot in the aqbanking
>>>> account doesn't
>>>> remember its value.
>>>
>>> Christian,
>>>
>>> Yeah, fixed and pushed, e210f8c.
>>>
>>> The difference was that your test used the defective accessor
>>> function and mine used qof_instance_get directly with the right
>>> parameter type (Timespec ** instead of Timespec *).
>>
>> Ah, the wonders of zero compile-time checks of variable argument lists in C...
>>
>> Thanks a lot for fixing this!
>>
>
> It's not C's fault; C compiler's know the difference between pointers
> and pointers-to-pointers and how to raise type errors. It's GValue's
> conversion of everything to void* that defeats the type checking.

Yeah, this will all get fixed in C++ :)

gpointer sucks :(

> Regards,
> John Ralls

-derek

--
       Derek Atkins, SB '93 MIT EE, SM '95 MIT Media Laboratory
       Member, MIT Student Information Processing Board  (SIPB)
       URL: http://web.mit.edu/warlord/    PP-ASEL-IA     N1NWH
       [hidden email]                        PGP key available
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: gnucash master: Add failing unittest for aqbanking lookup of trans_retrieval date.

Christian Stimming-4
In reply to this post by John Ralls-2
Am Montag, 8. September 2014, 07:17:47 schrieb John Ralls:
> On Sep 8, 2014, at 5:31 AM, Christian Stimming <[hidden email]>
wrote:

> > Zitat von John Ralls <[hidden email]>:
> >>> recently I noticed the "trans-retrieval" slot in the aqbanking account
> >>> doesn't remember its value.
> >>
> >> Christian,
> >>
> >> Yeah, fixed and pushed, e210f8c.
> >>
> >> The difference was that your test used the defective accessor function
> >> and mine used qof_instance_get directly with the right parameter type
> >> (Timespec ** instead of Timespec *).>
> > Ah, the wonders of zero compile-time checks of variable argument lists in
> > C...
> >
> > Thanks a lot for fixing this!
>
> It's not C's fault; C compiler's know the difference between pointers and
> pointers-to-pointers and how to raise type errors. It's GValue's conversion
> of everything to void* that defeats the type checking.

Err... no, it is indeed C's fault: The function in question, qof_instance_get
and set, uses a variable argument list (variadic arguments), terminated by the
NULL pointer in the call, and this variable arguments are not at all compile-
time checked. Using qof_instance_get is what you inserted here in
272655b60c0e30726, including the bug you fortunately now fixed. If there is
another interface that gives us some more compile time checking, feel free to
propose it here and/or in the docs of qof_instance_get. Or is this just a
fundamental (bad) property of g_object's property system that we can't avoid?

Best Regards,

Christian
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: gnucash master: Add failing unittest for aqbanking lookup of trans_retrieval date.

John Ralls-2

On Sep 8, 2014, at 11:16 AM, Christian Stimming <[hidden email]> wrote:

> Am Montag, 8. September 2014, 07:17:47 schrieb John Ralls:
>> On Sep 8, 2014, at 5:31 AM, Christian Stimming <[hidden email]>
> wrote:
>>> Zitat von John Ralls <[hidden email]>:
>>>>> recently I noticed the "trans-retrieval" slot in the aqbanking account
>>>>> doesn't remember its value.
>>>>
>>>> Christian,
>>>>
>>>> Yeah, fixed and pushed, e210f8c.
>>>>
>>>> The difference was that your test used the defective accessor function
>>>> and mine used qof_instance_get directly with the right parameter type
>>>> (Timespec ** instead of Timespec *).>
>>> Ah, the wonders of zero compile-time checks of variable argument lists in
>>> C...
>>>
>>> Thanks a lot for fixing this!
>>
>> It's not C's fault; C compiler's know the difference between pointers and
>> pointers-to-pointers and how to raise type errors. It's GValue's conversion
>> of everything to void* that defeats the type checking.
>
> Err... no, it is indeed C's fault: The function in question, qof_instance_get
> and set, uses a variable argument list (variadic arguments), terminated by the
> NULL pointer in the call, and this variable arguments are not at all compile-
> time checked. Using qof_instance_get is what you inserted here in
> 272655b60c0e30726, including the bug you fortunately now fixed. If there is
> another interface that gives us some more compile time checking, feel free to
> propose it here and/or in the docs of qof_instance_get. Or is this just a
> fundamental (bad) property of g_object's property system that we can't avoid?

It’s a fundamental bad property of variadic functions in general, but if the variadic function was calling a strongly-typed function (inside GObject, since qof_instance_get just wraps g_object_get) the compiler would have errored out. Since the GValue functions take void*, the compiler didn’t have a chance.

The interface which gives more compile-time checking uses templates instead of void*, at the expense of some really horrendous compiler error messages. Boosters of the new concepts feature due in C++17 and available in boost claim that it fixes the compiler error problem, but I haven’t tried it yet to see if they’re right.

I think replacing GObject properties and GValues will be part of converting the primary database classes to C++.

Regards,
John Ralls



_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel