Skip to content

Fixes #47 [Housekeeping]: Develop Plugin Tests - Models #257

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 16 commits into
base: dev
Choose a base branch
from
Open

Fixes #47 [Housekeeping]: Develop Plugin Tests - Models #257

wants to merge 16 commits into from

Conversation

pheus
Copy link
Contributor

@pheus pheus commented Mar 23, 2025 •
edited
Loading

Pull Request

Related Issue

Fixes: #47 [Housekeeping] Develop Plugin Tests - Models

New Behavior

  • Adds model tests for the NetBox ACLs Plugin to improve test coverage.

Contrast to Current Behavior

  • Introduces comprehensive test cases, increasing coverage and helping detect side effects in new development.

Discussion: Benefits and Drawbacks

  • Benefits:
    • Enhances confidence in changes by ensuring the plugin’s model behavior is verified.
    • Prevents regressions by catching issues early during development.
  • Drawbacks:
    • No significant drawbacks, as this is an improvement to test coverage.

Changes to the Documentation

  • Documentation may be updated to outline the extended test coverage for new features.

Proposed Release Note Entry

  • Expanded test coverage for NetBox ACLs Plugin models.

Attribution

Some of the code in this PR is based on previous work by @ryanmerolle in PR #134.

Double Check

  • I have explained my PR according to the information in the comments
    or in a linked issue.
  • My PR targets the dev branch.

@pheus
Copy link
Contributor Author

pheus commented Mar 23, 2025

Note: This is a draft PR.

I rebased the work by @ryanmerolle from PR #134, cleaned up the codebase, and added additional tests. I'll be adding tests for ACLStandardRule and ACLExtendedRule in the coming days.

@pheus pheus force-pushed the housekeeping/47-develop-plugin-tests-models branch 2 times, most recently from dc69c16 to a872eea Compare March 28, 2025 22:09
@pheus pheus marked this pull request as ready for review March 28, 2025 22:15
@pheus
Copy link
Contributor Author

pheus commented Mar 28, 2025

I included a fix for Bug #258, as it was necessary to properly test the uniqueness of Interface and Direction.

Additionally, I cleaned up the model files access_lists.py and access_list_rules.py and added i18n (internationalization) support.

I appreciate your time reviewing all these changes. Thank you, @cruse1977 !

@pheus pheus mentioned this pull request Apr 20, 2025
Open
@pheus pheus force-pushed the housekeeping/47-develop-plugin-tests-models branch from a872eea to 78dbba8 Compare April 20, 2025 22:28
@pheus
Copy link
Contributor Author

pheus commented May 15, 2025

Hi @cruse1977,

Thank you again for taking the time to review this PR.

I wanted to mention that I’m happy to break out some of the enhancements and housekeeping changes (such as the i18n additions and the model file cleanups) into separate issues or pull requests if that would help streamline the review process or better align with the project's contribution practices.

Please let me know what you prefer, and I’ll adjust accordingly.

Thanks again for your guidance and support!

@pheus pheus marked this pull request as draft May 17, 2025 08:51
ryanmerolle and others added 16 commits May 17, 2025 11:36
@ryanmerolle @pheus
add draft test for plugin models
0df90b3
@ryanmerolle @pheus
minimize duplicate code in model tests
0cd43ef
@ryanmerolle @pheus
build acl models tests
06db83a
@ryanmerolle @pheus
test ACLInterfaceAssignment draft
e2f33d7
@abhi1693 @pheus
fixed typo
c79433c
@ryanmerolle @pheus
correct acl key
0194437
@ryanmerolle @pheus
draft host logic model
8f73a31
@pheus
chore(tests): move model tests to dedicated directory
c5418f6 
Renames `test_models.py` to `models/test_accesslists.py` to improve
test organization and maintainability. This is a structural change
with no modifications to test logic.
@pheus
chore(tests): Split model tests into separate files
8cde4f8
@pheus
chore(tests): Enhance test fixtures
51bd56a 
Refactor test setup to improve data creation and reusability. Add
detailed fixtures for devices, virtual chassis, clusters,
virtual machines, and prefixes. Update all assignments to reference the
new fixtures for better consistency.
@pheus
chore(tests): Add tests for ACL creation
624f796 
Add test cases to validate AccessList creation and uniqueness for
VirtualChassis and VirtualMachine objects. Extend coverage by ensuring
proper handling of duplicate names and enforcing unique constraints.
@pheus
fix(models): Ensure interface host matches access list host
560d937 
Move validation to the `save` method to enforce that the assigned
interface's host matches the access list's host before saving.
This replaces validation in the `clean` method for better data integrity
enforcement.
@pheus
chore(tests): Enhance ACLInterfaceAssignment tests
6a2ee86 
Improve test coverage for ACLInterfaceAssignment validation.
Add tests for valid and invalid direction choices,
duplicate assignments, and missing host assignments to ensure robust
validation logic.
@pheus
refactor(models): Centralize ACL rule validation
871b0b7 
Moves ACL rule validation logic from forms to model methods to ensure
consistent enforcement across all usages. This improves maintainability
and eliminates redundant validation code.
@pheus
chore(tests): Add ACLStandardRule test coverage
df0491e 
Adds comprehensive tests for the ACLStandardRule model. Tests cover rule
creation scenarios, validation constraints, and edge cases,
ensuring thorough model behavior validation.

Fixes netbox-community#47
@pheus
chore(tests): Add ACLExtendedRule test coverage
3fa1506 
Introduces comprehensive test cases for the ACLExtendedRule model. These
cover rule creation, validation, and edge cases such as duplicate
indices and invalid combinations of parameters.

Fixes netbox-community#47
@pheus pheus force-pushed the housekeeping/47-develop-plugin-tests-models branch from eb4723a to 3fa1506 Compare May 17, 2025 09:45
@pheus pheus marked this pull request as ready for review May 17, 2025 09:54
@pheus
Copy link
Contributor Author

pheus commented May 17, 2025

Hi @cruse1977,

Quick update on this PR:

  • I’ve rebased the branch on the current dev to stay up to date.
  • The TODO change has been dropped to keep the scope focused.
  • The internationalization (i18n) changes have been split into their own feature request and a separate PR for clarity and easier review.
  • I also removed the model changes that would have required a migration, as they were outside the scope of adding model tests.
  • The changes related to issue [Bug]: ACL Interface Assignment Should Be Unique per Interface and Direction #258 have been removed to keep this PR focused solely on test coverage.

One note: the following commit is still part of this PR:

refactor(models): Centralize ACL rule validation

Moves ACL rule validation logic from forms to model methods to ensure
consistent enforcement across all usages. This improves maintainability
and eliminates redundant validation code.

These changes are necessary for the tests to pass. I'm happy to extract this into its own issue and PR if you prefer, but in that case, this models test PR would need to wait for those changes to be merged first.

Let me know how you’d like to proceed. Thanks again for your feedback and support!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Housekeeping]: Develop Plugin Tests - Models
3 participants
@pheus @ryanmerolle @abhi1693