Skip to content

Refactor unit-test data structure: improve flexibility and remove abandoned data #1498

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
cernymi wants to merge 5 commits into
base: devel
Choose a base branch
from
Open

Refactor unit-test data structure: improve flexibility and remove abandoned data #1498

cernymi wants to merge 5 commits into from

Conversation

@cernymi
Copy link
Contributor

@cernymi cernymi commented Dec 9, 2025

Related Issue

None

New Behavior

Not changed

...

Contrast to Current Behavior

The same

...

Discussion: Benefits and Drawbacks

This update improves how unit-test data is organized.
The structure is now more flexible, and abandoned data has been removed.
These adjustments lay the groundwork for a future change where the single large test_netbox_module test file will be split into multiple, logically grouped files.
That restructuring is not part of this PR, but this prepares for it.

  • Easier to add new test scenarios thanks to a cleaner, more flexible layout.
  • Sets up a smoother transition to smaller, focused test files.

...

Changes to the Documentation

None

...

Proposed Release Note Entry

  • Make the unit-test data structures more flexible.
  • Remove abandoned unit-test data.

...

Double Check

  • I have read the comments and followed the CONTRIBUTING.md.
  • I have explained my PR according to the information in the comments or in a linked issue.
  • My PR targets the devel branch.

@cernymi cernymi force-pushed the cernymi/unit_tests_data branch 3 times, most recently from c68c515 to db9f90e Compare December 9, 2025 22:54
@cernymi cernymi changed the title Cernymi/unit tests data Refactor unit-test data structure: improve flexibility and remove abandoned data Dec 9, 2025
@cernymi cernymi marked this pull request as draft December 9, 2025 23:59
@sc68cal
Copy link
Contributor

sc68cal commented Dec 10, 2025

Looks promising

@cernymi cernymi force-pushed the cernymi/unit_tests_data branch 5 times, most recently from 82e3aaf to 0a59eda Compare December 10, 2025 20:01
@cernymi
Rewrite load test data function in unit tests
07339b8 
Helper to load test data for unit tests.

This function supports loading data files relative to the test file.
It works even if the test file is in a subfolder, allowing your
test data to be organized in corresponding subfolders alongside the tests.
@cernymi
Move unit module_utils tests to netbox_utils folder
06ea34d
@cernymi
Rename unit test data.json files to file_name.json
a87c45b
@cernymi cernymi force-pushed the cernymi/unit_tests_data branch from 0a59eda to a87c45b Compare December 10, 2025 20:27
@cernymi cernymi marked this pull request as ready for review December 10, 2025 20:44
@cernymi
Copy link
Contributor Author

cernymi commented Dec 10, 2025

Differences between virtual-env paths and normal paths caused a few problems, but now load_test_data() handles both correctly.
Let me know if you'll find something to improve

@sc68cal
Copy link
Contributor

sc68cal commented Dec 10, 2025

OK so to close the loop on this, there was a pytest fixture in tests/unit/module_utils/test_netbox_base_class.py named choices_data that would use these static JSON files, but 36c6b16 removed that fixture and these files were left orphaned, correct?

@cernymi
Copy link
Contributor Author

cernymi commented Dec 11, 2025

That’s almost certainly correct. The choices_data fixture was probably meant to load data from fixtures/choices, but I can’t confirm it — I can’t find a single use of the fixture anywhere.

It looks like choices_data was introduced in commit 96540e8 and then replaced the next day by the new load_test_data function. Since then, the fixtures/choices files appear to have been abandoned.

Should I make the commit message more specific? Happy to take suggestions.

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.

2 participants

@cernymi @sc68cal