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

[UT] [Portsyncd] Added Unit Tests for portsyncd #2297

Merged
merged 29 commits into from
Sep 1, 2022

Conversation

vivekrnv
Copy link
Contributor

@vivekrnv vivekrnv commented May 27, 2022

What I did

Added Unit tests for portsyncd module:

  • Create a new make target under mock_tests/Makefile.am to include a target for linksync.cpp
  • Mocked a rtnl_link object for testing onMsg method of Linksync.cpp
  • Used/updated the existing mock infra to allow for testing.

Why I did it

To increase code/feature coverage

How I verified it

vkarri@e4bc0a3e5630:/sonic$ make -f slave.mk target/debs/bullseye/swss_1.0.0_amd64.deb
..........
make[3]: Entering directory '/sonic/src/sonic-swss/tests'
Making check in mock_tests
make[4]: Entering directory '/sonic/src/sonic-swss/tests/mock_tests'
make  check-TESTS
make[5]: Entering directory '/sonic/src/sonic-swss/tests/mock_tests'
make[6]: Entering directory '/sonic/src/sonic-swss/tests/mock_tests'
PASS: tests_portsyncd
PASS: tests_intfmgrd
PASS: tests
============================================================================
Testsuite summary for sonic-swss 1.0
============================================================================
# TOTAL: 3
# PASS:  3
# SKIP:  0
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0
============================================================================

vkarri@4897dcba242c:/sonic/src/sonic-swss/tests/mock_tests$ make tests_portsyncd 
.............
..............
..............
vkarri@5e69834fbb01:/sonic/src/sonic-swss/tests/mock_tests$ ./tests_portsyncd 
Running main() from /build/googletest-YnT0O3/googletest-1.10.0.20201025/googletest/src/gtest_main.cc
[==========] Running 6 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 6 tests from PortSyncdTest
[ RUN      ] PortSyncdTest.test_linkSyncInit
[       OK ] PortSyncdTest.test_linkSyncInit (0 ms)
[ RUN      ] PortSyncdTest.test_cacheOldIfaces
[       OK ] PortSyncdTest.test_cacheOldIfaces (0 ms)
[ RUN      ] PortSyncdTest.test_onMsgNewLink
[       OK ] PortSyncdTest.test_onMsgNewLink (0 ms)
[ RUN      ] PortSyncdTest.test_onMsgDelLink
[       OK ] PortSyncdTest.test_onMsgDelLink (0 ms)
[ RUN      ] PortSyncdTest.test_onMsgMgmtIface
[       OK ] PortSyncdTest.test_onMsgMgmtIface (0 ms)
[ RUN      ] PortSyncdTest.test_onMsgIgnoreOldNetDev
[       OK ] PortSyncdTest.test_onMsgIgnoreOldNetDev (0 ms)
[----------] 6 tests from PortSyncdTest (1 ms total)

[----------] Global test environment tear-down
[==========] 6 tests from 1 test suite ran. (1 ms total)
[  PASSED  ] 6 tests.

Details if related

vivekrnv and others added 18 commits April 27, 2022 18:38
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>
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>
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@vivekrnv vivekrnv requested a review from prsunny as a code owner May 27, 2022 06:20
@liat-grozovik
Copy link
Collaborator

/azp run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vivekrnv
Copy link
Contributor Author

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

keboliu
keboliu previously approved these changes Jun 1, 2022
@vivekrnv
Copy link
Contributor Author

vivekrnv commented Jun 8, 2022

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vivekrnv
Copy link
Contributor Author

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@vivekrnv
Copy link
Contributor Author

vivekrnv commented Aug 2, 2022

@vivekrnv could you please handle conflicts and followup on checkers?

Handled. @prsunny can you please review

Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
@prsunny
Copy link
Collaborator

prsunny commented Aug 4, 2022

i dont think we should combine refactoring and portsyncd changes together. Can you please have this PR only for the portsyncd?

@vivekrnv
Copy link
Contributor Author

vivekrnv commented Aug 4, 2022

i dont think we should combine refactoring and portsyncd changes together. Can you please have this PR only for the portsyncd?

Will update

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 5, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

@vivekrnv
Copy link
Contributor Author

vivekrnv commented Aug 5, 2022

i dont think we should combine refactoring and portsyncd changes together. Can you please have this PR only for the portsyncd?

updated

@vivekrnv
Copy link
Contributor Author

vivekrnv commented Aug 8, 2022

/easycla

@vivekrnv
Copy link
Contributor Author

vivekrnv commented Aug 8, 2022

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vivekrnv
Copy link
Contributor Author

vivekrnv commented Aug 8, 2022

/azpw run Azure.sonic-swss

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-swss

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@@ -0,0 +1,64 @@
#include "portsyncd/linksync.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we discussed it last time, please move this to portsyncd itself. I don't see a reason why it has to be part of helper file and cause merge conflicts for future bug fixes.

Copy link
Contributor Author

@vivekrnv vivekrnv Aug 13, 2022

Choose a reason for hiding this comment

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

If these are moved back to portsyncd.cpp, i won't be able to test those since i can't link my test binary with portsyncd.cpp. Because with gtest we already link it with -lgtest_main so the linker wouldn't allow another main() function to be present in any other files. Thus i need a seperate file to test handlePortConfigFromConfigDB method and g_portSet

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>
@vivekrnv
Copy link
Contributor Author

@prsunny, reviewed the comments discussed offline. Please review

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.

6 participants