Feature/allocation model get information tests #707
Conversation
Eric-Butcher
force-pushed
the
feature/allocation-model-get-information-tests
branch
from
b427b6b to
5e8a5ae
Compare
|
The test cases where the Also it seems like |
Eric-Butcher
force-pushed
the
feature/allocation-model-get-information-tests
branch
from
5e8a5ae to
82c8445
Compare
Eric-Butcher
force-pushed
the
feature/allocation-model-get-information-tests
branch
from
82c8445 to
f8fddb7
Compare
| name_not_in_view_list_2 = "This Is Not In List" | ||
| sample_allocation_attribute_view_list = [name_in_view_list_1, name_in_view_list_2] | ||
|
|
||
| def find_percent(self, numerator, denominator): |
There was a problem hiding this comment.
We should add a comment to explain the math here without having to dig into what is happening in models.py, especially the multiplication by 10000
There was a problem hiding this comment.
(Kind of) done.
This rounding is actually a lot more troublesome than it might appear. I do not think the original function was written that soundly, probably could have just used a percent formatter instead. Because of floating point and rounding imprecision problems tiny deviations in how that percentage is obtained could alter results in some minor instances even though it probably does not matter
| @override_settings(ALLOCATION_ATTRIBUTE_VIEW_LIST=sample_allocation_attribute_view_list) | ||
| def test_attribute_type_not_in_view_list_returns_without_type_substring_included(self): | ||
| """Test that the get_information method returns a string without the attribute type substring when the type name is not in ALLOCATION_ATTRIBUTE_VIEW_LIST.""" | ||
| allocation: Allocation = AllocationFactory() |
There was a problem hiding this comment.
I think we we move this allocation factory call to a test setup function if every test in this class runs the same setup step
| # Allocation that we are testing, needed for creating AllocationAttributes | ||
| tested_allocation: Allocation = AllocationFactory() | ||
|
|
||
| # AllocationAttribute for each AllocationAttributeType |
There was a problem hiding this comment.
I have concerns about how maintainable this test will be, I think maybe adding some comments to provide more context for how you're constructing the test example might make it a little easier to parse and understand.
There was a problem hiding this comment.
If it is too much I could reduce it down just just a single one which is in view and a single one which is not inside view, but the models are just so interdependent and necessary for the function that all of those steps have to be in there. I tried to give them variable names that were as descriptive as possible but if you have any suggestions on that front let me know
Eric-Butcher
force-pushed
the
feature/allocation-model-get-information-tests
branch
from
f8fddb7 to
16577fb
Compare
Signed-off-by: Eric Butcher <[email protected]>
Eric-Butcher
force-pushed
the
feature/allocation-model-get-information-tests
branch
from
16577fb to
10214cd
Compare
No description provided.