Review Request 124779: Some unit tests for classes that I've written / refactored

[Laszlo Kis-Adam]

> On Aug. 17, 2015, 2:23 p.m., Kevin Funk wrote:
> > shell/tests/test_filteredproblemstore.cpp, line 80
> > <https://git.reviewboard.kde.org/r/124779/diff/1/?file=395249#file395249line80>
> >
> >     Is this really needed?
> 
> Laszlo Kis-Adam wrote:
>     QCOMPARE simply returns, so it cannot be used in a function that retuns a value (bool for example). So yes, it's neccessary.
> 
> Kevin Funk wrote:
>     Your test-methods should use void. There should be no branching in unit test code. => MYCOMPARE could be dropped
> 
> Laszlo Kis-Adam wrote:
>     That will lead to lots of repeated code...
> 
> Kevin Funk wrote:
>     Hm? Not sure I'm missing something; I'm just asking you to turn `bool checkFoo(...)` into `void checkFoo(...)` (and use QCOMPARE instead of MYCOMPARE inside `checkFoo`). Does that lead to code duplication?
> 
> Laszlo Kis-Adam wrote:
>     That way the testcase doesn't get interrupted if the check fails (since it only returns from checkFoo, not the testcase). So to make it interrupt the testcase I'd have to copypasta the code in checkFoo.
> 
> Kevin Funk wrote:
>     It *should* interrupt if the first assumption is wrong. If a an assumption is wrong, then any following failing assumption (which is based on the preceeding assumption (cf. `QVERIFY(ptr1); QCOMPARE(ptr1, ptr2)`)) has no informative value.
>     
>     You only do that in external functions, so it's not too bad. Still, I'm not considering it "best-style". (I don't have a too-strong opinion on that, so whatever...)

Well since it only returns from the external function it doesn't interrupt the test case. That's the entire point...


- Laszlo


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


On Aug. 18, 2015, 12:25 a.m., Laszlo Kis-Adam wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124779/
> -----------------------------------------------------------
> 
> (Updated Aug. 18, 2015, 12:25 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> Tests for the following classes:
> * DetectedProblem
> * ProblemStoreNode
> * ProblemStore
> * FilteredProblemStore
> * ProblemModel
> * ProblemModelSet
> * ProblemsView
> 
> 
> Diffs
> -----
> 
>   plugins/problemreporter/CMakeLists.txt e179232 
>   plugins/problemreporter/tests/CMakeLists.txt PRE-CREATION 
>   plugins/problemreporter/tests/test_problemsview.cpp PRE-CREATION 
>   shell/filteredproblemstore.h d6cc909 
>   shell/filteredproblemstore.cpp a27b9ac 
>   shell/problemstore.h 97f34f3 
>   shell/problemstore.cpp 4b114b2 
>   shell/tests/CMakeLists.txt 6b18470 
>   shell/tests/test_detectedproblem.cpp PRE-CREATION 
>   shell/tests/test_filteredproblemstore.cpp PRE-CREATION 
>   shell/tests/test_problemmodel.cpp PRE-CREATION 
>   shell/tests/test_problemmodelset.cpp PRE-CREATION 
>   shell/tests/test_problemstore.cpp PRE-CREATION 
>   shell/tests/test_problemstorenode.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124779/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Laszlo Kis-Adam
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20150817/ffb262e5/attachment.html>