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

Adding snappi vlan traffic testcase (#5) #117

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

selldinesh
Copy link
Contributor

This PR contains the test cases for running l2 traffic using snappi and SAI api with data driven model

Copy link
Contributor

@chrispsommers chrispsommers left a comment

Choose a reason for hiding this comment

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

Please see comments about test cases being able to run with SW or HW traffic generators, and avoiding vendor-specific settings if possible

@andriy-kokhan
Copy link
Contributor

@selldinesh , as a common practice, a test should contain both setup and teardown.

It make sense to split setup and teardown into separate test-cases either in case the same basic configuration should be used by the number of tests (by test suite) or when we want to test some object removal as a separate scenario. In such case it's better to group the tests into the test class. E.g.,

class TestIngressACL:

Please correct me if I'm wrong but this PR contains neither of these two. It's more like a standalone VLAN scenario with traffic. Something very similar to

def test_l2_access_to_access_vlan(npu, dataplane):

So, I'd suggest to make it as a single TC. Also, please make sure the teardown procedure reverts the configuration to what was right before TC execution.

npu.objects_discovery()


def test_l2_untagged_vlan_traffic(npu, dataplane):
Copy link
Contributor

@andriy-kokhan andriy-kokhan Sep 15, 2023

Choose a reason for hiding this comment

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

Please make sure this TC will we skipped in case ((pytest is executed without specifying the test file of or tests wildcard) and dataplane is not snappi).

Copy link

Choose a reason for hiding this comment

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

added the skip all fixture

Copy link
Contributor

Choose a reason for hiding this comment

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

As I mentioned above, please make sure that the traffic related part of TC will be skipped in case dataplane type is not snappi.

if npu.run_traffic and dataplane.config["type"] == "snappi":
    # Traffic check

selldinesh and others added 8 commits October 16, 2023 15:41
* adding snappi folder with vlan case

Signed-off-by: selldinesh <dinesh.sellappan@keysight.com>

* formatted

Signed-off-by: selldinesh <dinesh.sellappan@keysight.com>

---------

Signed-off-by: selldinesh <dinesh.sellappan@keysight.com>
Signed-off-by: Dinesh Kumar Sellappan <dinesh.sellappan@keysight.com>
Signed-off-by: selldinesh <dinesh.sellappan@keysight.com>
Signed-off-by: Dinesh Kumar Sellappan <dinesh.sellappan@keysight.com>
Signed-off-by: selldinesh <dinesh.sellappan@keysight.com>
Signed-off-by: Dinesh Kumar Sellappan <dinesh.sellappan@keysight.com>
Signed-off-by: selldinesh <dinesh.sellappan@keysight.com>
Signed-off-by: Dinesh Kumar Sellappan <dinesh.sellappan@keysight.com>
Signed-off-by: selldinesh <dinesh.sellappan@keysight.com>
Signed-off-by: Dinesh Kumar Sellappan <dinesh.sellappan@keysight.com>
Signed-off-by: selldinesh <dinesh.sellappan@keysight.com>
Signed-off-by: Dinesh Kumar Sellappan <dinesh.sellappan@keysight.com>
Signed-off-by: selldinesh <dinesh.sellappan@keysight.com>
Signed-off-by: Dinesh Kumar Sellappan <dinesh.sellappan@keysight.com>
adding data capture for tagged traffic

Signed-off-by: Dinesh Kumar Sellappan <dinesh.sellappan@keysight.com>
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