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

Write tests for untested code. #104

Merged
merged 29 commits into from
Oct 20, 2023

Conversation

khalford
Copy link
Member

Cleaned up tests. Renamed files and refactored file structure. Moved some methods to more appropriate locations.

These changes were lost in a git checkout stash pop problem.
Classes no longer inherit from netbox_connect anymore. This caused issues with certain ,ethods needing access to the netbox api object and others not. To fix this the api object will be created once at the beginning of a user method and passed around each method thereafter. Also, fixed some type hinting mistakes.
These methods have been moved as they are to do with formatting dictionaries. It makes more sense to keep them in format_dict for now. Changed the test formatting to accommodate.
This test will need to test the netbox get id methods.
Pynetbox_Data_Uploader/.pylintrc Show resolved Hide resolved
Pynetbox_Data_Uploader/lib/netbox_api/netbox_check.py Outdated Show resolved Hide resolved
Also, it allows dependency injection testing.
"""
self.enums_no_id = DeviceInfoNoID
self.netbox = netbox
Copy link
Collaborator

Choose a reason for hiding this comment

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

Format dict shouldn't do anything which requires the netbox API....

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't do anything with Netbox. It just passes the api object through to the netbox_data.py . This allows you to initialise the api once at the start of a script and pass the api around from there, Rather than moving the url and tokens around and initialising a new api object for every method/class.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to add a file called __init__.py to utils to resolve the import error, since this is a module but we're missing the module "marker file"

Copy link
Member Author

Choose a reason for hiding this comment

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

utils does an __init__.py already in the directory.

Pynetbox_Data_Uploader/lib/utils/csv_to_dict.py Outdated Show resolved Hide resolved
Pynetbox_Data_Uploader/lib/utils/csv_to_dict.py Outdated Show resolved Hide resolved
@khalford
Copy link
Member Author

Pylint is going to fail on this branch due to the .pylintrc edit. I have already fixed these issues in another branch which I will merge once this branch merges.

@DavidFair
Copy link
Collaborator

Sorry I sent you down the wrong track:

 tests/test_csv_to_dict.py:2: in <module>
    from utils.csv_to_dict import FormatDict

The test file name (csv_to_dict.py) and the import needs fixing for this case, to match your rename to format_dict.py

Once that's done I'm happy to pair program the pylint error, as suppressing it means it's not actually checking that file

@meoflynn meoflynn merged commit 929845d into stfc:master Oct 20, 2023
2 checks passed
@khalford khalford deleted the write_tests_for_untested_code_v2 branch October 20, 2023 08:49
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