Closed nautics889 closed 1 year ago
IDK why tests are failing on URL assertion, when i run it locally, those tests are passed well.
Either via pytest
directy:
$ pytest
...
tests/test_app.py::AppCustomFieldsTestCase::test_custom_fields PASSED [ 2%]
tests/test_app.py::AppCustomFieldChoicesTestCase::test_custom_field_choices PASSED [ 2%]
...
or via invoke
$ export NAUTOBOT_URL="http://localhost:8000"
$ invoke tests
...
tests/test_app.py::AppCustomFieldsTestCase::test_custom_fields PASSED [ 2%]
tests/test_app.py::AppCustomFieldChoicesTestCase::test_custom_field_choices PASSED [ 2%]
...
Working on it...
Done, tests are fixed. The problem was that i'd added checking for passed URL like that:
expect_url in mock_obj.call_args.args
However .args
and .kwargs
attributes are only available from 3.8 version.
That's why tests were failing only on 3.7.
@pszulczewski with what you have been seeing, this look good?
@pszulczewski Apologies for the delayed response. Thank you for the review!
I have uploaded a commit with all the changes you've suggested.
custom_fields()
and custom_field_choices()
✓.format()
✓return_value
instead of side_effect
for mocks in related test-cases ✓All CI checks are passed. I'd appreciate if you could take a look when you have time.
p. s. about docstrings: i had inteded to make them as you described at first, but i was afraid that would look too huge for a docstring =)
@pszulczewski thank you for the feedback!
I've uploaded the last minor changes you've mentioned
You need just that after logger.
p. s. there was some problem with CI runners (or rather with dependencies being installed for tests as seems to me according to logs). Failed job: https://github.com/nautics889/pynautobot/actions/runs/5660539569/job/15336640442
p. s. there was some problem with CI runners (or rather with dependencies being installed for tests as seems to me according to logs). Failed job: https://github.com/nautics889/pynautobot/actions/runs/5660539569/job/15336640442
Those are the actions in your fork. They are passing in this repo: https://github.com/nautobot/pynautobot/actions/runs/5660540032/job/15336460260
Your branch is 20 commits behind develop, so may be worth pulling in the latest changes....or just disable and ignore your own CI.
Those are the actions in your fork
Indeed, didn't notice, thank you
Since Nautobot API has two different endpoints for custom fields and custom field choices (/api/extras/custom-fields/ and /api/extras/custom-field-choices/ respectively), it might seem some ambiguous that we have a method
custom_choices()
which actually sends a request to /custom-fields/.Rename:
App.custom_choices()
→App.custom_fields()
(sends a request to /extras/custom-fields/)AppCustomChoicesTestCase
→AppCustomFieldsTestCase
(tests for sending a request to /extras/custom-fields/)Add:
App.custom_field_choices()
(sends a request to /extras/custom-field-choices/)AppCustomFieldChoicesTestCase
(tests for sending a request to /extras/custom-field-choices/)I also add checking of called URL in the tests. I presume it's a point to be discussed, because without this checking there's a possibility that tests don't display an error when the method being tested sends request to wrong URL. Considering we have mocks all over test module. So as for me, it'd be good to add such assertions in other places of test modules.