[libvirt] [PATCH v2 07/10] viriscsitest: Test virISCSIConnectionLogin

[Michal Prívozník]

On 07/23/2018 02:12 PM, John Ferlan wrote:
> 
> 
> On 07/23/2018 04:01 AM, Michal Prívozník wrote:
>> On 07/17/2018 09:14 PM, John Ferlan wrote:
>>>
>>>
>>> On 07/04/2018 05:23 AM, Michal Privoznik wrote:
>>>> Introduce one basic test that tests the simplest case:
>>>> logging into portal without any IQN.
>>>>
>>>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>>>> ---
>>>>  tests/viriscsitest.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 46 insertions(+)
>>>>
>>>> diff --git a/tests/viriscsitest.c b/tests/viriscsitest.c
>>>> index 3bb3993196..a7287069ba 100644
>>>> --- a/tests/viriscsitest.c
>>>> +++ b/tests/viriscsitest.c
>>>> @@ -94,6 +94,16 @@ static void testIscsiadmCb(const char *const*args,
>>>>                 args[8] && STREQ(args[8], "nonpersistent") &&
>>>>                 args[9] == NULL) {
>>>>          ignore_value(VIR_STRDUP(*output, iscsiadmSendtargetsOutput));
> 
> here we do some sort of comparison

What comparison? The only comparison I see is "args[X] && STREQ(args[X],
...)".

If you're referring to VIR_STRDUP() that is setting the pretended output
of the iscsiadm command. It's not comparing anything.

> 
>>>> +    } else if (args[0] && STREQ(args[0], ISCSIADM) &&
>>>> +               args[1] && STREQ(args[1], "--mode") &&
>>>> +               args[2] && STREQ(args[2], "node") &&
>>>> +               args[3] && STREQ(args[3], "--portal") &&
>>>> +               args[4] && STREQ(args[4], "10.20.30.40:3260,1") &&
>>>> +               args[5] && STREQ(args[5], "--targetname") &&
>>>> +               args[6] && STREQ(args[6], "iqn.2004-06.example:example1:iscsi.test") &&
>>>> +               args[7] && STREQ(args[7], "--login") &&
>>>> +               args[8] == NULL) {
>>>> +        /* nada */
>>>
>>>
>>> Hmm.. can we place a hold on that R-By - I missed this gem "nada"  -
>>> why is that?
>>>
>>> If we have *output because we've sent the dryRun, then why not compare?
>>
>> I'm not quite sure what you mean. What should be compared?
> 
> here we do not - instead we just use nada. 

That is correct because we do need to accept that cmd line but we do not
need to take any extra step to mimic iscsiadm behaviour we're after.

> So after reading patch10
> where nada is again used, I began to doubt/wonder about using it here
> especially since there are comparisons for all other cases.
> 
> John
> 

Michal