This repository was archived by the owner on Dec 20, 2023. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds everything necessary to write tests for the functions in
apiserver/data/context
, i.e. the ones that can be overridden easily for testing using Context (the current version introduced in #64). It also adds an initial routing test and some docs on when/how to use modules vs context functions.data/api
functions) and should not instantiate any connections themselves. This is still a somewhat leaky abstraction, as we still havesource_session
. Modules should be tested, as they only call context functions which can be easily replaced in tests.data/api
) and other functions if they need business logic. The amount of logic should be minimized, they should really be all about loading and storing data. If possible, logic should be separated into different functions that can be easily unit tested. Context functions shouldn't really need to be tested, but the functions they call should be.Sometimes, a router or module wants to directly call just a single
data/api
function. To make it testable, they should only call context functions. However, this adds an additional layer between the data API functions and using them and it is quite a lot of boilerplate to copy them, instantiate connections, add typing, add context stubs, etc. So, a new solution was invented:ctxlize
is a new function in thedatacontext
package. It takes any function and a wrapper function and then allows it to be called with a WrapContext. The arguments can be transformed in an arbitrary way by the wrapper function, which is defined at the application level. We use it to turn the AsyncConnection argument into a Source argument by instantiating a connection in the wrapper function. Example:It doesn't require stubs to be written nor a decorator on the original function. You simply create a WrapContext for each test and add only the functions you want to change to it and then replace the
wrap
attribute of the app's Code object. This means you do have to check yourself that they match properly, this isn't checked like it is for the usual way of creating context functions. The types look quite complicated but if you use it with Pylance in VS Code it should work quite well.Some functions still need to be switched to this. Also, the
require_admin
etc. needs proper context functions and probably some refactoring. But for now this should be enough.