On 9/27/19 5:32 PM, Daniel Henrique Barboza wrote:


On 9/27/19 5:33 PM, Cole Robinson wrote:
On 9/26/19 10:56 AM, Daniel Henrique Barboza wrote:
Signed-off-by: Daniel Henrique Barboza <danielhb...@gmail.com>
---

I've made this test file to make sure I wasn't messing
up with the logic at patch 8. The idea of having this
test seems okay, but probably I could do it shorter/cleaner.

Feel free to discard it if it's not applicable or
unnecessary. Adding this new file make the whole patch series,
aimed at reduce code repetition and lines of code, add more
lines than before. The irony is real.


It will reduce lines if you fold the data.entityName = X bit into the TEST_ calls, passing the string value as the first argument for example

Got it.


We don't have a ton of fine grained unittesting like this in libvirt, but it doesn't hurt. Though in this case it seems kinda overkill IMO to call it for possible combo that it's used in. I think it's better to just call it in all the invocations to give full code coverage. If we want to test the individual driver usage of the function then we would want to find a way to test those entry points directly IMO. Maybe others have opinions.

Do you mean we should just test the actual usage of the function in the code instead of testing all possible fail scenarios? For example, the code does not call virConnectValidateURIPath("/system", X , false) anywhere, so it's ok to
not test them since it's not being exercised anyway - otherwise we're
entering TDD territory (which I don't mind - kind of a fan TBH), testing all
possible stuff just for sake of testing.


Is that what you're saying? I'm fine with it, and it will cut a good chunk of
lines in this file too.


I was thinking, add test cases that are needed to hit every bit of logic in the function. So

privileged=false, /session path
privileged=false, non-/session path failure
privileged=true, /system path
privileged=true, non-/system path failure
privileged=true, vbox /session
privileged=true, qemu /session
privileged=true, other driver /session failure

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to