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

Add tests to IPAM plugin #257

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

Add tests to IPAM plugin #257

wants to merge 2 commits into from

Conversation

KapilYadava
Copy link
Contributor

@KapilYadava KapilYadava commented Feb 10, 2025

Fixes #92 for IPAM

@github-actions github-actions bot added enhancement New feature or request size/L labels Feb 10, 2025
@KapilYadava KapilYadava changed the title Add test for IPAM plugin Add test to IPAM plugin Feb 13, 2025
@KapilYadava KapilYadava requested a review from damyan February 13, 2025 07:30
@KapilYadava KapilYadava marked this pull request as ready for review February 13, 2025 07:31
@KapilYadava KapilYadava self-assigned this Feb 13, 2025
@damyan damyan changed the title Add test to IPAM plugin Add tests to IPAM plugin Feb 17, 2025
Copy link
Member

@damyan damyan left a comment

Choose a reason for hiding this comment

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

Improvements needed:

  • squash first 4 commits to a single one, adding some meaningful name (adding some meaningful names for commits/branches/PRs/descriptions is generally a good idea, because those will appear on default branch history)
  • enhance the create IPAM IP tests to:
    • create a new IPAM IP for some MAC address
    • update already created IPAM IP for the same MAC, because IP address differs
    • update already created IPAM IP for the same MAC, because subnet differs
  • remove redundant creation of default config. idea:
    • change loadConfig() to setup6() in suite_test, so the client is parametrized with the proper configuration. use that config in all straightforward tests
    • delete config creation from plugin_test
    • whenever a different/wrong/malformed/non-existent config is needed: create it in the test and call loadConfig()

Let's discuss the changes in our next meeting

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tests for all plugins
2 participants