Review Request 129519: Improve autotests for kpackage

Marco Martin notmart at gmail.com
Mon Nov 21 12:54:12 UTC 2016



> On Nov. 21, 2016, 12:39 p.m., Marco Martin wrote:
> > autotests/querytest.h, line 34
> > <https://git.reviewboard.kde.org/r/129519/diff/1/?file=486358#file486358line34>
> >
> >     +1 for testing more stuff, not particularly liking uniting those 2 functions, as a failed test message would be more vague.
> >     
> >     it would be better even duplicate some code between the 2 methods if you need to, but keeping the install and query methods separate
> 
> Bhushan Shah wrote:
>     I don't see reason for keeping ::install function, as there is no verification done on if package was installed or not.. so that function just serves as simple helper and is not "unittest" at all. If there is failure it would point to proper Location in file which wouldn't really be vague.

well, there should be a test in it ;)

but yeah, go ahead and push as is an improvement already


- Marco


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


On Nov. 21, 2016, 12:34 p.m., Bhushan Shah wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129519/
> -----------------------------------------------------------
> 
> (Updated Nov. 21, 2016, 12:34 p.m.)
> 
> 
> Review request for Plasma, Aleix Pol Gonzalez and Marco Martin.
> 
> 
> Repository: kpackage
> 
> 
> Description
> -------
> 
> There are multiple changes in this,
> 
> a) Previously it used to install all packages in one go and then just verify number. now this tries to do it step by step.
> b) Added testcase for invalid metadata
> c) This tries to install the package with dependencies specified, which as expected would fail. One would need to mock dep resolver for testcases. (I will address this in next patch)
> 
> 
> Diffs
> -----
> 
>   autotests/data/testinvalidmetadata/contents/ui/main.qml PRE-CREATION 
>   autotests/data/testinvalidmetadata/metadata.json PRE-CREATION 
>   autotests/data/testpackagesdep/metadata.json a634d9d 
>   autotests/querytest.h 6628eff 
>   autotests/querytest.cpp 7765c7f 
> 
> Diff: https://git.reviewboard.kde.org/r/129519/diff/
> 
> 
> Testing
> -------
> 
> make test works, and coverage is also bit up now.
> 
> 
> Thanks,
> 
> Bhushan Shah
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20161121/52a9b47e/attachment.html>


More information about the Plasma-devel mailing list