Skip to content

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

Closed
wants to merge 3 commits into from

Conversation

thesujai
Copy link
Contributor

@thesujai thesujai commented Aug 9, 2024

Linked Issue(s)

Related to #506

Acceptance Criteria fulfillment

  • Configure test client

Proposed changes (including videos or screenshots)

  1. Refactored the app.py by defining 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.
  2. Created a conftest file where all the test configuration are there. Used sqlite as the testing db
  3. Created a HelloWorldTest for a reference for writing other tests
  4. Written test for get_resolved_incidents

Further comments

@thesujai
Copy link
Contributor Author

thesujai commented Aug 9, 2024

The only major refactor it does is in the app.py, where I moved all the blueprint import and their registration to create_app. This doesn't have any regression with my testing.

The only reason for doing this is getting the app object, which if I imported without doing the refactoring would have resulted in the test sharing the same database as the main app(Which ig should be separated)

@thesujai
Copy link
Contributor Author

thesujai commented Aug 9, 2024

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.

@adnanhashmi09
Copy link
Contributor

@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.

@adnanhashmi09
Copy link
Contributor

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.

@thesujai
Copy link
Contributor Author

thesujai commented Aug 12, 2024

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 less mocking here(I don't like it either!) (I didn't put much effort into writing this test, as it was for initial review) so I will be setting up mock data into the table before running tests, but complex e2e tests involving setting up 3-4 tables before hitting an API can be dealt with as a follow-up issue, where we will have the base for API tests configured already? This will make writing integration tests much easier imo.

@thesujai
Copy link
Contributor Author

You can look into testcontainers for spawning postgres containers wherever needed. It is a very convenient library and allows to mimic your production environment.

I will check this out

@adnanhashmi09 adnanhashmi09 marked this pull request as draft September 5, 2024 05:03
@thesujai
Copy link
Contributor Author

thesujai commented Sep 22, 2024

As discussed in slack, not a requirement right now, might be in future!

@thesujai thesujai closed this Sep 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants