-
Notifications
You must be signed in to change notification settings - Fork 82
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
Test query host by ID #268
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe some of these tests overlap with some tests in the QueryTestCase (test_query_using_host_id_list, test_query_using_host_id_list_include_nonexistent_host_ids, etc).
However, I like the approach of moving these tests into their own class. This makes the purpose of these tests more clear and should allow for easier splitting of these tests into their own files, etc later on.
Please remove the duplicated tests from QueryTestCase class and add the other host_id_list query related (tests test_query_using_host_id_list_one_host_id_does_not_include_hyphens) to this new class.
You’re right, @dehort! Some old tests are fully covered by the new ones, resulting in overlap:
Moved the missing host ID tests to the new class:
The host ID list tests are now together and so are the all hosts tests. Thanks! |
Aaaaargh, a conflict! |
The plain /hosts/{host_id_list} query has not had any tests yet. Added some testing: * Regular queries for a single and multiple hosts * Queries with valid, but non-existent IDs * Invalid queries This complements the create hosts tests of UUID validation. Reused the existing pagination tests.
The tests in QueryTestCase overlaps to some extent with new tests in QueryByHostIdTestCase. Removed those from QueryTestCase that are now completely redundant and moved those that test getting hosts by ID list to QueryByHostIdTestCase. Now, there are related tests in separate classes, entropy is lower.
97a8304
to
2724e2d
Compare
Resolved. 🎉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it.
The plain /hosts/{host_id_list} query has not had any tests yet. Added some testing:
This complements the create hosts tests of UUID validation. Reused the existing pagination tests.
This is a part of fixes for #265. Related to #92.