Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[config] Add unit tests for 'config interface breakout' command #1223

Merged
merged 6 commits into from
Dec 22, 2020

Conversation

praveen-li
Copy link

@praveen-li praveen-li commented Nov 8, 2020

Test Plan:

Consideration for sonic utilities breakout config tests: 

-- create a breakout config file,  
-- mock load_configMgmt() to return specially constructed configMgmtDPB instance. 
-- Device info get_ path*__file function should return default file so it should be mocked. 
-- config db must have breakout config table. 
-- mock get child ports to return interfaces, if needed.
-- mock shut down interfaces, if needed.

*Test Plan:* 

*>>> Test _get_ breakout options()* 
-- create a file breakout config. This can be a PY test fixture. 
-- mock device information part to return breakout file. 
-- pass argument incomplete as below and observe the results: 
  a.) 1x1 b.) 4x c.) 2x d.) Wrong interface. 

*>> Test Verbose Args:* 
Pass verbose as args, and observe that loadConfigMgmt is called with verbose. 

 *>> Test breakout_extra_table_warning Args:* 
Test breakout_extra_table_warning for breakout port. 
Add unknown_table to config with port which will be broken out 
Observe: Warning and ask User confirmation for the tables WithOut Yang models. 

*>>> Test Negative case: 
Test negative case of breakout port. Such as: 
Wrong Interface, wrong option and Wrong breakout Mode for DPB Command. 

*>>> Test below Config Breakout commands:* 

       [Mock loadConfigMgmt and device info part for all below tests] 
       [Load a fixed config in Data Tree, i.e. may not be same as configDB from mockTables] 
       [Replace the configDB.json with test config if needed. Or correct configDB then use it. Also replace it at the end with original Config] 
       [Note: we are testing only main.py part not config_mgmt.py] 
       [Observe point for all below will be: a.) Click.echo part and b.) Update to BRK_CFG table in in-memory DB] 

        # Ethernet8: start from 4x25G-->2x50G with -f -l 
           sudo config interface breakout Ethernet8 2x50G -f -l 

         # Ethernet8: move from 2x50G-->1x100G without force, list deps 
           sudo config interface breakout Ethernet8 1x100G[40G] -f -l 

        # Ethernet8: move from 2x50G-->1x100G with force, where deps exists 
           sudo config interface breakout Ethernet8 1x100G[40G] -f  

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

        # Ethernet8: move from 4x25G-->1x100G with force, no deps 
           sudo config interface breakout Ethernet8 1x100G -f 

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

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

Signed-off-by: Praveen Chaudharypchaudhary@linkedin.com

- What I did
Sonic Utils test for 'config interface breakout' command.

- How I did it
As per test plan above.

- How to verify it

tests/aclshow_test.py ...........                                        [  5%]
tests/bgp_commands_test.py ...                                           [  6%]
tests/config_dpb_test.py .....                                           [  8%]
tests/config_mgmt_test.py ....                                           [ 10%]
tests/config_test.py ..                                                  [ 10%]
tests/console_test.py ............................................       [ 26%]

.......
.......
utilities_common/constants.py                  8      0   100%
utilities_common/db.py                        12      2    83%
utilities_common/intf_filter.py               30     18    40%
utilities_common/multi_asic.py                70      7    90%
utilities_common/netstat.py                   37     11    70%
utilities_common/util_base.py                 32     32     0%
watchdogutil/__init__.py                       0      0   100%
watchdogutil/main.py                          54     54     0%
--------------------------------------------------------------
TOTAL                                      15461   9532    38%
Coverage HTML written to dir htmlcov
Coverage XML written to file coverage.xml


========================= 276 passed in 115.19 seconds =========================
pchaudha@df0493b4ca14:/sonic/src/sonic-utilities$ exit
exit

- 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)

@lguohan
Copy link
Contributor

lguohan commented Nov 8, 2020

5860 is not going to be approved. please add natsorted in your test.

@praveen-li
Copy link
Author

5860 is not going to be approved. please add natsorted in your test.

@lguohan
Adding natsort in test will not help, because sonic-cfggen is using it in sort_data(), which must be changed. Any ways, a call to sort_data() was not necessary in my tests, so I removed it. All checks r good now.

@praveen-li
Copy link
Author

@lguohan: This PR is up for review now, i.e not dependent on other PR. Thanks.

tests/config_dpb_test.py Outdated Show resolved Hide resolved
lguohan
lguohan previously approved these changes Nov 12, 2020
@praveen-li
Copy link
Author

retest default please

@praveen-li
Copy link
Author

retest this please

@zhenggen-xu
Copy link
Collaborator

Retest this please

Praveen Chaudhary added 2 commits November 19, 2020 15:05
…' command.

Signed-off-by: Praveen Chaudhary<pchaudhary@linkedin.com>
Signed-off-by: Praveen Chaudhary<pchaudhary@linkedin.com>
@praveen-li
Copy link
Author

Doing some force pushes to fix a issue seen with PY3, do not mind.

@praveen-li praveen-li force-pushed the config_brk_tests branch 2 times, most recently from 5bcf2f5 to ddcc2de Compare November 20, 2020 00:13
….py for PY3 support.

changes:
-- Changing Class name
-- minor fix in main.py for PY3 support
-- remove sort_data from config_mgmt.py

Signed-off-by: Praveen Chaudhary<pchaudhary@linkedin.com>
@praveen-li
Copy link
Author

@jleveque and @lguohan : I had to try couple of PY3 fix, so did force push.

Now I see that new tests are always passing, but there are console_test.py failures, which are not related to my changes. I will rerun my tests once console_test.py is solved. Thx.
.

16:31:49  tests/config_dpb_test.py .....                                           [  9%]<<<<<< New Test
16:31:49  tests/config_mgmt_test.py ....                                           [ 10%]
16:31:49  tests/config_test.py ..                                                  [ 10%]
16:31:50  tests/console_test.py ...............................................F.. [ 25%]

@praveen-li
Copy link
Author

retest this please

3 similar comments
@praveen-li
Copy link
Author

retest this please

@praveen-li
Copy link
Author

retest this please

@praveen-li
Copy link
Author

retest this please

@praveen-li
Copy link
Author

retest this please

@praveen-li
Copy link
Author

@jleveque and @lguohan : I had to try couple of PY3 fix, so did force push.

Now I see that new tests are always passing, but there are console_test.py failures, which are not related to my changes. I will rerun my tests once console_test.py is solved. Thx.
.

16:31:49  tests/config_dpb_test.py .....                                           [  9%]<<<<<< New Test
16:31:49  tests/config_mgmt_test.py ....                                           [ 10%]
16:31:49  tests/config_test.py ..                                                  [ 10%]
16:31:50  tests/console_test.py ...............................................F.. [ 25%]

@jleveque and @lguohan

I realized that, I was adding below line on teardown() of my test class, which makes console_test fail. Resolve it.

os.environ["PATH"] = os.pathsep.join(os.environ["PATH"].split(os.pathsep)[:-1])

@jleveque
Copy link
Contributor

jleveque commented Dec 16, 2020

I realized that, I was adding below line on teardown() of my test class, which makes console_test fail. Resolve it.

os.environ["PATH"] = os.pathsep.join(os.environ["PATH"].split(os.pathsep)[:-1])

Aha. Yes, I found and fixed other tests recently which were doing the same thing in the teardown function but not appending to the PATH in the setup function, thus breaking the PATH. 👍

@praveen-li
Copy link
Author

@jleveque
This PR adds only new tests, not much change in Dev Code, it will be great if we can review and merge this one. Because for any future PR we want these config breakout tests to execute. Thx.

@jleveque jleveque changed the title [test_config_dpb.py]: Sonic Utils test for 'config interface breakout… [test_config_dpb.py] Add unit tests for 'config interface breakout' command Dec 22, 2020
@jleveque jleveque changed the title [test_config_dpb.py] Add unit tests for 'config interface breakout' command [config] Add unit tests for 'config interface breakout' command Dec 22, 2020
@jleveque jleveque merged commit b6221f4 into sonic-net:master Dec 22, 2020
anand-kumar-subramanian pushed a commit to anand-kumar-subramanian/sonic-utilities that referenced this pull request Mar 2, 2021
…c-net#1223)

Consideration for sonic utilities breakout config tests: 

-- create a breakout config file,  
-- mock load_configMgmt() to return specially constructed configMgmtDPB instance. 
-- Device info get_ path*__file function should return default file so it should be mocked. 
-- config db must have breakout config table. 
-- mock get child ports to return interfaces, if needed.
-- mock shut down interfaces, if needed.

*Test Plan:* 

*>>> Test _get_ breakout options()* 
-- create a file breakout config. This can be a PY test fixture. 
-- mock device information part to return breakout file. 
-- pass argument incomplete as below and observe the results: 
  a.) 1x1 b.) 4x c.) 2x d.) Wrong interface. 

*>> Test Verbose Args:* 
Pass verbose as args, and observe that loadConfigMgmt is called with verbose. 

 *>> Test breakout_extra_table_warning Args:* 
Test breakout_extra_table_warning for breakout port. 
Add unknown_table to config with port which will be broken out 
Observe: Warning and ask User confirmation for the tables WithOut Yang models. 

*>>> Test Negative case: 
Test negative case of breakout port. Such as: 
Wrong Interface, wrong option and Wrong breakout Mode for DPB Command. 

*>>> Test below Config Breakout commands:* 

       [Mock loadConfigMgmt and device info part for all below tests] 
       [Load a fixed config in Data Tree, i.e. may not be same as configDB from mockTables] 
       [Replace the configDB.json with test config if needed. Or correct configDB then use it. Also replace it at the end with original Config] 
       [Note: we are testing only main.py part not config_mgmt.py] 
       [Observe point for all below will be: a.) Click.echo part and b.) Update to BRK_CFG table in in-memory DB] 

        # Ethernet8: start from 4x25G-->2x50G with -f -l 
           sudo config interface breakout Ethernet8 2x50G -f -l 

         # Ethernet8: move from 2x50G-->1x100G without force, list deps 
           sudo config interface breakout Ethernet8 1x100G[40G] -f -l 

        # Ethernet8: move from 2x50G-->1x100G with force, where deps exists 
           sudo config interface breakout Ethernet8 1x100G[40G] -f  

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

        # Ethernet8: move from 4x25G-->1x100G with force, no deps 
           sudo config interface breakout Ethernet8 1x100G -f 

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

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

Signed-off-by: Praveen Chaudhary<pchaudhary@linkedin.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants