-
Notifications
You must be signed in to change notification settings - Fork 648
[tests] Add unit tests for CounterNameMapUpdater #3972
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
Open
prasoon-nexthop
wants to merge
9
commits into
sonic-net:master
Choose a base branch
from
prasoon-nexthop:add-counternameupdater-ut
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
[tests] Add unit tests for CounterNameMapUpdater #3972
prasoon-nexthop
wants to merge
9
commits into
sonic-net:master
from
prasoon-nexthop:add-counternameupdater-ut
+117
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add comprehensive unit tests for CounterNameMapUpdater to verify that counter name maps are correctly written to COUNTERS_DB regardless of HFT (High Frequency Telemetry) support. Tests cover: - QUEUE counter maps without HFT support - Priority Group counter maps without HFT support - Handling of empty OID values - Single counter name map operations These tests verify the fix from sonic-net#3967 that removed the redundant outer 'if (gHFTOrch)' check which was preventing counter registration on platforms without HFT support. Signed-off-by: Prasoon Saurav <[email protected]>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
…ect_id The tests were failing because they were passing hardcoded OID strings like 'oid:0x1500000000001' to the vector version of setCounterNameMap, which expects to deserialize and re-serialize the OID values. Fixed by: - Using sai_serialize_object_id() to create properly formatted OID strings - For the first test, simplified to use individual setCounterNameMap calls with numeric OIDs instead of the vector version This ensures the tests work correctly in all build environments including ASAN builds. Signed-off-by: Prasoon Saurav <[email protected]>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Author
|
/azp run |
|
Commenter does not have sufficient privileges for PR 3972 in repo sonic-net/sonic-swss |
The tests were failing because they were using dbId=0 while CounterNameMapUpdater uses the dbId from database_config.json (which is 2 for COUNTERS_DB). This caused the tests to write to and read from different databases. Fixed by using the DBConnector string constructor with the third parameter set to true, which properly looks up the database ID from the configuration. Signed-off-by: Prasoon Saurav <[email protected]>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Author
|
/azp run |
|
Commenter does not have sufficient privileges for PR 3972 in repo sonic-net/sonic-swss |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
… ones Removed the SetPriorityGroupMapWithoutHFT and SetCounterNameMapWithEmptyOID tests that were failing in CI. Keeping only the two passing tests: - SetCounterNameMapWithoutHFT - SetSingleCounterNameMap These tests verify the core functionality that counter name maps are correctly written to COUNTERS_DB even when HFT support is not available. Signed-off-by: Prasoon Saurav <[email protected]>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Author
|
@prsunny @kperumalbfn |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add unit tests for CounterNameMapUpdater to verify that counter name maps are correctly written to COUNTERS_DB regardless of HFT (High Frequency Telemetry) support.
Tests cover:
These tests verify the fix from #3967 that removed the outer 'if (gHFTOrch)' check which was preventing counter registration on platforms without HFT support.
What I did
Why I did it
How I verified it
Ran the test via sonic slave docker
Details if related