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

[dump] Optimized dump state cli and modified tests to not use common data #2175

Merged
merged 15 commits into from
May 30, 2022

Conversation

vivekrnv
Copy link
Contributor

Why I did

Optimize Dump State Cli:
With the optimizations made, an entire run of "dump state" will only create 1 Sonicv2Connector Object and only call connect method once per db. i.e one DBConnector Object per DB

Test Stability:
Since the dump state tests are based on data (key and field-value pairs) any changes to those tables will lead to failure in dump state tests. Thus migrated the data_sources for dump_state_test & match_engine_tests from mock_tables to dump_input/ thus making them more stable.

How I did it

How to verify it

vkarri@230fae5c3278:/sonic/src/sonic-utilities$ pytest-3 tests/dump_tests/
========================================= test session starts ==========================================
platform linux -- Python 3.9.2, pytest-6.0.2, py-1.10.0, pluggy-0.13.0
rootdir: /sonic/src/sonic-utilities/tests, configfile: pytest.ini
plugins: pyfakefs-4.5.6, cov-2.10.1
collected 140 items                                                                                    

tests/dump_tests/dump_state_test.py .............                                                [  9%]
tests/dump_tests/match_engine_test.py .................................                          [ 32%]
tests/dump_tests/module_tests/copp_test.py ..........                                            [ 40%]
tests/dump_tests/module_tests/evpn_test.py ......                                                [ 44%]
tests/dump_tests/module_tests/fdb_test.py ............                                           [ 52%]
tests/dump_tests/module_tests/interface_test.py ..........                                       [ 60%]
tests/dump_tests/module_tests/port_test.py ......                                                [ 64%]
tests/dump_tests/module_tests/portchannel_member_test.py ...                                     [ 66%]
tests/dump_tests/module_tests/portchannel_test.py ....                                           [ 69%]
tests/dump_tests/module_tests/route_test.py ..........                                           [ 76%]
tests/dump_tests/module_tests/vlan_test.py .....................                                 [ 91%]
tests/dump_tests/module_tests/vxlan_tunnel_map_test.py .....                                     [ 95%]
tests/dump_tests/module_tests/vxlan_tunnel_test.py .......                                       [100%]

========================================= 140 passed in 1.04s ==========================================

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)

vivekrnv and others added 10 commits April 14, 2022 01:39
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@lgtm-com
Copy link

lgtm-com bot commented May 19, 2022

This pull request introduces 1 alert when merging 642c6e6 into 9881f3e - view on LGTM.com

new alerts:

  • 1 for Unused import

dgsudharsan
dgsudharsan previously approved these changes May 19, 2022
dump/main.py Outdated Show resolved Hide resolved
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
Copy link
Contributor

@SuvarnaMeenakshi SuvarnaMeenakshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants