-
Notifications
You must be signed in to change notification settings - Fork 119
Configure Unittest for API #515
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
af43dc5
to
a0561d2
Compare
The only major refactor it does is in the app.py, where I moved all the blueprint import and their registration to The only reason for doing this is getting the |
This is only for the initial review, if my approach for writing test is okay to you then only I will move forward with more tests. |
@thesujai Thank you for the proposal. :) I have a few concerns. We use Postgres as our primary database and I am not sure if sqlite provides all the functionalities we use in our database layer. SqlAlchemy might provide an abstraction layer over this, but we can't be sure if all the functions behave the same in sqlite and postgres, especially advanced datatypes like json and arrays, which we use quite heavily throughout our codebase. It would make the testing and prod environment behave differently which would be much harder to catch. You can look into testcontainers for spawning postgres containers wherever needed. It is a very convenient library and allows to mimic your production environment. |
Speaking of mimicing production environment, I think we can take this opportunity to introduce integration tests at either the API layer or at the Service layer. We can have a ton of unit tests as they are easier to write with mocked behaviour, or we can have a fewer "integration tests" to catch any bad assumptions unit tests were making. For example, the test of resolved incidents api in this pr, doesn't really test for much, except how the response is being adapted with the assumption the underlying business logic is correct. And this would be a fair assumption if the underlying logic/modules were well tested for how they behave with the database. But unfortunately, that's not the case. PS: When I mention "integration tests" i mean tests interacting with a real database instance, instead of mocking out that behaviour. Not sure what the exact term for that is. @jayantbh @samad-yar-khan Would like your input in this as well. |
I would love to add some functional testing as well where the test DB is actually(I wrote tests like those for a different OSS, but that was easier for me as I was very familiar with the Database Design there). I will try to use |
I will check this out |
392a7ce
to
4c2617e
Compare
As discussed in slack, not a requirement right now, might be in future! |
Linked Issue(s)
Related to #506
Acceptance Criteria fulfillment
Proposed changes (including videos or screenshots)
create_app
which will return the app object. Done this so I can use the same flask app object in the test_client also(reducing the need to duplicate the code). This will not affect the current working of the app in anyways.get_resolved_incidents
Further comments