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

Added tests for untagged and any from mef_eline changes #213

Merged
merged 3 commits into from
Mar 14, 2023

Conversation

Alopalao
Copy link

@Alopalao Alopalao commented Mar 8, 2023

Related to mef_eline PR

Summary

Tests including the recent addition to vlan available values untagged and any.

Local Tests

+ python3 -m pytest tests/test_e2e_10_mef_eline.py -k '160 or 165 or 170 or 175 or 180 or 185 or 190 or 195 or 200 or 205' --reruns 2 -r fEr
============================= test session starts ==============================
platform linux -- Python 3.9.2, pytest-7.2.0, pluggy-1.0.0
rootdir: /tests
plugins: timeout-2.1.0, rerunfailures-10.2
collected 39 items / 29 deselected / 10 selected

tests/test_e2e_10_mef_eline.py ..........                                [100%]

@Alopalao Alopalao requested a review from a team as a code owner March 8, 2023 17:19
@Alopalao Alopalao changed the title Added tests for mef_eline changes Added tests for untagged and any from mef_eline changes Mar 8, 2023
@viniarck viniarck requested a review from italovalcy March 9, 2023 20:08

api_url = KYTOS_API + '/flow_manager/v2/flows/00:00:00:00:00:00:00:01'
response = requests.get(api_url)
data = response.json()["00:00:00:00:00:00:00:01"]["flows"][2]
Copy link
Member

Choose a reason for hiding this comment

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

This sequential list access [2] on the output list of v2/flows/dpid one day might backfire. Usually, flows are returned on priority, if we ended up with other higher priority flows from other NApps it could also break this test. Since the list of the flows is expected to be relatively small it's ok to iterate on it, maybe you could refactor this part trying making sure that at least of the entries matches with the expected dict?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe a helper function to search for a expected dict flow in a list would be handy to reuse on other tests too.

Copy link
Author

Choose a reason for hiding this comment

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

Added a new function get_flow_by_vlan_match

Copy link
Member

@viniarck viniarck left a comment

Choose a reason for hiding this comment

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

@Alopalao looks great to me. Nicely done covering the cases, including with inter and intra EVCs as well. The spreadsheet that you created facilitated a lot to review (and it'll be linked int the release notes), so much appreciated you creating that as well.

I'll leave this PR for @italovalcy to finish the review since you both have already been collaborating on a related review.

Copy link
Collaborator

@italovalcy italovalcy left a comment

Choose a reason for hiding this comment

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

Very well done, @Alopalao. The overall PR looks good to me. I just left a few suggestion to remove kytos restart calls because this will already happen as part of the setup_method.

tests/test_e2e_10_mef_eline.py Outdated Show resolved Hide resolved
tests/test_e2e_10_mef_eline.py Outdated Show resolved Hide resolved
tests/test_e2e_10_mef_eline.py Outdated Show resolved Hide resolved
tests/test_e2e_10_mef_eline.py Outdated Show resolved Hide resolved
tests/test_e2e_10_mef_eline.py Outdated Show resolved Hide resolved
tests/test_e2e_10_mef_eline.py Outdated Show resolved Hide resolved
tests/test_e2e_10_mef_eline.py Outdated Show resolved Hide resolved
tests/test_e2e_10_mef_eline.py Outdated Show resolved Hide resolved
tests/test_e2e_10_mef_eline.py Outdated Show resolved Hide resolved
tests/test_e2e_10_mef_eline.py Outdated Show resolved Hide resolved
@Alopalao Alopalao merged commit e7b96bb into kytos-ng:master Mar 14, 2023
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.

3 participants