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

[Alexander Richardson]

> On Feb. 17, 2014, 7:39 a.m., Matthew John Dawson wrote:
> > autotests/kconfig_compiler/kconfigcompiler_test_signals.cpp, line 26
> > <https://git.reviewboard.kde.org/r/115634/diff/1/?file=243130#file243130line26>
> >
> >     Is this a copy + paste error?

Yes, leftover from old version of this unittest


> On Feb. 17, 2014, 7: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.

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


> On Feb. 17, 2014, 7: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.

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


- Alexander


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


On Feb. 11, 2014, 1:47 a.m., Alexander Richardson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115634/
> -----------------------------------------------------------
> 
> (Updated Feb. 11, 2014, 1:47 a.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/20140217/a5a447b6/attachment-0001.html>