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

feat(anta.tests): Added testcase to verify interface speed #552

Merged
merged 22 commits into from
Apr 23, 2024

Conversation

MaheshGSLAB
Copy link
Contributor

Description

Added testcase to verify the interface speed.

Verifies the speed, lanes, auto-negotiation status, and mode as full duplex for interfaces.

If speed is auto then verify auto-negotiation as success and mode as full duplex.
If speed is auto with a value(auto 10g) then verify auto-negotiation as success, mode as full duplex and speed/lanes as per input.
If speed is forced with a value(forces 10g) then verify mode as full duplex and speed as per input.
If speed with lane(100g-8) then verify mode as full duplex and speed/lanes as per input.

Fixes #542

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have run pre-commit for code linting and typing (pre-commit run)
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes (tox -e testenv)


# Collecting actual interface details
actual_interface_output = {
"auto negotiation": auto_negotiation if "auto" in interface.speed else "None",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

"None" need to replace with Null

Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you using "None"? It looks wrong no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To create a generalize dict of actual and expected speed details of various type of speeds i used "None" in case that parameter is not needed for verification. For example if speed is 100g-4 in that case we dont need to validate auto negotiation hence updating auto negotiation key as None in both actual and expected output. So while comparing both actual and expected parameters match. I know if speed is 100g-4 in that case we should not define auto negotiation in actual and expected output but to achieve this we will have multiple condition for each type.

Let me know in case we need to change it.

Copy link
Contributor

Choose a reason for hiding this comment

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

If we need a string let's put N/A

@MaheshGSLAB MaheshGSLAB marked this pull request as ready for review February 6, 2024 08:11
anta/tests/interfaces.py Outdated Show resolved Hide resolved
anta/tests/interfaces.py Outdated Show resolved Hide resolved
anta/tests/interfaces.py Outdated Show resolved Hide resolved
anta/custom_types.py Outdated Show resolved Hide resolved
anta/tests/interfaces.py Outdated Show resolved Hide resolved

# Collecting actual interface details
actual_interface_output = {
"auto negotiation": auto_negotiation if "auto" in interface.speed else "None",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you using "None"? It looks wrong no?

Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.


# Collecting actual interface details
actual_interface_output = {
"auto negotiation": auto_negotiation if "auto" in interface.speed else "None",
Copy link
Contributor

Choose a reason for hiding this comment

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

If we need a string let's put N/A

Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

anta/custom_types.py Outdated Show resolved Hide resolved
anta/tests/interfaces.py Outdated Show resolved Hide resolved
anta/custom_types.py Show resolved Hide resolved
anta/tests/interfaces.py Outdated Show resolved Hide resolved
anta/tests/interfaces.py Outdated Show resolved Hide resolved
anta/tools/utils.py Outdated Show resolved Hide resolved
anta/tools/utils.py Outdated Show resolved Hide resolved
anta/tools/utils.py Outdated Show resolved Hide resolved
anta/tools/utils.py Outdated Show resolved Hide resolved
anta/tools/utils.py Outdated Show resolved Hide resolved
anta/tests/interfaces.py Outdated Show resolved Hide resolved
anta/tools/utils.py Outdated Show resolved Hide resolved
tests/units/anta_tests/test_interfaces.py Outdated Show resolved Hide resolved
@MaheshGSLAB MaheshGSLAB requested a review from gmuloc March 5, 2024 14:48
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

anta/tools/utils.py Outdated Show resolved Hide resolved
anta/tests/interfaces.py Outdated Show resolved Hide resolved
@MaheshGSLAB MaheshGSLAB requested a review from gmuloc March 21, 2024 09:51
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@gmuloc gmuloc added this to the v1.0.0 milestone Apr 12, 2024
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

anta/tests/interfaces.py Outdated Show resolved Hide resolved
anta/tests/interfaces.py Outdated Show resolved Hide resolved
anta/tools.py Outdated Show resolved Hide resolved
anta/tools.py Outdated Show resolved Hide resolved
Comment on lines 788 to 791
- If the speed is set to auto with a value (e.g., 'auto 10g'), the class verifies that auto-negotiation is successful,
the mode is set to full duplex, and the speed/lanes match the input.
- If the speed is set with a lane value (e.g., '100g-8'), the class verifies that the mode is set to full duplex and
the speed/lanes match the input.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this still reflect the latest changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated. Thanks

Comment on lines 795 to 796
* Success: The test will pass if an interface is configured correctly with the specified speed, lanes, auto-negotiation status, and mode.
* Failure: The test will fail if an interface is not found, or if the speed, lanes, auto-negotiation status, or mode do not match the input.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this reflects the latest changes since duplex mode is not an input anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated. Thanks

Copy link
Contributor

@carl-baillargeon carl-baillargeon left a comment

Choose a reason for hiding this comment

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

Small adjustments for docstrings otherwise LGTM. Tested locally.

@gmuloc gmuloc merged commit 7c8bf44 into aristanetworks:main Apr 23, 2024
17 checks passed
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.

Add the testcase to verify interface speed
3 participants