Review Request 115634: Add kconfig_compiler autotest that checks whether signals are emitted

[Matthew John Dawson]

> On Feb. 17, 2014, 1:39 a.m., Matthew John Dawson wrote:
> > autotests/kconfig_compiler/kconfigcompiler_test_signals.cpp, line 153
> > <https://git.reviewboard.kde.org/r/115634/diff/1/?file=243130#file243130line153>
> >
> >     As this appears to preform the same check as the function below, I rather just have the one set of tests.  As the previous test breaks up the different elements under test such that Qt sees the different tests, I prefer for that version to be used.
> 
> Alexander Richardson wrote:
>     Doesn't quite do the same thing. This test checks that signals get emitted when using the generated setters. The other test uses the reflective approach with setProperty().
>     
>     This test actually mostly works before https://git.reviewboard.kde.org/r/115635/ except for the last check with setDefaults().
>     The second test function only starts working after the patch to kconfig_compiler

Ah, I see.  Can you merge the two sets of tests, so that they both use the data driven system?  I just prefer the output Qt generates when that is used.


> On Feb. 17, 2014, 1:39 a.m., Matthew John Dawson wrote:
> > autotests/kconfig_compiler/kconfigcompiler_test_signals.cpp, line 226
> > <https://git.reviewboard.kde.org/r/115634/diff/1/?file=243130#file243130line226>
> >
> >     As the solution for this problem ends up being a wrapper that works for every type that correctly implements the KConfigSkeletonItem contract, I don't think we need to test every possible type.  Did you experience any particular issues with the other types?  Otherwise I rather just test one type, which should help speed up the test suite.
> 
> Alexander Richardson wrote:
>     Do you mean every type of generated KConfigSkeleton (dpointer vs no dpointer and singleton vs no singleton) or every type of stored data (QString vs int, etc).
>     
>     I actually found a bug in kconfig_compiler due to testing every type: https://projects.kde.org/projects/frameworks/kconfig/repository/revisions/b1487f0c6b4ea64ae9c1ce96348fce8b5f0828df
>     
>     But in general there should be no difference depending on what gets saved. Maybe there should be another unit test that generates a class that uses config items of every possible type to make sure something like this does not happen and restrict the signals test to a single property. Don't really which solution gets chosen, but I guess I can then finally get rid of that ugly macro in testSetters

Sorry, I meant types of data stored.  I think reducing this down to one single data type is best, as it avoids testing too many different ideas in one set of tests.  In the future, it definitely would be good to have all the different types tested.

I think it is good to test the different ways KConfigSekeleton is generated, as the type does change how signals are generated.


- Matthew John


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115634/#review50007
-----------------------------------------------------------


On Feb. 10, 2014, 7:47 p.m., Alexander Richardson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115634/
> -----------------------------------------------------------
> 
> (Updated Feb. 10, 2014, 7:47 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kconfig
> 
> 
> Description
> -------
> 
> Add kconfig_compiler autotest that checks whether signals are emitted
> 
> Currently this works when using the setters, however when using
> setProperty() on the KCoreConfigSkeleton* (as done by KConfigDialog) no
> signals are emitted.
> 
> 
> Diffs
> -----
> 
>   autotests/kconfig_compiler/CMakeLists.txt a2ebb9453bacb2c7507bc9477b6753a34bbcd434 
>   autotests/kconfig_compiler/kconfigcompiler_test_signals.cpp PRE-CREATION 
>   autotests/kconfig_compiler/signals_test.kcfg PRE-CREATION 
>   autotests/kconfig_compiler/signals_test_no_singleton.kcfgc PRE-CREATION 
>   autotests/kconfig_compiler/signals_test_no_singleton_dpointer.kcfgc PRE-CREATION 
>   autotests/kconfig_compiler/signals_test_singleton.kcfgc PRE-CREATION 
>   autotests/kconfig_compiler/signals_test_singleton_dpointer.kcfgc PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/115634/diff/
> 
> 
> Testing
> -------
> 
> Compiles, tests fail until https://git.reviewboard.kde.org/r/115635/ is applied, then they pass.
> 
> Rather ugly code IMO, open for suggestions to improve it...
> 
> 
> Thanks,
> 
> Alexander Richardson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140218/697a3513/attachment-0001.html>