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

E2e tests #9

Merged
merged 7 commits into from
Dec 17, 2024
Merged

E2e tests #9

merged 7 commits into from
Dec 17, 2024

Conversation

gretelliz
Copy link
Contributor

This PR is to test the return codes.

@gretelliz gretelliz requested a review from italovalcy December 4, 2024 03:22
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.

Hi @gretelliz as we discussed on today's meeting, can you please refactor this test and split the return code validation function into a new file, let's say test_06_l2vpn_return_codes.py ?

@gretelliz
Copy link
Contributor Author

Sure @italovalcy, please note that I did what you suggested in the last commit.

@gretelliz gretelliz requested a review from italovalcy December 9, 2024 21:15
@italovalcy
Copy link
Collaborator

Hi Gretel I see that some of the changes described on this PR are also listed on PR #8. I believe that is because the target branch here is also main, while the branch you are using here was based on the previous one from PR #8. I will change the target branch here to facilitate the review, okay?

@italovalcy italovalcy changed the base branch from main to e2e_tests_topology December 12, 2024 07:30
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.

Hi Gretel, amazing to see how this is progressing well! Congratulations! I have two comments for you please to consider:

  1. Can you please make a slight change on the name of the tests to add a very short description about what the test is doing? I added suggestions for the two first ones, but please also apply this change on the others!
  2. Can you please run a pylint or similar on this code? I see a few commas on the wrong place and pylint will help you identify some improvements on the formatting style.

tests/test_06_l2vpn_return_codes.py Outdated Show resolved Hide resolved
tests/test_06_l2vpn_return_codes.py Outdated Show resolved Hide resolved
tests/test_06_l2vpn_return_codes.py Outdated Show resolved Hide resolved
tests/test_06_l2vpn_return_codes.py Outdated Show resolved Hide resolved
@gretelliz
Copy link
Contributor Author

Thank you for your review @italovalcy.

@gretelliz gretelliz requested a review from italovalcy December 17, 2024 15:17
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 nicely done, Gretel! LGTM!

@gretelliz
Copy link
Contributor Author

@italovalcy, thank you!

@gretelliz gretelliz merged commit 2c7be24 into e2e_tests_topology Dec 17, 2024
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.

2 participants