[Unit Test Failure]: ut_seattelib_tcptime.py

[vladimir-v-diaz]

The ut_seattlelib_tcptime.py unit test fails with the following error:

$ python utf.py -f ut_seattlelib_tcptime.py 
Testing module: seattlelib
    Running: ut_seattlelib_tcptime.py                           [ FAIL ]
--------------------------------------------------------------------------------
Standard out :
..............................Produced..............................
[FAILED]: TCP time failed for following reason: time_updatetime called before time_register_method!
..............................Expected..............................
None
--------------------------------------------------------------------------------
$ 

Unfortunately, the code seems to be testing an unexpected function. According to the comment in the docstring header, the tcp_time_updatetime() function should be tested, but time_interface.time_updatetime(timeport) is being called in the code. I expect tcp_time.tcp_time_updatetime() to be the actual call.

Likewise, the ut_seattlelib_ntptime.py unit test appears to be testing the wrong function, although its unit test does pass for the wrong reason.

[aaaaalbert]

@karthikgvss will take a look at this issuel

[karthikgvss]

I see that tcp_time will set a mapping by calling time_register_method
time_interface.r2py will use the update method of this implementor as per the mapping. So, i think we need not change anything in ut_seattlelib_tcptime.py

similarly, for ut_seattlelib_ntptime.py, time_interface.r2py has to be called instead of time.r2py as we are testing only ntp here. Also, port has to be determined by calling list(getresources()[0]["connport"])[0] instead of hard coding 12345. Please suggest.

[vladimir-v-diaz]

I find the interaction between these modules a little surprising. It is not immediately obvious that importing tcp_time.r2py causes the behavior of time_interface.r2py to change. So these time modules are essentially performing actions on import? If this is what we really want, maybe state this side effect more clearly in the header block. Registering "tcp" time in tcp_time.r2py by calling time_interface.time_register_method() at the end of the module (i.e., the action on import) might also be a readability problem:

https://github.com/SeattleTestbed/seattlelib_v2/blob/master/tcp_time.r2py#L123-L124
Note: But I do understand that it's done this way because the tcp_time_updatetime() function needs to be defined..

What happens if we import both tcp_time.r2py and ntp_time.r2py? What if another module imports them separately, for example if these unit tests are executed in a testing framework? Or there is a circular import? I'm assuming time_interface.r2py would choose one of the available time functions and these unit tests would pass if at least one of the registered methods is successful. In other words, is it possible for this unit test to pass if ntp_time.r2py is registered beforehand and the registered tcp time module is not called at all?