SeattleTestbed / seattlelib_v2

RepyV2 libraries to use with SeattleTestbed
MIT License
1 stars 20 forks source link

Bug fix (#195): Handle type check of bool seperately due to conflict with int type #196

Closed rrgodhorus closed 1 year ago

rrgodhorus commented 1 year ago

In wrapper.r2py line 460, there's a type check to see if arguments passed to a function match the expected types. It uses the isinstance() built-in to do the check. However, since bool is a subtype of int in python, trying to restrict arguments passed to a function using int will also include bool, which can lead to unexpected results.

This violates the principle of least astonishment, since users would not expect bool to be included when they restrict argument types with int.

Also, there is also a potential inconsistency in behaviour, as repy library functions use a different type check, that will catch erroneously passed bools.

rrgodhorus commented 1 year ago

You'll also want to add in some unit tests that test the encasement library. At a minimum, please check:

1. passing a bool does not pass when the restriction is a list containing an int (and not bool)

2. passing a bool does not pass when the restriction is an int

3. passing a bool works in both of the above cases when using a bool type

4. passing an int behaves normally

Note, these are likely going to be in a set of small files added into the tests directory. https://github.com/SeattleTestbed/seattlelib_v2/tree/master/tests Please double check you are able to run the test runner and that the tests pass before starting...

It seems like many of the tests (mainly Networking based) are failing when I try to run python utf.py -a (after initializing and building following the steps given in travis.yml). Not quite sure why. I did notice that my Mac asked for some permission in the middle, so some of these tests might not work on a modern system.

rrgodhorus commented 1 year ago

I have added a unit test and verified that it runs using python utf.py -f ut_seattlelib_wrapper_check_args.r2py. Please review.

JustinCappos commented 1 year ago

Looks good. @vchrombie, can you also please do a review / test before merge?