VariantEffect / mavedb-api

MaveDB API
GNU Affero General Public License v3.0
8 stars 2 forks source link

Consider moving app creation to a factory model #183

Open bencap opened 4 months ago

bencap commented 4 months ago

Right now, the FastAPI app is defined in a script like fashion. This forces our test suite to override dependencies on the fly to mock admin / anonymous user rights (and would force us to do this for additional roles as we add them, if we would like to test their permission boundaries).

Moving to an app factory would let us have different test clients with different permissions boundaries, which seems much cleaner. Note though, this would also probably substantially increase the test suite runtime since app instances would need to be set up and torn down between each test.

afrubin commented 4 months ago

Permissions have been a source of disruptive bugs and user reports over the life of the project so I'm strongly in favor of making them easier to test (especially if we are going to add new roles, such as curator, in addition to admins and regular users).

The increased test suite runtime doesn't bother me, but then again I'm only touching the code rarely at this point. Presumably most folks aren't running all the tests all the time so this would mostly only affect development that's touching the permissions and CI runs, right?