Open angus-lherrou opened 1 year ago
@pwildenhain I would like to add a unit test for this but I'm not sure how to mock an internal server error in the testing framework
Merging #263 (ab45bd9) into master (a507d1a) will decrease coverage by
0.35%
. The diff coverage is71.42%
.
@@ Coverage Diff @@
## master #263 +/- ##
===========================================
- Coverage 100.00% 99.65% -0.35%
===========================================
Files 19 19
Lines 568 573 +5
===========================================
+ Hits 568 571 +3
- Misses 0 2 +2
Impacted Files | Coverage Δ | |
---|---|---|
redcap/request.py | 97.22% <71.42%> (-2.78%) |
:arrow_down: |
@pwildenhain I would like to add a unit test for this but I'm not sure how to mock an internal server error in the testing framework
Yea the unit testing framework is a lot to take in at first, but it's actually pretty neat, and could be good to learn if you're ever looking to add unit tests for one of your own API wrapper packages
I would recommend building in a fake json decode error and fake 500 response into some handlers for the survey project test fixture (found in tests/unit/test_survey_project.py at the top)
You'll see in that fixture that it's dynamically looking for request handlers, which you can find for the survey project fixture at the end of tests/unit/callback_utils.py
I suggest adding a few more handlers for things like DAGs, users, etc and then raising a json decode error in one of them and returning a 500 (instead of a 201) in the other one.
Then add tests for that survey project where you try to access those records and use pytest.raises(RedCapError) to ensure that they were caught
Let me know if this is unclear, or if you are not interested in adding tests and I can try to add them in.
Also sorry for the crap formatting I'm writing this on mobile
Thanks, I will take a crack at it when I have a chance this week.
Fixes #261.
Also raises a RedcapError from any JSONDecodeError in the response.json() call, but this can be reverted if it's too out of scope for this PR.