Split up test_discount_processor.py into tests for the discount processor and pi-su credit processor#290
Split up test_discount_processor.py into tests for the discount processor and pi-su credit processor#290marcoagonzales007 wants to merge 1 commit into
Conversation
QuanMPhm
left a comment
QuanMPhm
left a comment
There was a problem hiding this comment.
Aside from the comment below, can you add your PR description into the commit message as well?
| ) | ||
|
|
||
| processor = PISUCreditProcessor( | ||
| processor = test_utils.new_pi_su_credit_processor( |
There was a problem hiding this comment.
Since this file only tests the features of DiscountProcessor. You should not be using the New-PI processor here. I would suggest using the DiscountProcessor and call apply_flat_discount() directly instead. Something like this:
processor = DiscountProcessor(data=pandas.Dataframe())
processor.apply_flat_discount(...)
...You shouldn't need to add an function for DiscountProcessor in util.py, since its only used once here.
marcoagonzales007
force-pushed
the
main
branch
2 times, most recently
from
46aaf3b to
57c6909
Compare
QuanMPhm
left a comment
QuanMPhm
left a comment
There was a problem hiding this comment.
One minor comment. Otherwise, looks good!
| !process_report/tests/e2e/test_data/test_invoices/*.csv | ||
| __pycache__/ | ||
| *.py[cod] | ||
| .vscode/ |
There was a problem hiding this comment.
I assume this is something Visual Studio Code added to the repo when you worked on it, in which case I would suggest not putting this here, since it doesn't benefit anyone else developing on this repository. If everyone added things to .gitignore that's specific to their work setup, the file will become messy quick.
If you want a personal ignore rules for your fork, I'd suggest something described here
- Moved test_one_eligible_project_only and test_all_eligible_projects into new test_pi_su_credit_processor.py - Kept test_preexisting_credit in test_discount_processor.py, using DiscountProcessor directly - Added new_pi_su_credit_processor() factory to tests/util.py
2 participants
-Moved
test_one_eligible_project_onlyandtest_all_eligible_projectsinto a newtest_pi_su_credit_processor.pybecause they test thePISUCreditProcessor._processlogic-Kept
test_preexisting_creditintest_discount_processor.py-Added
new_pi_su_credit_processor()totests/util.pyto match the other processors-Updated both of the test files to use
test_utilsinstead of directly usingPISUCreditProcessorCloses Issue #289