[sonic-utilities-tests/config_mgmt_test.py]: Unit Tests for config_mg…

[praveen-li]

…mt.py library.

Signed-off-by: Praveen Chaudhary pchaudhary@linkedin.com

This is a dummy PR just for better description. Committed in [https://github.com//pull/765/commits/77e3734c66339f5b92be560c10f4d8a291ccb7e7]

Depend on #765.

Depend on sonic-net/sonic-buildimage#4798.

- What I did
Write Unit tests for Config_mgmt library introduced in PR #765.

- How I did it
Key Decisions:
-- Config_mgmt interaction with Redis DB:
These tests will be executed at build time, the entire interaction with Redis DB is mocked using:
    import mock_tables.dbconnector
This mocks below classes used in config_mgmt
from swsssdk import ConfigDBConnector, SonicV2Connector, port_util

-- Mock sonic_yang or not:
Most of the functions in config_mgmt calls APIs from sonic_yang_mgmt (https://github.com/Azure/sonic-buildimage/tree/master/src/sonic-yang-mgmt) library. It is important to catch any compatibility issues during build time rather than run time. So it is decided not to mock sonic_yang but use real instance. Due to this sonic_utilities will have a build time dependency on sonic_yang_mgmt.

-- Unit Test each function or the functions exposed to caller:
1. To unit test each function, we need to mock sonic_yang, which is against the previous decision. 

2. Also calling the functions which are exposed to the caller is sufficient to unit test all functions. Because functions like breakOutPort internally call every function in the library, and the expected result is not produced if internal functions are not working properly.

3. In the real scenario, the caller will use this library for config validation and to break out the port in a continuous manner. To emulate a real scenario it is decided to exercise only the functions exposed to the caller.


Check Result:
-- How to check expected result for breakout Command:
1. A successful call to breakOutPort API results in a call to writeConfigDB API twice.

One time with config to be deleted and second time with config to be added. Both calls are necessary and should be done in order. This is tested by mocking writeConfigDB API as below:

cmdpb = config_mgmt.ConfigMgmtDPB(source=config_mgmt.CONFIG_DB_JSON_FILE)
cmdpb.writeConfigDB = MagicMock(return_value=True)

Then we can check result as below:

calls = [call(dConfig), call(aConfig)]
        assert cmdpb.writeConfigDB.call_count == 2
        cmdpb.writeConfigDB.assert_has_calls(calls, any_order=False)

Note: it is important to check that both calls are done, rather than just checking final config.

Tests: 
-- Create the configMgmt Class object and validate config.

-- Have a table without yang model in config, Check if this table is returned by tablesWithoutYang()

-- Test configWithKeys() to search ports in the config.

-- Important Test case, Test below sequence for port breakout:

        #Ethernet8: start from 4x25G-->2x50G with -f -l

        #Ethernet8: move from 2x50G-->1x100G without force, list deps

        # Ethernet8: move from 2x50G-->1x100G with force, where deps exists

        # Ethernet8: move from 1x100G-->4x25G without force, no deps

        # Ethernet8: move from 4x25G-->1x100G with force, no deps

        # Ethernet8: move from 1x100G-->1x50G(2)+2x25G(2) with -f -l,

        # Ethernet4: breakout from 4x25G to 2x50G with -f -l

Note: this is important to do this sequence in one test case, because it is good to test with continuous  change in Config. Also this way, a new configuration for each test is not needed.
 
Expected Result:
All automated tests should pass.

Each breakout test is checked for expected result  as below:
Have a hard coded value of expected dConfig and aConfig, where
dConfig: Config to be deleted.
aConfig: Config to be deleted.

    And check if writeConfigDB is called using the above configurations.
Example: 

dConfig = {u'PORT': {u'Ethernet8': None}}

aConfig = {u'ACL_TABLE': {u'NO-NSW-PACL-V4': {u'ports': ['Ethernet0', \
        'Ethernet4', 'Ethernet8', 'Ethernet10']}, u'NO-NSW-PACL-TEST': {u'ports': \
        ['Ethernet11']}}, u'INTERFACE': {u'Ethernet11|2a04:1111:40:a709::1/126': \
        {u'scope': u'global', u'family': u'IPv6'}, u'Ethernet11': {}}, \
        u'VLAN_MEMBER': {u'Vlan100|Ethernet8': {u'tagging_mode': u'untagged'}, \
        u'Vlan100|Ethernet11': {u'tagging_mode': u'untagged'}}, u'PORT': \
        {'Ethernet8': {'speed': '50000', 'lanes': '73,74'}, 'Ethernet10': \
        {'speed': '25000', 'lanes': '75'}, 'Ethernet11': {'speed': '25000', 'lanes': '76'}}}
        
self.checkResult(cmdpb, dConfig, aConfig)

Actual Result:

============================================================ test session starts ============================================================
platform linux2 -- Python 2.7.13, pytest-3.0.6, py-1.4.32, pluggy-0.4.0
rootdir: /sonic/src/sonic-utilities/sonic-utilities-tests, inifile: pytest.ini
plugins: cov-2.4.0
collected 4 items

config_mgmt_test.py ....

========================================================= 4 passed in 0.67 seconds ==========================================================

- How to verify it
Actual Result:

============================================================ test session starts ============================================================
platform linux2 -- Python 2.7.13, pytest-3.0.6, py-1.4.32, pluggy-0.4.0
rootdir: /sonic/src/sonic-utilities/sonic-utilities-tests, inifile: pytest.ini
plugins: cov-2.4.0
collected 4 items

config_mgmt_test.py ....

========================================================= 4 passed in 0.67 seconds ==========================================================

- Previous command output (if the output of a command-line utility has changed)

- New command output (if the output of a command-line utility has changed)

[samaity]

I was thinking if you could create a folder containing two mock data json files like configDbJson and portBreakOutConfigDbJson and then import it to this function.
something like below.
This Function in acl_loader_test.py file uses acl_input folder.
It will help to structurize this test by dividing the code and the input data. What do you think?

[praveen-li]

Since right now file is relatively small, It seems okie to have json in file itself.