[libvirt] [PATCH v4 12/12] tests: add a test for driver.c:virConnectValidateURIPath()

[Cole Robinson]

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 <danielhb413 at 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