new unit test module for auth_handlers.py #611
Conversation
|
@wasade @ElDeveloper @josenavas could you please look into the Travis error. I have the feeling that it is not related to my unit tests. |
|
Weird... either a config entry changed or something in the BG text? Not On Tue, Sep 6, 2016 at 6:26 PM, Stefan Janssen [email protected]
|
|
Master is having the same problem: #612 (I only added an empty file). @EmbrietteH are you aware of any config changes? |
|
Not that I am aware of... |
|
found the bug: its a ’ character in the SHIPPING_ADDRESS of amgut/lib/locale_data/british_gut.py line 86. |
|
@wasade @josenavas @EmbrietteH now that Travis passes, I am happy to get some feedback about my first unit test for AG. |
| (port, url_escape(text_locale['handlers']['INVALID_KITID']))) | ||
|
|
||
| unregistered_kits = ag_data.get_all_handout_kits() | ||
| if len(unregistered_kits) <= 0: |
There was a problem hiding this comment.
if you are modifying the database, you should add a tearDown method in the test class that reverts those changes. If you do that, this error will never be raised, so you can remove this code.
There was a problem hiding this comment.
Honestly, I don't know how I can undo the changed. I have the feeling that there is no python method to undo this specific operation, thus I need to operate on the database myself or revert the complete database?! Any suggestions how I should proceed here?
There was a problem hiding this comment.
@josenavas, in the CI environment, the database is reset on every build, and on a local system, this would require probably thousands of runs before its an issue, right?
There was a problem hiding this comment.
Not sure - but I'm more concerned if a change in the order of the tests is going to fire up a stochastic failure. If we don't foresee this issue, what we can do is open an issue, put a TODO comment on the code with the link to the issue and try to fix later, so we don't loose track of that. I don't know from the top of my head how to revert this changes in the database, as that is specific to the code being executed, but I was told that in this repository, instead of reseting the full database in each test, the local changes are reverted
There was a problem hiding this comment.
I dived into the SQL code. When a kit is registered to an user, the kit is inserted into the table ag_kit and removed from the table ag_handout_kits. During this process, we loose the information in column "created_on". Thus, I cannot revert this function.
To have the DB in the same state as before the test, I would need to write code which anticipates what is going to happen, store the information which is going to be lost somewhere else, perform the SQL operation which I want to test, and create a new function which takes the otherwise stored information to revert the tested SQL operation. This sounds not only quite complicated, but surely introduces potential mistakes + the need to keep code in sync.
What shall I do??
There was a problem hiding this comment.
Thanks for taking a look @sjanssen2 !! I think for the time being you can create a new issue, and put a comment in the code: #TODO: see issue #XXX so we can keep track of it.
I think at some point we can do something like what we are doing on Qiita ATM, reset the DB on each class (rather than test), but I think this will need a longer discussion.
There was a problem hiding this comment.
I opened the issue #618 and added a TODO comment in the code
|
Thanks @sjanssen2 minor comments! |
…into unittest-auth_handlers
My first try to add proper unit tests for a handler class. All comments are appreciated.