Proposal: Writing unit tests for api/

[thesujai]

Why do we need this ?

If I can correctly infer the backend/ directory structure, I observe tests/ directory is structured to give tests as per the mhq/ dir. I only see tests for service/. No api tests are there.
Tests looks like good-to-have but it is a critical part of code imo.

Acceptance Criteria

• Configure test client
• Write unit tests

Further Comments / References

I have some good experience writing tests(not more than a year though:)). I can provide pretty good coverage for this.

[thesujai]

@jayantbh cc

[jayantbh]

Thank you for opening the issue, @thesujai. :)
I agree. We've not been doing great in the testing area, but are catching up on it.

Any help is welcome. :)

[thesujai]

I am taking this on, you can assign me.

I will create one PR for a .py file in api/(or adjust it not letting a PR be too long and non-reviewable).

[samad-yar-khan]

@thesujai Would like to know how you plan to achieve this before you start working on this. Although the current tests are not for APIs, they are for most of the core analytical functions that are used across the code base and in the APIs. API tests do make sense, would love to know your approach.

[jayantbh]

@thesujai ^ would like to know that.

[thesujai]

@jayantbh @samad-yar-khan
With my approach, we would first create a test_client (This will ensure that the main application is not affected by API requests, nor will we have to keep running the backend server for the test period).
Flask and pytest allow us to create a test client like below:

@pytest.fixture
def client(self):
        app = Flask(__name__)
        app.testing = True
        with app.test_client() as client:
            yield client

Now for each API endpoint we will use this client to make requests, and match the actual response with the expected response.

For example if we take the endpoint /teams/<team_id>/lead_time/prs we would write test like this:

test_get_lead_time_prs(some_mocks):
   # we setup the mocks here, like what should some of the function return when it is an actual API call
   # And we also define expected output, based on how we mock
   # then make a call like below
   response = client.get(
            f"/teams/team1/lead_time/prs",
            query_string={
                "from_time": from_time,
                "to_time": to_time,
                "pr_filter": json.dumps({"key": "value"})
            }
        )
   assert response.status_code = 200
   assert response.get_json() = expected_output

The above one I showed is a success case. There can be some 400, or 500 cases which also need to be checked. So for them, I will be asserting, for a bad request, if the server responding with an error(Or is the server behaving unexpectedly)

One thing I can say is writing tests won't be as straightforward as it looks, lots of API endpoints are a little tricky and would require some complex mocking. Anyway, I would try to do some actual data testing(Avoiding mocks in someplace).

[thesujai]

Please let me know what you think about this approach, open to suggestions!