Closed naved001 closed 6 years ago
I am actually not sure why we didn't we use the initial_db fixture in the first place. Couldn't find it in #617 (there are too many comments to sort through, and github is crashing when loading a page with that many comments).
@Izhmash Addressed your comments, and fixed some more typos that I found.
This looks basically reasonable to me.
I agree re: initial_db; There was a lot in that PR that I was really annoyed with at the time, and merging it was much more of a quality compromise than I'm normally willing to make.
should we get the user tests to use mock authentication too?
I explained this in person but for posterity, the reason we can't use mock auth for Test_user is because that class is actually testing the calls specific to the database auth backend (creating users, deleting users etc).
Ack, looking at the merge conflicts now and I'm realizing I didn't think this through. There's a big problem with using initial_db vs. the code that I'm wrote in #1007; none of the obmd_uris in initial_db are actually functional, and they need to be for the integration tests.
Hmmm, you coud use the additional_db fixture (which you can modify to have valid obmd parameters). Or maybe just register a new node or two with valid details just for testing obm calls.
Ended up doing the latter; I've merged master back in on my branch. Will set about getting the cli tests to pass again.
Made some changes in the client tests so that they are faster:
In travis, the client tests were taking around 190 seconds to run, now it runs in like 100 seconds.