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

[macsec/test_dataplane]: Add macsec counter clear test #15257

Merged

Conversation

liamkearney-msft
Copy link
Contributor

@liamkearney-msft liamkearney-msft commented Oct 30, 2024

Description of PR

Add test for macsec counters clear command.

Summary:
Fixes #13347

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • [x ] Test case(new/improvement)

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • [ x] 202405

Approach

What is the motivation for this PR?

Fix test gap by adding sonic-clear macsec tests

How did you verify/test it?

Ran locally on t2 chassis, test passes

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

arlakshm
arlakshm previously approved these changes Nov 5, 2024
@rlhui
Copy link

rlhui commented Nov 26, 2024

@liamkearney-msft please resolve conflicts? Thanks.

@rlhui rlhui added the chassis label Nov 27, 2024
Signed-off-by: Liam Kearney <liamkearney@microsoft.com>
According to pytest docs it is bad practice to assert in fixtures, as
failure causes teardown to not run

Signed-off-by: Liam Kearney <liamkearney@microsoft.com>
Signed-off-by: Liam Kearney <liamkearney@microsoft.com>
@mssonicbld
Copy link
Collaborator

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...........................................(no files to check)Skipped
check for added large files..............................................Passed
check python ast.........................................................Passed
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

tests/macsec/test_dataplane.py:10:1: F401 'tests.common.macsec.macsec_helper.get_macsec_sa_name' imported but unused
tests/macsec/test_dataplane.py:215:9: F821 undefined name 'clear_macsec_counters'

flake8...............................................(no files to check)Skipped
check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

  1. Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
    the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
    docker container.
  2. Ensure that the pre-commit package is installed:
sudo pip install pre-commit
  1. Go to repository root folder
  2. Install the pre-commit hooks:
pre-commit install
  1. Use pre-commit to check staged file:
pre-commit
  1. Alternatively, you can check committed files using:
pre-commit run --from-ref <commit_id> --to-ref <commit_id>

Signed-off-by: Liam Kearney <liamkearney@microsoft.com>
Signed-off-by: Liam Kearney <liamkearney@microsoft.com>
…y-msft/sonic-mgmt into pub-dev/macsec-clear-counters
@liamkearney-msft
Copy link
Contributor Author

@rlhui conflicts have been resolved

arlakshm
arlakshm previously approved these changes Nov 28, 2024
@arlakshm
Copy link
Contributor

@judyjoseph, please help signoff this change

Signed-off-by: Liam Kearney <liamkearney@microsoft.com>
@judyjoseph
Copy link
Contributor

@liamkearney-msft could you add a test run PASS o/p result -- for the test_counters() and new test test_clear_counters() in the PR description?

@liamkearney-msft
Copy link
Contributor Author

liamkearney-msft commented Dec 9, 2024

@judyjoseph I have run these tests locally and clear counters works on latest 20240510.17 build. test_counters is failing but its due to a regression unrelated to these changes.

@judyjoseph judyjoseph merged commit a1446ea into sonic-net:master Dec 11, 2024
16 checks passed
@bingwang-ms
Copy link
Collaborator

@liamkearney-msft Is the new test test_clear_counters for T2 only or for T0 and T2? Can you paste the test result to the PR since it's a new test?

@liamkearney-msft
Copy link
Contributor Author

Hi @bingwang-ms , this test should be topo agnostic.
Ive run these tests manually on both arista 7800 & nokia 7250, and it passes :

04:10:41 __init__.pytest_terminal_summary         L0067 INFO   | Can not get Allure report URL. Please check logs
=============================================================== 1 passed, 1 warning in 953.59s (0:15:53) ===============================================================
DEBUG:tests.conftest:[log_custom_msg] item: <Function test_clear_counters[MACSEC_PROFILE]>

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.

[Test Gap] Test to check sonic-clear macsec
6 participants