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

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(a)gmail.com&gt; >> --- >> >> 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.