Comment 1 Sönke Schwardt-Krummrich 2014-11-26 10:53:57 CET

ucs-school-lib/python/smbstatus.py parses the output of smbstatus. Every now and then the samba project changes the output of smbstatus and this breaks our parser.

Please try to test if the parser is still working.

Comment 2 Ammar Najjar 2014-11-27 11:48:24 CET

A new script is created with the name "103_smbstatus_parser" to test the output of the smbstatus parser class, including:

  - creating three connections to the host (netlogon, sysvol, IPC$)
  - check the attributes of the result SMB_Status class ('pid', 'username', 'group', 'machine', 'services', 'ipaddress')
  - script fails if no connection was established or if any of the mentioned attributes were not parsed.

Tested on UCS 3.2.3 and UCS4.0 single master and multi-server.

Comment 3 Florian Best 2014-11-27 12:04:33 CET

Is it possible to also check if
* the PID of the processes you are starting is in the output
* the username actual is 'Administrator'
* if the IP address is the actual IP address
* close pop{1,2,3} after running the test

Comment 4 Ammar Najjar 2014-11-28 10:04:59 CET

(In reply to Florian Best from comment #3)
> Is it possible to also check if
> * the PID of the processes you are starting is in the output
> * the username actual is 'Administrator'
> * if the IP address is the actual IP address
> * close pop{1,2,3} after running the test

Yes.

- Additional checks are added to the same script to check matching:
(pid, username, services, ipaddress) with the actual values.
- All Popen objects are terminated manually before the script exits.

Comment 5 Florian Best 2015-01-15 11:20:08 CET

Created attachment 6603 [details]
103_ucsschool_smbstatus_parser

The test is good, here are some suggestions for improvement:

* Currently it is not correctly checked if e.g. the service of pop1 is 'netlogon' but it is checked if it is one of 'netlogon', 'IPC' or 'sysvol'.
* The test fails if there are some other concurrent smb connections which are not initiated by the test case:
"""Attribute (services) is parsed wrong as (['Marktplatz']), expected in ([['netlogon'], ['sysvol'], ['IPC$']])"""
To fix this I suggest to go through the processes by the services list.

It would be good to preserve the argument names in the check_attribute function. It is easier to understand what 'process' is than what is meant with '_dict'. It is also good to place the check_attribute method underneath of main() as this is the way we read the code.

You can use getattr(obj, attribute_name) instead of obj.__getattribute__(attribute_name)

The following code fails if IPv6 would be used:
if attr == 'ipaddress':
  value = value.split(':', 1)[0]
→ value.rsplit have to be used.

Attached is a alternative working test script, which could also be easier adapted to test for IPv6 or 2 services with 1 smb-connection.

Comment 6 Ammar Najjar 2015-01-16 10:52:00 CET

(In reply to Florian Best from comment #5)

> * Currently it is not correctly checked if e.g. the service of pop1 is
> 'netlogon' but it is checked if it is one of 'netlogon', 'IPC' or 'sysvol'.
> * The test fails if there are some other concurrent smb connections which
> are not initiated by the test case:
> """Attribute (services) is parsed wrong as (['Marktplatz']), expected in
> ([['netlogon'], ['sysvol'], ['IPC$']])"""
> To fix this I suggest to go through the processes by the services list.
> 

As Comment 3 suggested: to check if
* the PID of the processes you are starting is in the output

Popen.pid does not return the pid of the invoked process. so it is not possible to check which pid matches which service, so I checked if the pids/services all are running without making a match to each ones Popen object.

> It would be good to preserve the argument names in the check_attribute
> function. It is easier to understand what 'process' is than what is meant
> with '_dict'. It is also good to place the check_attribute method underneath
> of main() as this is the way we read the code.
> 

OK.

> You can use getattr(obj, attribute_name) instead of
> obj.__getattribute__(attribute_name)
> 

OK.

> The following code fails if IPv6 would be used:
> if attr == 'ipaddress':
>   value = value.split(':', 1)[0]
> → value.rsplit have to be used.
> 

OK.

> Attached is a alternative working test script, which could also be easier
> adapted to test for IPv6 or 2 services with 1 smb-connection.

The alternative test script is taken including:
- Ignore pids.
- Check via service attribute.
- Rename _dict, and use getattr for the used dictionary.
- Reorder the functions in the script.
- Use rsplit instead of split.

Comment 7 Florian Best 2015-01-19 12:15:54 CET

OK